Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type

[Lars Schneider]

On 17 Feb 2016, at 14:12, Johannes Schindelin  
wrote:

> Hi Lars,
> 
> On Mon, 15 Feb 2016, larsxschnei...@gmail.com wrote:
> 
>> From: Lars Schneider 
>> 
>> Use the config type to print more detailed error messages that inform
>> the user about the origin of a config error (file, stdin, blob).
> 
> Given that you settled on `--show-origin` as the command-line option, I
> would have expected s/type/origin/g, too...

Agreed. Junio suggested "origin_type" which I like, too.

Thanks,
Lars--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type

[Johannes Schindelin]

Hi Lars,

On Mon, 15 Feb 2016, larsxschnei...@gmail.com wrote:

> From: Lars Schneider 
> 
> Use the config type to print more detailed error messages that inform
> the user about the origin of a config error (file, stdin, blob).

Given that you settled on `--show-origin` as the command-line option, I
would have expected s/type/origin/g, too...

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type

[Ramsay Jones]

On 15/02/16 17:42, Jeff King wrote:
> On Mon, Feb 15, 2016 at 11:17:45AM +0100, larsxschnei...@gmail.com wrote:
> 
>> From: Lars Schneider 
>>
>> Use the config type to print more detailed error messages that inform
>> the user about the origin of a config error (file, stdin, blob).
>>
>> Signed-off-by: Lars Schneider 
>> ---
>>  cache.h|  6 --
>>  config.c   | 36 +---
>>  submodule-config.c |  4 ++--
>>  t/t1300-repo-config.sh |  8 +++-
>>  t/t1308-config-set.sh  |  4 ++--
>>  5 files changed, 40 insertions(+), 18 deletions(-)
> 
> Looks good to me.
> 
> -Peff
> 

Hi Lars,

Could you please squash this into your patch.

Thanks!

ATB,
Ramsay Jones

-- >8 --
From: Ramsay Jones 
Date: Tue, 16 Feb 2016 21:35:41 +
Subject: [PATCH] config: avoid declaration missing prototype warnings

Signed-off-by: Ramsay Jones 
---
 cache.h  | 4 ++--
 config.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index 3ff94dc..7f1fa65 100644
