Re: [PATCH v12 11/13] bisect--helper: `bisect_next_check` & bisect_voc shell function in C

[Pranit Bauva]

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]

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

[Junio C Hamano]

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

[Pranit Bauva]

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 *) _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,