| Message ID | 20230921110727.789156-1-oswald.buddenhagen@gmx.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 43abaaf0087c72fccc5bbf3fc103eb148465480d |
| Headers | show |
| Series | am: fix error message in parse_opt_show_current_patch() | expand |
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes: > The argument order was incorrect. This was introduced by 246cac8505 > (i18n: turn even more messages into "cannot be used together" ones, > 2022-01-05). > > Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> > > --- > fwiw, this is currently the only message that actually uses the %s=%s > format, so as of now, factoring out the argument names has only > theoretical value. I am not sure I follow, if you mean that the programmer needs to pass "--show-current-patch" only once if we used something like "%1$s=%2s and %1$s=%3s", I agree that it probably has little value. The patch looks good. It seems that we are seeing a crack in test coverage, by the way? Will queue. Thanks. > diff --git a/builtin/am.c b/builtin/am.c > index 202040b62e..6655059a57 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -2303,7 +2303,8 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar > if (resume->mode == RESUME_SHOW_PATCH && new_value != resume->sub_mode) > return error(_("options '%s=%s' and '%s=%s' " > "cannot be used together"), > - "--show-current-patch", "--show-current-patch", arg, valid_modes[resume->sub_mode]); > + "--show-current-patch", arg, > + "--show-current-patch", valid_modes[resume->sub_mode]); > > resume->mode = RESUME_SHOW_PATCH; > resume->sub_mode = new_value;
On Thu, Sep 21, 2023 at 12:09:10PM -0700, Junio C Hamano wrote: >Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes: >> fwiw, this is currently the only message that actually uses the %s=%s >> format, so as of now, factoring out the argument names has only >> theoretical value. > >I am not sure I follow, if you mean that the programmer needs to >pass "--show-current-patch" only once if we used something like >"%1$s=%2s and %1$s=%3s", I agree that it probably has little value. > no, i mean that that the usual pattern is just "options '%s' and '%s' cannot be used together". this format string is indeed used many times, so it makes sense to factor out the option names to avoid duplication of translatable strings. not so here. but this particular case is still a lot less specialized than many of the other strings replaced by the referenced patch, and it's at least plausible that further uses would be added at some point, so i left it as-is. i thought about the duplication of the option string as well, but compilers should merge the string constants, so that part is indeed de-duplicated. the cost of the extra pointer push could be avoided by use of the %1$s syntax, but afaict that's unprecedented in git, and i kind of expect that some printf implementation would throw up from it. also, it might reduce the chance of the format being used in another place, but who knows. regards
diff --git a/builtin/am.c b/builtin/am.c index 202040b62e..6655059a57 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2303,7 +2303,8 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar if (resume->mode == RESUME_SHOW_PATCH && new_value != resume->sub_mode) return error(_("options '%s=%s' and '%s=%s' " "cannot be used together"), - "--show-current-patch", "--show-current-patch", arg, valid_modes[resume->sub_mode]); + "--show-current-patch", arg, + "--show-current-patch", valid_modes[resume->sub_mode]); resume->mode = RESUME_SHOW_PATCH; resume->sub_mode = new_value;
The argument order was incorrect. This was introduced by 246cac8505 (i18n: turn even more messages into "cannot be used together" ones, 2022-01-05). Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> --- fwiw, this is currently the only message that actually uses the %s=%s format, so as of now, factoring out the argument names has only theoretical value. Cc: Jean-Noël Avila <jn.avila@free.fr> Cc: Johannes Sixt <j6t@kdbg.org> Cc: René Scharfe <l.s.r@web.de> Cc: Junio C Hamano <gitster@pobox.com> --- builtin/am.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)