--- a/cache.h
+++ b/cache.h
@@ -1546,8 +1546,8 @@ extern const char *get_log_output_encoding(void);
 extern const char *get_commit_output_encoding(void);
 
 extern int git_config_parse_parameter(const char *, config_fn_t fn, void 
*data);
-extern const char *current_config_type();
-extern const char *current_config_name();
+extern const char *current_config_type(void);
+extern const char *current_config_name(void);
 
 struct config_include_data {
int depth;
diff --git a/config.c b/config.c
index f1efba8..0a35323 100644
--- a/config.c
+++ b/config.c
@@ -2414,12 +2414,12 @@ int parse_config_key(const char *var,
return 0;
 }
 
-const char *current_config_type()
+const char *current_config_type(void)
 {
return cf && cf->type ? cf->type : "cmdline";
 }
 
-const char *current_config_name()
+const char *current_config_name(void)
 {
return cf && cf->name ? cf->name : "";
 }
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type

[Ramsay Jones]

On 15/02/16 10:17, larsxschnei...@gmail.com wrote:
> From: Lars Schneider 
> 
> Use the config type to print more detailed error messages that inform
> the user about the origin of a config error (file, stdin, blob).
> 
> Signed-off-by: Lars Schneider 
> ---
>  cache.h|  6 --
>  config.c   | 36 +---
>  submodule-config.c |  4 ++--
>  t/t1300-repo-config.sh |  8 +++-
>  t/t1308-config-set.sh  |  4 ++--
>  5 files changed, 40 insertions(+), 18 deletions(-)
> 
> diff --git a/cache.h b/cache.h
> index c63fcc1..8d86e5c 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1485,8 +1485,8 @@ struct git_config_source {
>  typedef int (*config_fn_t)(const char *, const char *, void *);
>  extern int git_default_config(const char *, const char *, void *);
>  extern int git_config_from_file(config_fn_t fn, const char *, void *);
> -extern int git_config_from_buf(config_fn_t fn, const char *name,
> -const char *buf, size_t len, void *data);
> +extern int git_config_from_mem(config_fn_t fn, const char *type,
> + const char *name, const char *buf, 
> size_t len, void *data);
>  extern void git_config_push_parameter(const char *text);
>  extern int git_config_from_parameters(config_fn_t fn, void *data);
>  extern void git_config(config_fn_t fn, void *);
> @@ -1525,6 +1525,8 @@ extern const char *get_log_output_encoding(void);
>  extern const char *get_commit_output_encoding(void);
>  
>  extern int git_config_parse_parameter(const char *, config_fn_t fn, void 
> *data);
> +extern const char *current_config_type();
> +extern const char *current_config_name();
>  
>  struct config_include_data {
>   int depth;
> diff --git a/config.c b/config.c
> index 86a5eb2..5cf11b5 100644
> --- a/config.c
> +++ b/config.c
> @@ -24,6 +24,7 @@ struct config_source {
>   size_t pos;
>   } buf;
>   } u;
> + const char *type;
>   const char *name;
>   const char *path;
>   int die_on_error;
> @@ -471,9 +472,9 @@ static int git_parse_source(config_fn_t fn, void *data)
>   break;
>   }
>   if (cf->die_on_error)
> - die(_("bad config file line %d in %s"), cf->linenr, cf->name);
> + die(_("bad config line %d in %s %s"), cf->linenr, cf->type, 
> cf->name);
>   else
> - return error(_("bad config file line %d in %s"), cf->linenr, 
> cf->name);
> + return error(_("bad config line %d in %s %s"), cf->linenr, 
> cf->type, cf->name);
>  }
>  
>  static int parse_unit_factor(const char *end, uintmax_t *val)
> @@ -588,9 +589,9 @@ static void die_bad_number(const char *name, const char 
> *value)
>   if (!value)
>   value = "";
>  
> - if (cf && cf->name)
> - die(_("bad numeric config value '%s' for '%s' in %s: %s"),
> - value, name, cf->name, reason);
> + if (cf && cf->type && cf->name)
> + die(_("bad numeric config value '%s' for '%s' in %s %s: %s"),
> + value, name, cf->type, cf->name, reason);
>   die(_("bad numeric config value '%s' for '%s': %s"), value, name, 
> reason);
>  }
>  
> @@ -1061,11 +1062,13 @@ static int do_config_from(struct config_source *top, 
> config_fn_t fn, void *data)
>  }
>  
>  static int do_config_from_file(config_fn_t fn,
> - const char *name, const char *path, FILE *f, void *data)
> + const char *type, const char *name, const char *path, FILE *f,
> + void *data)
>  {
>   struct config_source top;
>  
>   top.u.file = f;
> + top.type = type;
>   top.name = name;
>   top.path = path;
>   top.die_on_error = 1;
> @@ -1078,7 +1081,7 @@ static int do_config_from_file(config_fn_t fn,
>  
>  static int git_config_from_stdin(config_fn_t fn, void *data)
>  {
> - return do_config_from_file(fn, "", NULL, stdin, data);
> + return do_config_from_file(fn, "stdin", "", NULL, stdin, data);

I think this should be:
return do_config_from_file(fn, "file", "", NULL, stdin, data);

ie.  is not a separate type, but a file that does not exist in
the filesystem and, thus, has no name. (what you use internally is a
separate issue, but  works for me.)

Also, I'm so used to '' as the 'name' (token/handle) of that
file, that it looks very odd spelt otherwise. :-D

>  }
>  
>  int git_config_from_file(config_fn_t fn, const char *filename, void *data)
> @@ -1089,21 +1092,22 @@ int git_config_from_file(config_fn_t fn, const char 
> *filename, void *data)
>   f = fopen(filename, "r");
>   if (f) {
>   flockfile(f);
> - ret = do_config_from_file(fn, filename, filename, f, data);
> + ret = do_config_from_file(fn, "file", filename, filename, f, 
> data);
>   funlockfile(f);
>   fclose(f);
>   }
>   

Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type

[Jeff King]

On Mon, Feb 15, 2016 at 11:17:45AM +0100, larsxschnei...@gmail.com wrote:

> From: Lars Schneider 
> 
> Use the config type to print more detailed error messages that inform
> the user about the origin of a config error (file, stdin, blob).
> 
> Signed-off-by: Lars Schneider 
> ---
>  cache.h|  6 --
>  config.c   | 36 +---
>  submodule-config.c |  4 ++--
>  t/t1300-repo-config.sh |  8 +++-
>  t/t1308-config-set.sh  |  4 ++--
>  5 files changed, 40 insertions(+), 18 deletions(-)

Looks good to me.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html