# [PATCH] i18n: config: unfold error messages marked for translation 7 messages from 2016-07-27 to 2016-07-28. Participants: Vasco Almeida, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/42942 ## Vasco Almeida, 2016-07-27 17:59 Subject: [PATCH] i18n: config: unfold error messages marked for translation Message-ID: <1469642375-27305-1-git-send-email-vascomalmeida@sapo.pt> URL: https://gitlist.dev/e/1469642375-27305-1-git-send-email-vascomalmeida%40sapo.pt ``` Introduced in 473166b ("config: add 'origin_type' to config_source struct", 2016-02-19), Git can inform the user about the origin of a config error, but the implementation does not allow translators to translate the keywords 'file', 'blob, 'standard input', and 'submodule-blob'. Moreover, for the second message, a reason for the error is appended to the message, not allowing translators to translate that reason either. Unfold the message into several templates for each known origin_type. That would result in better translation at the expense of code verbosity. Add enum config_oringin_type to ease management of the various configuration origin types (blob, file, etc). Previously origin type was considered from command line if cf->origin_type == NULL, i.e., uninitialized. Now we set origin_type to CONFIG_ORIGIN_CMDLINE in git_config_from_parameters() and configset_add_value(). For error message in git_parse_source(), use xstrfmt() function to prepare the message string, instead of doing something like it's done for die_bad_number(), because intelligibility and code conciseness are improved for that instance. Signed-off-by: Vasco Almeida --- cache.h | 12 +++++- config.c | 117 ++++++++++++++++++++++++++++++++++++++++++++--------- submodule-config.c | 2 +- 3 files changed, 109 insertions(+), 22 deletions(-) diff --git a/cache.h b/cache.h index 3855ddf..c802098 100644 --- a/cache.h +++ b/cache.h @@ -1566,10 +1566,18 @@ struct git_config_source { const char *blob; }; +enum config_origin_type { + CONFIG_ORIGIN_BLOB, + CONFIG_ORIGIN_FILE, + CONFIG_ORIGIN_STDIN, + CONFIG_ORIGIN_SUBMODULE_BLOB, + CONFIG_ORIGIN_CMDLINE +}; + 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_mem(config_fn_t fn, const char *origin_type, +extern int git_config_from_mem(config_fn_t fn, const enum config_origin_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); @@ -1713,7 +1721,7 @@ extern int ignore_untracked_cache_config; struct key_value_info { const char *filename; int linenr; - const char *origin_type; + enum config_origin_type origin_type; enum config_scope scope; }; diff --git a/config.c b/config.c index bea937e..2b12730 100644 --- a/config.c +++ b/config.c @@ -24,7 +24,7 @@ struct config_source { size_t pos; } buf; } u; - const char *origin_type; + enum config_origin_type origin_type; const char *name; const char *path; int die_on_error; @@ -245,6 +245,7 @@ int git_config_from_parameters(config_fn_t fn, void *data) memset(&source, 0, sizeof(source)); source.prev = cf; + source.origin_type = CONFIG_ORIGIN_CMDLINE; cf = &source; /* sq_dequote will write over it */ @@ -453,6 +454,8 @@ static int git_parse_source(config_fn_t fn, void *data) int comment = 0; int baselen = 0; struct strbuf *var = &cf->var; + int error_return = 0; + char *error_msg = NULL; /* U+FEFF Byte Order Mark in UTF8 */ const char *bomptr = utf8_bom; @@ -507,10 +510,40 @@ static int git_parse_source(config_fn_t fn, void *data) if (get_value(fn, data, var) < 0) break; } + + switch (cf->origin_type) { + case CONFIG_ORIGIN_BLOB: + error_msg = xstrfmt(_("bad config line %d in blob %s"), + cf->linenr, cf->name); + break; + case CONFIG_ORIGIN_FILE: + error_msg = xstrfmt(_("bad config line %d in file %s"), + cf->linenr, cf->name); + break; + case CONFIG_ORIGIN_STDIN: + error_msg = xstrfmt(_("bad config line %d in standard input %s"), + cf->linenr, cf->name); + break; + case CONFIG_ORIGIN_SUBMODULE_BLOB: + error_msg = xstrfmt(_("bad config line %d in submodule-blob %s"), + cf->linenr, cf->name); + break; + case CONFIG_ORIGIN_CMDLINE: + error_msg = xstrfmt(_("bad config line %d in command line %s"), + cf->linenr, cf->name); + break; + default: + error_msg = xstrfmt(_("bad config line %d in %s"), + cf->linenr, cf->name); + } + if (cf->die_on_error) - die(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name); + die(error_msg); else - return error(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name); + error_return = error(error_msg); + + free(error_msg); + return error_return; } static int parse_unit_factor(const char *end, uintmax_t *val) @@ -619,16 +652,47 @@ int git_parse_ulong(const char *value, unsigned long *ret) NORETURN static void die_bad_number(const char *name, const char *value) { - const char *reason = errno == ERANGE ? - "out of range" : - "invalid unit"; if (!value) value = ""; - if (cf && cf->origin_type && cf->name) - die(_("bad numeric config value '%s' for '%s' in %s %s: %s"), - value, name, cf->origin_type, cf->name, reason); - die(_("bad numeric config value '%s' for '%s': %s"), value, name, reason); + if (!(cf && cf->name)) + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s': out of range") + : _("bad numeric config value '%s' for '%s': invalid unit"), + value, name); + + switch (cf->origin_type) { + case CONFIG_ORIGIN_BLOB: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in blob %s: out of range") + : _("bad numeric config value '%s' for '%s' in blob %s: invalid unit"), + value, name, cf->name); + case CONFIG_ORIGIN_FILE: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in file %s: out of range") + : _("bad numeric config value '%s' for '%s' in file %s: invalid unit"), + value, name, cf->name); + case CONFIG_ORIGIN_STDIN: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in standard input %s: out of range") + : _("bad numeric config value '%s' for '%s' in standard input %s: invalid unit"), + value, name, cf->name); + case CONFIG_ORIGIN_SUBMODULE_BLOB: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in submodule-blob %s: out of range") + : _("bad numeric config value '%s' for '%s' in submodule-blob %s: invalid unit"), + value, name, cf->name); + case CONFIG_ORIGIN_CMDLINE: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in command line %s: out of range") + : _("bad numeric config value '%s' for '%s' in command line %s: invalid unit"), + value, name, cf->name); + default: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in %s: out of range") + : _("bad numeric config value '%s' for '%s' in %s: invalid unit"), + value, name, cf->name); + } } int git_config_int(const char *name, const char *value) @@ -1105,7 +1169,8 @@ 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 *origin_type, const char *name, const char *path, FILE *f, + const enum config_origin_type origin_type, + const char *name, const char *path, FILE *f, void *data) { struct config_source top; @@ -1124,7 +1189,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, "standard input", "", NULL, stdin, data); + return do_config_from_file(fn, CONFIG_ORIGIN_STDIN, "", NULL, stdin, data); } int git_config_from_file(config_fn_t fn, const char *filename, void *data) @@ -1135,14 +1200,14 @@ 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, "file", filename, filename, f, data); + ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename, filename, f, data); funlockfile(f); fclose(f); } return ret; } -int git_config_from_mem(config_fn_t fn, const char *origin_type, +int git_config_from_mem(config_fn_t fn, const enum config_origin_type origin_type, const char *name, const char *buf, size_t len, void *data) { struct config_source top; @@ -1179,7 +1244,7 @@ static int git_config_from_blob_sha1(config_fn_t fn, return error("reference '%s' does not point to a blob", name); } - ret = git_config_from_mem(fn, "blob", name, buf, size, data); + ret = git_config_from_mem(fn, CONFIG_ORIGIN_BLOB, name, buf, size, data); free(buf); return ret; @@ -1390,12 +1455,12 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha if (cf->name) { kv_info->filename = strintern(cf->name); kv_info->linenr = cf->linenr; - kv_info->origin_type = strintern(cf->origin_type); + kv_info->origin_type = cf->origin_type; } else { /* for values read from `git_config_from_parameters()` */ kv_info->filename = NULL; kv_info->linenr = -1; - kv_info->origin_type = NULL; + kv_info->origin_type = CONFIG_ORIGIN_CMDLINE; } kv_info->scope = current_parsing_scope; si->util = kv_info; @@ -2476,14 +2541,28 @@ int parse_config_key(const char *var, const char *current_config_origin_type(void) { - const char *type; + int type; if (current_config_kvi) type = current_config_kvi->origin_type; else if(cf) type = cf->origin_type; else die("BUG: current_config_origin_type called outside config callback"); - return type ? type : "command line"; + + switch (type) { + case CONFIG_ORIGIN_BLOB: + return "blob"; + case CONFIG_ORIGIN_FILE: + return "file"; + case CONFIG_ORIGIN_STDIN: + return "standard input"; + case CONFIG_ORIGIN_SUBMODULE_BLOB: + return "submodule-blob"; + case CONFIG_ORIGIN_CMDLINE: + return "command line"; + default: + die("BUG: unknown config origin type"); + } } const char *current_config_name(void) diff --git a/submodule-config.c b/submodule-config.c index 077db40..1753e01 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -448,7 +448,7 @@ static const struct submodule *config_from(struct submodule_cache *cache, parameter.commit_sha1 = commit_sha1; parameter.gitmodules_sha1 = sha1; parameter.overwrite = 0; - git_config_from_mem(parse_config, "submodule-blob", rev.buf, + git_config_from_mem(parse_config, CONFIG_ORIGIN_SUBMODULE_BLOB, rev.buf, config, config_size, ¶meter); free(config); -- 2.7.4 ``` ## Junio C Hamano, 2016-07-27 20:00 Subject: Re: [PATCH] i18n: config: unfold error messages marked for translation Message-ID: URL: https://gitlist.dev/e/xmqqy44mvol9.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1469642375-27305-1-git-send-email-vascomalmeida@sapo.pt> ``` Vasco Almeida writes: > + > + switch (cf->origin_type) { > + case CONFIG_ORIGIN_BLOB: > + error_msg = xstrfmt(_("bad config line %d in blob %s"), > + cf->linenr, cf->name); > + break; > + case CONFIG_ORIGIN_FILE: > + error_msg = xstrfmt(_("bad config line %d in file %s"), > + cf->linenr, cf->name); > + break; Hmm, if we were to switch the message based on the origin type, then things like this: > + case CONFIG_ORIGIN_STDIN: > + error_msg = xstrfmt(_("bad config line %d in standard input %s"), > + cf->linenr, cf->name); > + break; and this: > + case CONFIG_ORIGIN_STDIN: > + die(errno == ERANGE > + ? _("bad numeric config value '%s' for '%s' in standard input %s: out of range") > + : _("bad numeric config value '%s' for '%s' in standard input %s: invalid unit"), > + value, name, cf->name); that does not have any useful cf->name can be improved even more, no? The only reason why we pass an empty string here: > static int git_config_from_stdin(config_fn_t fn, void *data) > { > - return do_config_from_file(fn, "standard input", "", NULL, stdin, data); > + return do_config_from_file(fn, CONFIG_ORIGIN_STDIN, "", NULL, stdin, data); > } is because do_config_from_file() function whose primary purpose is to read from file was reused, the interface to that function requires the filename there for reporting, and we do not have anything useful when reading from the standard input. ``` ## Junio C Hamano, 2016-07-27 22:41 Subject: Re: [PATCH] i18n: config: unfold error messages marked for translation Message-ID: URL: https://gitlist.dev/e/xmqqvazqu2ke.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1469642375-27305-1-git-send-email-vascomalmeida@sapo.pt> ``` Vasco Almeida writes: > + default: > + error_msg = xstrfmt(_("bad config line %d in %s"), > + cf->linenr, cf->name); > + } > + > if (cf->die_on_error) > - die(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name); > + die(error_msg); As error_msg is a result of xstrfmt() and there is no further interpolation needed, you would want to say die("%s", error_msg); here. It triggers config.c:541:3: error: format not a string literal and no format arguments [-Werror=format-security] Similarly for error() below. > else > - return error(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name); > + error_return = error(error_msg); ``` ## Vasco Almeida, 2016-07-28 10:38 Subject: Re: [PATCH] i18n: config: unfold error messages marked for translation Message-ID: <1469702302.1884.8.camel@sapo.pt> URL: https://gitlist.dev/e/1469702302.1884.8.camel%40sapo.pt In-Reply-To: ``` A Qua, 27-07-2016 às 13:00 -0700, Junio C Hamano escreveu: > Vasco Almeida writes: > > > > > + > > + switch (cf->origin_type) { > > + case CONFIG_ORIGIN_BLOB: > > + error_msg = xstrfmt(_("bad config line %d in blob > > %s"), > > +       cf->linenr, cf->name); > > + break; > > + case CONFIG_ORIGIN_FILE: > > + error_msg = xstrfmt(_("bad config line %d in file > > %s"), > > +       cf->linenr, cf->name); > > + break; > > Hmm, if we were to switch the message based on the origin type, then > things like this: > > > > > + case CONFIG_ORIGIN_STDIN: > > + error_msg = xstrfmt(_("bad config line %d in > > standard input %s"), > > +       cf->linenr, cf->name); > > + break; > > and this: > > > > > + case CONFIG_ORIGIN_STDIN: > > + die(errno == ERANGE > > +     ? _("bad numeric config value '%s' for '%s' in > > standard input %s: out of range") > > +     : _("bad numeric config value '%s' for '%s' in > > standard input %s: invalid unit"), > > +     value, name, cf->name); > > that does not have any useful cf->name can be improved even more, no? > > The only reason why we pass an empty string here: > > > > >  static int git_config_from_stdin(config_fn_t fn, void *data) > >  { > > - return do_config_from_file(fn, "standard input", "", NULL, > > stdin, data); > > + return do_config_from_file(fn, CONFIG_ORIGIN_STDIN, "", > > NULL, stdin, data); > >  } > > is because do_config_from_file() function whose primary purpose is > to read from file was reused, the interface to that function > requires the filename there for reporting, and we do not have > anything useful when reading from the standard input. Probably that is why I have omitted cf->name for CONFIG_ORIGIN_STDIN instance first time I sent this patch [1]. This patch was dropped from previous i18n batch because it conflicted with other changes in flux. Forgot about that and, for no good reason, added cf->name. Thank you for pointing out. [1] http://www.mail-archive.com/git%40vger.kernel.org/msg96368.html ``` ## Vasco Almeida, 2016-07-28 13:14 Subject: [PATCH v2] i18n: config: unfold error messages marked for translation Message-ID: <1469711643-10367-1-git-send-email-vascomalmeida@sapo.pt> URL: https://gitlist.dev/e/1469711643-10367-1-git-send-email-vascomalmeida%40sapo.pt In-Reply-To: <1469642375-27305-1-git-send-email-vascomalmeida@sapo.pt> ``` Introduced in 473166b ("config: add 'origin_type' to config_source struct", 2016-02-19), Git can inform the user about the origin of a config error, but the implementation does not allow translators to translate the keywords 'file', 'blob, 'standard input', and 'submodule-blob'. Moreover, for the second message, a reason for the error is appended to the message, not allowing translators to translate that reason either. Unfold the message into several templates for each known origin_type. That would result in better translation at the expense of code verbosity. Add enum config_oringin_type to ease management of the various configuration origin types (blob, file, etc). Previously origin type was considered from command line if cf->origin_type == NULL, i.e., uninitialized. Now we set origin_type to CONFIG_ORIGIN_CMDLINE in git_config_from_parameters() and configset_add_value(). For error message in git_parse_source(), use xstrfmt() function to prepare the message string, instead of doing something like it's done for die_bad_number(), because intelligibility and code conciseness are improved for that instance. Signed-off-by: Vasco Almeida --- cache.h | 12 +++++- config.c | 117 ++++++++++++++++++++++++++++++++++++++++++++--------- submodule-config.c | 2 +- 3 files changed, 109 insertions(+), 22 deletions(-) diff --git a/cache.h b/cache.h index 3855ddf..c802098 100644 --- a/cache.h +++ b/cache.h @@ -1566,10 +1566,18 @@ struct git_config_source { const char *blob; }; +enum config_origin_type { + CONFIG_ORIGIN_BLOB, + CONFIG_ORIGIN_FILE, + CONFIG_ORIGIN_STDIN, + CONFIG_ORIGIN_SUBMODULE_BLOB, + CONFIG_ORIGIN_CMDLINE +}; + 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_mem(config_fn_t fn, const char *origin_type, +extern int git_config_from_mem(config_fn_t fn, const enum config_origin_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); @@ -1713,7 +1721,7 @@ extern int ignore_untracked_cache_config; struct key_value_info { const char *filename; int linenr; - const char *origin_type; + enum config_origin_type origin_type; enum config_scope scope; }; diff --git a/config.c b/config.c index bea937e..62b83d9 100644 --- a/config.c +++ b/config.c @@ -24,7 +24,7 @@ struct config_source { size_t pos; } buf; } u; - const char *origin_type; + enum config_origin_type origin_type; const char *name; const char *path; int die_on_error; @@ -245,6 +245,7 @@ int git_config_from_parameters(config_fn_t fn, void *data) memset(&source, 0, sizeof(source)); source.prev = cf; + source.origin_type = CONFIG_ORIGIN_CMDLINE; cf = &source; /* sq_dequote will write over it */ @@ -453,6 +454,8 @@ static int git_parse_source(config_fn_t fn, void *data) int comment = 0; int baselen = 0; struct strbuf *var = &cf->var; + int error_return = 0; + char *error_msg = NULL; /* U+FEFF Byte Order Mark in UTF8 */ const char *bomptr = utf8_bom; @@ -507,10 +510,40 @@ static int git_parse_source(config_fn_t fn, void *data) if (get_value(fn, data, var) < 0) break; } + + switch (cf->origin_type) { + case CONFIG_ORIGIN_BLOB: + error_msg = xstrfmt(_("bad config line %d in blob %s"), + cf->linenr, cf->name); + break; + case CONFIG_ORIGIN_FILE: + error_msg = xstrfmt(_("bad config line %d in file %s"), + cf->linenr, cf->name); + break; + case CONFIG_ORIGIN_STDIN: + error_msg = xstrfmt(_("bad config line %d in standard input"), + cf->linenr); + break; + case CONFIG_ORIGIN_SUBMODULE_BLOB: + error_msg = xstrfmt(_("bad config line %d in submodule-blob %s"), + cf->linenr, cf->name); + break; + case CONFIG_ORIGIN_CMDLINE: + error_msg = xstrfmt(_("bad config line %d in command line %s"), + cf->linenr, cf->name); + break; + default: + error_msg = xstrfmt(_("bad config line %d in %s"), + cf->linenr, cf->name); + } + if (cf->die_on_error) - die(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name); + die("%s", error_msg); else - return error(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name); + error_return = error("%s", error_msg); + + free(error_msg); + return error_return; } static int parse_unit_factor(const char *end, uintmax_t *val) @@ -619,16 +652,47 @@ int git_parse_ulong(const char *value, unsigned long *ret) NORETURN static void die_bad_number(const char *name, const char *value) { - const char *reason = errno == ERANGE ? - "out of range" : - "invalid unit"; if (!value) value = ""; - if (cf && cf->origin_type && cf->name) - die(_("bad numeric config value '%s' for '%s' in %s %s: %s"), - value, name, cf->origin_type, cf->name, reason); - die(_("bad numeric config value '%s' for '%s': %s"), value, name, reason); + if (!(cf && cf->name)) + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s': out of range") + : _("bad numeric config value '%s' for '%s': invalid unit"), + value, name); + + switch (cf->origin_type) { + case CONFIG_ORIGIN_BLOB: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in blob %s: out of range") + : _("bad numeric config value '%s' for '%s' in blob %s: invalid unit"), + value, name, cf->name); + case CONFIG_ORIGIN_FILE: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in file %s: out of range") + : _("bad numeric config value '%s' for '%s' in file %s: invalid unit"), + value, name, cf->name); + case CONFIG_ORIGIN_STDIN: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in standard input: out of range") + : _("bad numeric config value '%s' for '%s' in standard input: invalid unit"), + value, name); + case CONFIG_ORIGIN_SUBMODULE_BLOB: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in submodule-blob %s: out of range") + : _("bad numeric config value '%s' for '%s' in submodule-blob %s: invalid unit"), + value, name, cf->name); + case CONFIG_ORIGIN_CMDLINE: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in command line %s: out of range") + : _("bad numeric config value '%s' for '%s' in command line %s: invalid unit"), + value, name, cf->name); + default: + die(errno == ERANGE + ? _("bad numeric config value '%s' for '%s' in %s: out of range") + : _("bad numeric config value '%s' for '%s' in %s: invalid unit"), + value, name, cf->name); + } } int git_config_int(const char *name, const char *value) @@ -1105,7 +1169,8 @@ 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 *origin_type, const char *name, const char *path, FILE *f, + const enum config_origin_type origin_type, + const char *name, const char *path, FILE *f, void *data) { struct config_source top; @@ -1124,7 +1189,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, "standard input", "", NULL, stdin, data); + return do_config_from_file(fn, CONFIG_ORIGIN_STDIN, "", NULL, stdin, data); } int git_config_from_file(config_fn_t fn, const char *filename, void *data) @@ -1135,14 +1200,14 @@ 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, "file", filename, filename, f, data); + ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename, filename, f, data); funlockfile(f); fclose(f); } return ret; } -int git_config_from_mem(config_fn_t fn, const char *origin_type, +int git_config_from_mem(config_fn_t fn, const enum config_origin_type origin_type, const char *name, const char *buf, size_t len, void *data) { struct config_source top; @@ -1179,7 +1244,7 @@ static int git_config_from_blob_sha1(config_fn_t fn, return error("reference '%s' does not point to a blob", name); } - ret = git_config_from_mem(fn, "blob", name, buf, size, data); + ret = git_config_from_mem(fn, CONFIG_ORIGIN_BLOB, name, buf, size, data); free(buf); return ret; @@ -1390,12 +1455,12 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha if (cf->name) { kv_info->filename = strintern(cf->name); kv_info->linenr = cf->linenr; - kv_info->origin_type = strintern(cf->origin_type); + kv_info->origin_type = cf->origin_type; } else { /* for values read from `git_config_from_parameters()` */ kv_info->filename = NULL; kv_info->linenr = -1; - kv_info->origin_type = NULL; + kv_info->origin_type = CONFIG_ORIGIN_CMDLINE; } kv_info->scope = current_parsing_scope; si->util = kv_info; @@ -2476,14 +2541,28 @@ int parse_config_key(const char *var, const char *current_config_origin_type(void) { - const char *type; + int type; if (current_config_kvi) type = current_config_kvi->origin_type; else if(cf) type = cf->origin_type; else die("BUG: current_config_origin_type called outside config callback"); - return type ? type : "command line"; + + switch (type) { + case CONFIG_ORIGIN_BLOB: + return "blob"; + case CONFIG_ORIGIN_FILE: + return "file"; + case CONFIG_ORIGIN_STDIN: + return "standard input"; + case CONFIG_ORIGIN_SUBMODULE_BLOB: + return "submodule-blob"; + case CONFIG_ORIGIN_CMDLINE: + return "command line"; + default: + die("BUG: unknown config origin type"); + } } const char *current_config_name(void) diff --git a/submodule-config.c b/submodule-config.c index 077db40..1753e01 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -448,7 +448,7 @@ static const struct submodule *config_from(struct submodule_cache *cache, parameter.commit_sha1 = commit_sha1; parameter.gitmodules_sha1 = sha1; parameter.overwrite = 0; - git_config_from_mem(parse_config, "submodule-blob", rev.buf, + git_config_from_mem(parse_config, CONFIG_ORIGIN_SUBMODULE_BLOB, rev.buf, config, config_size, ¶meter); free(config); -- 2.7.4 ``` ## Junio C Hamano, 2016-07-28 16:12 Subject: Re: [PATCH v2] i18n: config: unfold error messages marked for translation Message-ID: URL: https://gitlist.dev/e/xmqqwpk5spvy.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1469711643-10367-1-git-send-email-vascomalmeida@sapo.pt> ``` Vasco Almeida writes: > Unfold the message into several templates for each known origin_type. > That would result in better translation at the expense of code > verbosity. Looks good now, except one minor nit I'll tweak out while queuing. > if (cf->die_on_error) > - die(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name); > + die("%s", error_msg); > else > - return error(_("bad config line %d in %s %s"), cf->linenr, cf->origin_type, cf->name); > + error_return = error("%s", error_msg); s/ = / = /; Thanks. ``` ## Jeff King, 2016-07-28 16:17 Subject: Re: [PATCH v2] i18n: config: unfold error messages marked for translation Message-ID: <20160728161748.GA18515@sigill.intra.peff.net> URL: https://gitlist.dev/e/20160728161748.GA18515%40sigill.intra.peff.net In-Reply-To: <1469711643-10367-1-git-send-email-vascomalmeida@sapo.pt> ``` On Thu, Jul 28, 2016 at 01:14:03PM +0000, Vasco Almeida wrote: > static void die_bad_number(const char *name, const char *value) > { > - const char *reason = errno == ERANGE ? > - "out of range" : > - "invalid unit"; > if (!value) > value = ""; > > - if (cf && cf->origin_type && cf->name) > - die(_("bad numeric config value '%s' for '%s' in %s %s: %s"), > - value, name, cf->origin_type, cf->name, reason); > - die(_("bad numeric config value '%s' for '%s': %s"), value, name, reason); > + if (!(cf && cf->name)) > + die(errno == ERANGE > + ? _("bad numeric config value '%s' for '%s': out of range") > + : _("bad numeric config value '%s' for '%s': invalid unit"), > + value, name); > + > + switch (cf->origin_type) { > + case CONFIG_ORIGIN_BLOB: > + die(errno == ERANGE > + ? _("bad numeric config value '%s' for '%s' in blob %s: out of range") > + : _("bad numeric config value '%s' for '%s' in blob %s: invalid unit"), > + value, name, cf->name); I get that the point of this patch is to make more complete sentences for translation, rather than the lego-brick construction. And that makes sense to me. But it seems like the ":" is a natural separator, and it would be OK to make these: die(_("bad numeric config value '%s' for '%s' in blob %s: %s"), value, name, cf->name, reason); instead of having two separate strings for the errno values. After all, that's what we do everywhere else where "reason" is supplied by strerror(). It's just in this case that there is no errno value matching "invalid unit", so we have to fill it in ourselves. -Peff ```