Hi Phillip, On Fri, 18 Jan 2019, Phillip Wood wrote: > On 18/01/2019 12:19, Slavica Đukić wrote: > > > > On 18-Jan-19 12:20 PM, Phillip Wood wrote: > > > > > > I think this round is looking good I've got a couple of comments > > > about the translation of the help text but everything else looks > > > fine to me now. In future when you're posting a new version it's > > > helpful CC the people who commented on the previous version(s). > > > > > > Thanks for taking your time to review patches again. I'm sorry for > > omitting you > > > > in CC, but I've sent re-roll through GitGitGadget, and I guess I > > thought it would pick it up. > > > > I'll see what happened and keep that in mind. > > I'm not sure what GitGitGadget does about CC'ing people but Johannes > will know The idea is to have a footer (read: last line) of the PR description of the form: Cc: Name , Other i.e. a comma-separated list of recipients to Cc. Yes, it is under-documented, but I still need to implement more features before I can start to polish documentation. Ciao, Dscho > > > > On 18/01/2019 07:47, Slavica Djukic via GitGitGadget wrote: > > > > From: Slavica Djukic > > > > > > > > Implement show-help command in add-interactive.c and use it in > > > > builtin add--helper.c. > > > > > > > > Use command name "show-help" instead of "help": add--helper is > > > > builtin, hence add--helper --help would be intercepted by > > > > handle_builtin and re-routed to the help command, without ever > > > > calling cmd_add__helper(). > > > > > > > > Signed-off-by: Slavica Djukic > > > > --- > > > > add-interactive.c | 23 +++++++++++++++++++++++ > > > > add-interactive.h | 4 +++- > > > > builtin/add--helper.c | 7 ++++++- > > > > 3 files changed, 32 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/add-interactive.c b/add-interactive.c > > > > index c55d934186..76c3f4c3eb 100644 > > > > --- a/add-interactive.c > > > > +++ b/add-interactive.c > > > > @@ -244,3 +244,26 @@ void add_i_print_modified(void) > > > > free(files); > > > > hashmap_free(&s.file_map, 1); > > > > } > > > > + > > > > +void add_i_show_help(void) > > > > +{ > > > > + const char *help_color = get_color(COLOR_HELP); > > > > + color_fprintf(stdout, help_color, "%s%s", _("status"), > > > > + N_(" - show paths with changes")); > > > > + printf("\n"); > > > There seems to be a bit of confusion with the translation of these > > > messages. "status" does not want to be translated so it shouldn't be in > > > _() - it can just go in the format string as can the indentation and the > > > "\n" (or we could use color_fprintf_ln() to automatically add a newline > > > at the end. N_() is used to mark static strings for translation so the > > > gettext utilities pick up the text to be translated but (because > > > initializes for static variables must be compile-time constants) does > > > not do anything when the program runs - if you have 'const char *s = > > > N_(hello);' you have to do '_(s)' to get the translated version. Here we > > > can just pass the untranslated string directly to gettext so it should > > > be _("show paths with changes"). Putting all that together we get > > > > > > color_fprintf(stdout, help_color, "status - %s\n", > > > _("show paths with changes"); > > > > > > I thought _() was for strings that were already translated, > > and N_() for strings that weren't. And I now see that I also tried to > > translate command names as well, just the opposite of what you suggested... > > Thanks for clarifying this. > > I hope my explanation made sense, feel free to email if you want to check > anything. > > Having thought about it, I don't think we should add "\n" to the format string > as it means the color will be reset after the new line, it should use > color_fprintf_ln() instead which adds a new line after it has reset the color. > > Best Wishes > > Phillip > > > > Best Wishes > > > > > > Phillip > > > > > > > + color_fprintf(stdout, help_color, "%s%s", _("update"), > > > > + N_(" - add working tree state to the staged set of > > > > changes")); > > > > + printf("\n"); > > > > + color_fprintf(stdout, help_color, "%s%s", _("revert"), > > > > + N_(" - revert staged set of changes back to the HEAD > > > > version")); > > > > + printf("\n"); > > > > + color_fprintf(stdout, help_color, "%s%s", _("patch"), > > > > + N_(" - pick hunks and update selectively")); > > > > + printf("\n"); > > > > + color_fprintf(stdout, help_color, "%s%s", _("diff"), > > > > + N_(" - view diff between HEAD and index")); > > > > + printf("\n"); > > > > + color_fprintf(stdout, help_color, "%s%s", _("add untracked"), > > > > + N_(" - add contents of untracked files to the staged set of > > > > changes")); > > > > + printf("\n"); > > > > +} > > > > diff --git a/add-interactive.h b/add-interactive.h > > > > index 1f4747553c..46e17c5c71 100644 > > > > --- a/add-interactive.h > > > > +++ b/add-interactive.h > > > > @@ -5,4 +5,6 @@ int add_i_config(const char *var, const char *value, > > > > void *cbdata); > > > > > > > > void add_i_print_modified(void); > > > > > > > > -#endif > > > > \ No newline at end of file > > > > +void add_i_show_help(void); > > > > + > > > > +#endif > > > > diff --git a/builtin/add--helper.c b/builtin/add--helper.c > > > > index 43545d9af5..a3b3a68b68 100644 > > > > --- a/builtin/add--helper.c > > > > +++ b/builtin/add--helper.c > > > > @@ -10,7 +10,8 @@ static const char * const builtin_add_helper_usage[] = > > > > { > > > > > > > > enum cmd_mode { > > > > DEFAULT = 0, > > > > - STATUS > > > > + STATUS, > > > > + HELP > > > > }; > > > > > > > > int cmd_add__helper(int argc, const char **argv, const char *prefix) > > > > @@ -20,6 +21,8 @@ int cmd_add__helper(int argc, const char **argv, const > > > > char *prefix) > > > > struct option options[] = { > > > > OPT_CMDMODE(0, "status", &mode, > > > > N_("print status information with diffstat"), > > > > STATUS), > > > > + OPT_CMDMODE(0, "show-help", &mode, > > > > + N_("show help"), HELP), > > > > OPT_END() > > > > }; > > > > @@ -30,6 +33,8 @@ int cmd_add__helper(int argc, const char **argv, > > > > const char *prefix) > > > > > > > > if (mode == STATUS) > > > > add_i_print_modified(); > > > > + else if (mode == HELP) > > > > + add_i_show_help(); > > > > else > > > > usage_with_options(builtin_add_helper_usage, > > > > options); > > > > > > >