Hi Slavica On 18/01/2019 12:19, Slavica Đukić wrote:
Hi Phillip, On 18-Jan-19 12:20 PM, Phillip Wood wrote:Hi Slavica 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
On 18/01/2019 07:47, Slavica Djukic via GitGitGadget wrote:From: Slavica Djukic <slawica92@xxxxxxxxxxx> 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 <slawica92@xxxxxxxxxxx> --- 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);
