Re: [PATCH v12 11/13] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
Hey Junio, On Sat, Aug 13, 2016 at 12:19 AM, Junio C Hamano wrote: > Junio C Hamano writes: > >> Pranit Bauva writes: >> >>> +static int bisect_next_check(const struct bisect_terms *terms, >>> + const char *current_term) >>> +{ >>> + >>> +fprintf(stderr, N_("Warning: bisecting only with a %s >>> commit\n"), >>> +terms->term_bad.buf); >> >> Hmph, is this N_() and not _()? > > I recall you saying that you are not familiar with i18n. As it is a > good skill to have outside the context of this project, let's do a > quick primer. GSoC is a learning experience ;-). True! :) > There is a runtime function "gettext()" that takes a string, which > is typically a printf format string, and gives another string. You > feed your message in the original language, meant to be used in the > C locale, and the function gives it translated into the end-user's > language, specified by environment variables like $LC_MESSAGES. > > Since it is too cumbersome to write gettext() all the time, _() > exists as a short-hand for it. So running this > > printf(_("Hello, world\n")); > > would look up "Hello world\n" in database for the end-user's > language, and shows the translated message instead. > > By scanning the source text, you can extract these constant strings > passed to gettext() or _(). This is done by the i18n coordinator > with the "msgmerge" program. By doing so, we learn "Hello, world\n" > must be translated, and then ask i18n team to translate it to their > language. The result is what we have in the l10n database. They > are in po/*.po files in our source tree. > > Sometimes, the message you want to be translated may be in a > variable, e.g. > > void greeting(const char *message) > { > printf(_(message)); > } > > int main(int ac, char **av) > { > int i; > const char *message[] = { > "Hello, world\n", > "Goodbye, everybody\n", > }; > for (i = 0; i < ARRAY_SIZE(message); i++) > greeting(message[i]); > } > > And scanning the source would not find "Hello, world\n" or "Goodby, > everybody\n" must be translated. We do not want to do this: > > ... > const char *message[] = { > *BAD* _("Hello, world\n"), > *BAD* _("Goodbye, everybody\n"), > }; > ... > > because we do *NOT* want to call gettext() there (we call the > function at runtime inside greeting() instead). We use N_() > to mark such messages, like so: > > ... > const char *message[] = { > *GOOD* N_("Hello, world\n"), > *GOOD* N_("Goodbye, everybody\n"), > }; > ... > > The N_() macro is a no-op at runtime. It just is there so that > "msgmerge" can notice that the constant string there is something > that needs to be thrown into the l10n database. > > As a concrete example: > >> @@ -24,6 +25,7 @@ static const char * const git_bisect_helper_usage[] = { >> N_("git bisect--helper --bisect-reset []"), >> N_("git bisect--helper --bisect-write >> []"), >> N_("git bisect--helper --bisect-check-and-set-terms >> "), >> + N_("git bisect--helper --bisect-next-check [] >> > NULL >> }; > > this is such a use of N_(). You want to keep untranslated message > in the git_bisect_helper_usage[] array, to be given to gettext(), or > more likely its short-hand _(), when these usage strings are used, > and make sure these strings will be in the l10n database so that > translators will give you translations to them to be used at > runtime. > >> + /* >> + * have bad (or new) but not good (or old). We could bisect >> + * although this is less optimum. >> + */ >> + fprintf(stderr, N_("Warning: bisecting only with a %s >> commit\n"), >> + terms->term_bad.buf); > > This one wants to call gettext() function to give fprintf() the > translated warning message. It should be _(). > >> + /* >> + * TRANSLATORS: Make sure to include [Y] and [n] in your >> + * translation. The program will only accept English input >> + * at this point. >> + */ >> + yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO); > > Just like this one. Thanks a lot for taking time to explain this. I had searched a bit but couldn't find the difference between _() and N_()! Regards, Pranit Bauva -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 11/13] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
Junio C Hamano writes: > Pranit Bauva writes: > >> +static int bisect_next_check(const struct bisect_terms *terms, >> + const char *current_term) >> +{ >> + >> +fprintf(stderr, N_("Warning: bisecting only with a %s >> commit\n"), >> +terms->term_bad.buf); > > Hmph, is this N_() and not _()? I recall you saying that you are not familiar with i18n. As it is a good skill to have outside the context of this project, let's do a quick primer. GSoC is a learning experience ;-). There is a runtime function "gettext()" that takes a string, which is typically a printf format string, and gives another string. You feed your message in the original language, meant to be used in the C locale, and the function gives it translated into the end-user's language, specified by environment variables like $LC_MESSAGES. Since it is too cumbersome to write gettext() all the time, _() exists as a short-hand for it. So running this printf(_("Hello, world\n")); would look up "Hello world\n" in database for the end-user's language, and shows the translated message instead. By scanning the source text, you can extract these constant strings passed to gettext() or _(). This is done by the i18n coordinator with the "msgmerge" program. By doing so, we learn "Hello, world\n" must be translated, and then ask i18n team to translate it to their language. The result is what we have in the l10n database. They are in po/*.po files in our source tree. Sometimes, the message you want to be translated may be in a variable, e.g. void greeting(const char *message) { printf(_(message)); } int main(int ac, char **av) { int i; const char *message[] = { "Hello, world\n", "Goodbye, everybody\n", }; for (i = 0; i < ARRAY_SIZE(message); i++) greeting(message[i]); } And scanning the source would not find "Hello, world\n" or "Goodby, everybody\n" must be translated. We do not want to do this: ... const char *message[] = { *BAD* _("Hello, world\n"), *BAD* _("Goodbye, everybody\n"), }; ... because we do *NOT* want to call gettext() there (we call the function at runtime inside greeting() instead). We use N_() to mark such messages, like so: ... const char *message[] = { *GOOD* N_("Hello, world\n"), *GOOD* N_("Goodbye, everybody\n"), }; ... The N_() macro is a no-op at runtime. It just is there so that "msgmerge" can notice that the constant string there is something that needs to be thrown into the l10n database. As a concrete example: > @@ -24,6 +25,7 @@ static const char * const git_bisect_helper_usage[] = { > N_("git bisect--helper --bisect-reset []"), > N_("git bisect--helper --bisect-write > []"), > N_("git bisect--helper --bisect-check-and-set-terms > "), > + N_("git bisect--helper --bisect-next-check [] >NULL > }; this is such a use of N_(). You want to keep untranslated message in the git_bisect_helper_usage[] array, to be given to gettext(), or more likely its short-hand _(), when these usage strings are used, and make sure these strings will be in the l10n database so that translators will give you translations to them to be used at runtime. > + /* > + * have bad (or new) but not good (or old). We could bisect > + * although this is less optimum. > + */ > + fprintf(stderr, N_("Warning: bisecting only with a %s > commit\n"), > + terms->term_bad.buf); This one wants to call gettext() function to give fprintf() the translated warning message. It should be _(). > + /* > + * TRANSLATORS: Make sure to include [Y] and [n] in your > + * translation. The program will only accept English input > + * at this point. > + */ > + yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO); Just like this one. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 11/13] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
Pranit Bauva writes: > +static char *bisect_voc(char *revision_type) > +{ > + if (!strcmp(revision_type, "bad")) > + return "bad|new"; > + if (!strcmp(revision_type, "good")) > + return "good|old"; > + > + return NULL; > +} I think you can return "const char *" from the above function. Then you do not have to do xstrdup() on the return values to store in bad_syn and good_syn, and you do not have to free(3) them. > +static int bisect_next_check(const struct bisect_terms *terms, > + const char *current_term) > +{ > + > + fprintf(stderr, N_("Warning: bisecting only with a %s > commit\n"), > + terms->term_bad.buf); Hmph, is this N_() and not _()? > + > + } > + bad_syn = xstrdup(bisect_voc("bad")); > + good_syn = xstrdup(bisect_voc("good")); > + > + free(bad_syn); > + free(good_syn); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 11/13] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
Reimplement `bisect_next_check` shell function in C and add `bisect-next-check` subcommand to `git bisect--helper` to call it from git-bisect.sh . Also reimplement `bisect_voc` shell function in C and call it from `bisect_next_check` implementation in C. Using `--bisect-next-check` is a temporary measure to port shell function to C so as to use the existing test suite. As more functions are ported, this subcommand will be retired but its implementation will be called by some other methods. Mentored-by: Lars Schneider Mentored-by: Christian Couder Signed-off-by: Pranit Bauva --- builtin/bisect--helper.c | 103 ++- git-bisect.sh| 60 ++- 2 files changed, 106 insertions(+), 57 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 5c4350f..b6e9973 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -6,6 +6,7 @@ #include "dir.h" #include "argv-array.h" #include "run-command.h" +#include "prompt.h" static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS") static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV") @@ -24,6 +25,7 @@ static const char * const git_bisect_helper_usage[] = { N_("git bisect--helper --bisect-reset []"), N_("git bisect--helper --bisect-write []"), N_("git bisect--helper --bisect-check-and-set-terms "), + N_("git bisect--helper --bisect-next-check [] term_bad.buf); + char *good_glob = xstrfmt("%s-*", terms->term_good.buf); + char *bad_syn, *good_syn; + + if (ref_exists(bad_ref)) + missing_bad = 0; + free(bad_ref); + + for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/", +(void *) &missing_good); + free(good_glob); + + if (!missing_good && !missing_bad) + return 0; + + if (!current_term) + return -1; + + if (missing_good && !missing_bad && current_term && + !strcmp(current_term, terms->term_good.buf)) { + char *yesno; + /* +* have bad (or new) but not good (or old). We could bisect +* although this is less optimum. +*/ + fprintf(stderr, N_("Warning: bisecting only with a %s commit\n"), + terms->term_bad.buf); + if (!isatty(0)) + return 0; + /* +* TRANSLATORS: Make sure to include [Y] and [n] in your +* translation. The program will only accept English input +* at this point. +*/ + yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO); + if (starts_with(yesno, "N") || starts_with(yesno, "n")) + return -1; + + return 0; + } + bad_syn = xstrdup(bisect_voc("bad")); + good_syn = xstrdup(bisect_voc("good")); + if (!is_empty_or_missing_file(git_path_bisect_start())) { + error(_("You need to give me at least one %s and " + "%s revision. You can use \"git bisect %s\" " + "and \"git bisect %s\" for that. \n"), + bad_syn, good_syn, bad_syn, good_syn); + free(bad_syn); + free(good_syn); + return -1; + } + else { + error(_("You need to start by \"git bisect start\". You " + "then need to give me at least one %s and %s " + "revision. You can use \"git bisect %s\" and " + "\"git bisect %s\" for that.\n"), + good_syn, bad_syn, bad_syn, good_syn); + free(bad_syn); + free(good_syn); + return -1; + } + free(bad_syn); + free(good_syn); + + return 0; +} + int cmd_bisect__helper(int argc, const char **argv, const char *prefix) { enum { @@ -302,7 +393,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) BISECT_RESET, CHECK_EXPECTED_REVS, BISECT_WRITE, - CHECK_AND_SET_TERMS + CHECK_AND_SET_TERMS, + BISECT_NEXT_CHECK } cmdmode = 0; int no_checkout = 0, res = 0; struct option options[] = { @@ -320,6 +412,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE), OPT_CMDMODE(0, "check-and-set-terms", &cmdmode, N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS), + OPT_CMDMODE(0, "bisect-next-check", &cmdmode, +N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK), OPT_BOOL(0, "no-checkout", &n