parse-options: replace opterror() with optname()
Introduce optname() that does the early half of original opterror() to come up with the name of the option reported back to the user, and use it to kill opterror(). The callers of opterror() now directly call error() using the string returned by opterror() instead. There are a few issues with opterror() - it tries to assemble an English sentence from pieces. This is not great for translators because we give them pieces instead of a full sentence. - It's a wrapper around error() and needs some hack to let the compiler know it always returns -1. - Since it takes a string instead of printf format, one call site has to assemble the string manually before passing to it. Using error() directly solves the second and third problems. It kind helps the first problem as well because "%s does foo" does give a translator a full sentence in a sense and let them reorder if needed. But it has limitations, if the subject part has to change based on the rest of the sentence, that language is screwed. This is also why I try to avoid calling optname() when 'flags' is known in advance. Mark of these strings for translation as well while at there. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Nguyễn Thái Ngọc Duy committed
Nov 10, 2018 at 06:16 UTC
9440b831ad5b1750b65fa3f1d0b99179ab317c76
7 files changed
+40
-33
builtin/merge.c
+1
-1
@@ -128,7 +128,7 @@ static int option_read_message(struct parse_opt_ctx_t *ctx,
128
ctx->argc--;
129
arg = *++ctx->argv;
130
} else
131
- return opterror(opt, "requires a value", 0);
131
+ return error(_("option `%s' requires a value"), opt->long_name);
132
133
if (buf->len)
134
strbuf_addch(buf, '\n');
builtin/revert.c
+2
-1
@@ -69,7 +69,8 @@ static int option_parse_m(const struct option *opt,
69
70
replay->mainline = strtol(arg, &end, 10);
71
if (*end || replay->mainline <= 0)
72
- return opterror(opt, "expects a number greater than zero", 0);
72
+ return error(_("option `%s' expects a number greater than zero"),
73
+ opt->long_name);
74
75
return 0;
76
}
parse-options-cb.c
+4
-3
@@ -18,7 +18,8 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
18
} else {
19
v = strtol(arg, (char **)&arg, 10);
20
if (*arg)
21
- return opterror(opt, "expects a numerical value", 0);
21
+ return error(_("option `%s' expects a numerical value"),
22
+ opt->long_name);
23
if (v && v < MINIMUM_ABBREV)
24
v = MINIMUM_ABBREV;
25
else if (v > 40)
@@ -54,8 +55,8 @@ int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
55
arg = unset ? "never" : (const char *)opt->defval;
56
value = git_config_colorbool(NULL, arg);
57
if (value < 0)
57
- return opterror(opt,
58
- "expects \"always\", \"auto\", or \"never\"", 0);
58
+ return error(_("option `%s' expects \"always\", \"auto\", or \"never\""),
59
+ opt->long_name);
60
*(int *)opt->value = value;
61
return 0;
62
}
parse-options.c
+26
-20
@@ -32,7 +32,7 @@ static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
32
p->argc--;
33
*arg = *++p->argv;
34
} else
35
- return opterror(opt, "requires a value", flags);
35
+ return error(_("%s requires a value"), optname(opt, flags));
36
return 0;
37
}
38
@@ -49,7 +49,6 @@ static int opt_command_mode_error(const struct option *opt,
49
int flags)
50
{
51
const struct option *that;
52
- struct strbuf message = STRBUF_INIT;
52
struct strbuf that_name = STRBUF_INIT;
53
54
/*
@@ -67,13 +66,13 @@ static int opt_command_mode_error(const struct option *opt,
66
strbuf_addf(&that_name, "--%s", that->long_name);
67
else
68
strbuf_addf(&that_name, "-%c", that->short_name);
70
- strbuf_addf(&message, ": incompatible with %s", that_name.buf);
69
+ error(_("%s is incompatible with %s"),
70
+ optname(opt, flags), that_name.buf);
71
strbuf_release(&that_name);
72
- opterror(opt, message.buf, flags);
73
- strbuf_release(&message);
72
return -1;
73
}
76
- return opterror(opt, ": incompatible with something else", flags);
74
+ return error(_("%s : incompatible with something else"),
75
+ optname(opt, flags));
76
}
77
78
static int get_value(struct parse_opt_ctx_t *p,
@@ -86,11 +85,11 @@ static int get_value(struct parse_opt_ctx_t *p,
85
int err;
86
87
if (unset && p->opt)
89
- return opterror(opt, "takes no value", flags);
88
+ return error(_("%s takes no value"), optname(opt, flags));
89
if (unset && (opt->flags & PARSE_OPT_NONEG))
91
- return opterror(opt, "isn't available", flags);
90
+ return error(_("%s isn't available"), optname(opt, flags));
91
if (!(flags & OPT_SHORT) && p->opt && (opt->flags & PARSE_OPT_NOARG))
93
- return opterror(opt, "takes no value", flags);
92
+ return error(_("%s takes no value"), optname(opt, flags));
93
94
switch (opt->type) {
95
case OPTION_LOWLEVEL_CALLBACK:
@@ -176,7 +175,8 @@ static int get_value(struct parse_opt_ctx_t *p,
175
return -1;
176
*(int *)opt->value = strtol(arg, (char **)&s, 10);
177
if (*s)
179
- return opterror(opt, "expects a numerical value", flags);
178
+ return error(_("%s expects a numerical value"),
179
+ optname(opt, flags));
180
return 0;
181
182
case OPTION_MAGNITUDE:
@@ -191,9 +191,9 @@ static int get_value(struct parse_opt_ctx_t *p,
191
if (get_arg(p, opt, flags, &arg))
192
return -1;
193
if (!git_parse_ulong(arg, opt->value))
194
- return opterror(opt,
195
- "expects a non-negative integer value with an optional k/m/g suffix",
196
- flags);
194
+ return error(_("%s expects a non-negative integer value"
195
+ " with an optional k/m/g suffix"),
196
+ optname(opt, flags));
197
return 0;
198
199
default:
@@ -257,7 +257,8 @@ again:
257
if (!rest)
258
continue;
259
if (*rest == '=')
260
- return opterror(options, "takes no value", flags);
260
+ return error(_("%s takes no value"),
261
+ optname(options, flags));
262
if (*rest)
263
continue;
264
p->out[p->cpidx++] = arg - 2;
@@ -773,12 +774,17 @@ void NORETURN usage_msg_opt(const char *msg,
774
usage_with_options(usagestr, options);
775
}
776
776
-#undef opterror
777
-int opterror(const struct option *opt, const char *reason, int flags)
777
+const char *optname(const struct option *opt, int flags)
778
{
779
+ static struct strbuf sb = STRBUF_INIT;
780
+
781
+ strbuf_reset(&sb);
782
if (flags & OPT_SHORT)
780
- return error("switch `%c' %s", opt->short_name, reason);
781
- if (flags & OPT_UNSET)
782
- return error("option `no-%s' %s", opt->long_name, reason);
783
- return error("option `%s' %s", opt->long_name, reason);
783
+ strbuf_addf(&sb, "switch `%c'", opt->short_name);
784
+ else if (flags & OPT_UNSET)
785
+ strbuf_addf(&sb, "option `no-%s'", opt->long_name);
786
+ else
787
+ strbuf_addf(&sb, "option `%s'", opt->long_name);
788
+
789
+ return sb.buf;
790
}
parse-options.h
+1
-4
@@ -189,10 +189,7 @@ extern NORETURN void usage_msg_opt(const char *msg,
189
const struct option *options);
190
191
extern int optbug(const struct option *opt, const char *reason);
192
-extern int opterror(const struct option *opt, const char *reason, int flags);
193
-#if defined(__GNUC__)
194
-#define opterror(o,r,f) (opterror((o),(r),(f)), const_error())
195
-#endif
192
+const char *optname(const struct option *opt, int flags);
193
194
/*----- incremental advanced APIs -----*/
195
ref-filter.c
+5
-3
@@ -2302,9 +2302,11 @@ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)
2302
2303
if (rf->merge) {
2304
if (no_merged) {
2305
- return opterror(opt, "is incompatible with --merged", 0);
2305
+ return error(_("option `%s' is incompatible with --merged"),
2306
+ opt->long_name);
2307
} else {
2307
- return opterror(opt, "is incompatible with --no-merged", 0);
2308
+ return error(_("option `%s' is incompatible with --no-merged"),
2309
+ opt->long_name);
2310
}
2311
}
2312
@@ -2318,7 +2320,7 @@ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)
2320
rf->merge_commit = lookup_commit_reference_gently(the_repository,
2321
&oid, 0);
2322
if (!rf->merge_commit)
2321
- return opterror(opt, "must point to a commit", 0);
2323
+ return error(_("option `%s' must point to a commit"), opt->long_name);
2324
2325
return 0;
2326
}
t/t4211-line-log.sh
+1
-1
@@ -25,7 +25,7 @@ canned_test_failure () {
25
test_bad_opts () {
26
test_expect_success "invalid args: $1" "
27
test_must_fail git log $1 2>errors &&
28
- grep '$2' errors
28
+ test_i18ngrep '$2' errors
29
"
30
}
31