format_branch_comparison: diverged message has only plural case#2239
Open
HaraldNordgren wants to merge 1 commit intogit:masterfrom
Open
format_branch_comparison: diverged message has only plural case#2239HaraldNordgren wants to merge 1 commit intogit:masterfrom
HaraldNordgren wants to merge 1 commit intogit:masterfrom
Conversation
|
There are issues in commit bc9ce03:
|
bc9ce03 to
27432a5
Compare
|
There is an issue in commit 27432a5:
|
Drop Q_() singular form and use _() with the plural string only. Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
27432a5 to
62bf7a5
Compare
Contributor
Author
|
/submit |
|
Submitted as pull.2239.git.git.1773479526823.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Harald Nordgren wrote on the Git mailing list (how to reply to this email): >> Harald Nordgren (2):
>> refactor format_branch_comparison in preparation
>> status: show comparison with push remote tracking branch
>>
>> remote.c | 183 ++++++++++++++++++++-------
>> t/t6040-tracking-info.sh | 262 +++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 403 insertions(+), 42 deletions(-)
>>
>>
>> base-commit: d529f3a197364881746f558e5652f0236131eb86
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2138%2FHaraldNordgren%2Fahead_of_main_status-v20
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2138/HaraldNordgren/ahead_of_main_status-v20
>> Pull-Request: https://github.com/git/git/pull/2138
>>
>> Range-diff vs v19:
>>
>> 1: 451d7a4986 ! 1: bb3e00863b refactor format_branch_comparison in preparation
>> @@ remote.c: int format_tracking_info(struct branch *branch, struct strbuf *sb,
>> if (advice_enabled(ADVICE_STATUS_HINTS))
>> strbuf_addstr(sb,
>> _(" (use \"git pull\" to update your local branch)\n"));
>> -@@ remote.c: int format_tracking_info(struct branch *branch, struct strbuf *sb,
>> - "and have %d and %d different commits each, "
>> - "respectively.\n",
>> - ours + theirs),
>> + } else {
>> + strbuf_addf(sb,
>> +- Q_("Your branch and '%s' have diverged,\n"
>> +- "and have %d and %d different commit each, "
>> +- "respectively.\n",
>> +- "Your branch and '%s' have diverged,\n"
>> +- "and have %d and %d different commits each, "
>> +- "respectively.\n",
>> +- ours + theirs),
>> - base, ours, theirs);
>> ++ "Your branch and '%s' have diverged,\n"
>> ++ "and have %d and %d different commits each, respectively.\n",
>> + branch_name, ours, theirs);
>> if (show_divergence_advice &&
>> advice_enabled(ADVICE_STATUS_HINTS))
>
> Could you not mix the ours+theirs thing into the same step? Either
> make it a standalone patch to clean up before or after your main 2
> patches, or leave it totally outside the series and send it after
> this series settles.
Making a change that was left out of https://lore.kernel.org/git/xmqqzf6lqs9w.fsf@gitster.g/
Harald |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Drop Q_() singular form and use _() with the plural string only.
I know the commit title talks about plural-only, but please make
sure that the body of the log message carries all the necessary
information to justify the change standalone. "In the else clause,
both ours and theirs are positive integers so ours+theirs must be at
least 2, hence there is no need to prepare singular and plural
variants of the message", or something to that effect, perhaps.
The patch text and the reasoning behind it does sound familiar and I
vaguely recall discussing about it ;-)
Thanks.
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> format_branch_comparison: diverged message has only plural case
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2239%2FHaraldNordgren%2Fformat_branch_comparison__plural-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2239/HaraldNordgren/format_branch_comparison__plural-v1
> Pull-Request: https://github.com/git/git/pull/2239
>
> remote.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 7ca2a6501b..12136dfa23 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2307,13 +2307,8 @@ static void format_branch_comparison(struct strbuf *sb,
> _(" (use \"git pull\" to update your local branch)\n"));
> } else {
> strbuf_addf(sb,
> - Q_("Your branch and '%s' have diverged,\n"
> - "and have %d and %d different commit each, "
> - "respectively.\n",
> - "Your branch and '%s' have diverged,\n"
> - "and have %d and %d different commits each, "
> - "respectively.\n",
> - ours + theirs),
> + _("Your branch and '%s' have diverged,\n"
> + "and have %d and %d different commits each, respectively.\n"),
> branch_name, ours, theirs);
> if (use_divergence_advice && advice_enabled(ADVICE_STATUS_HINTS))
> strbuf_addstr(sb,
>
> base-commit: dc6ecd5354dca88d51b6d6562777fc8fc10d77e1 |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): Junio C Hamano <gitster@pobox.com> writes:
> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Harald Nordgren <haraldnordgren@gmail.com>
>>
>> Drop Q_() singular form and use _() with the plural string only.
>
> I know the commit title talks about plural-only, but please make
> sure that the body of the log message carries all the necessary
> information to justify the change standalone. "In the else clause,
> both ours and theirs are positive integers so ours+theirs must be at
> least 2, hence there is no need to prepare singular and plural
> variants of the message", or something to that effect, perhaps.
>
> The patch text and the reasoning behind it does sound familiar and I
> vaguely recall discussing about it ;-)
>
> Thanks.
I queued the patch, tentatively with this rewritten message:
remote: don't use Q_() when it is not needed
In this code path, both ours and theirs are already known to be
positive integers, so ours + theirs will always be plural, never
using the first variant given to Q_().
Just use _() with the plural string only.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thanks. |
|
Phillip Wood wrote on the Git mailing list (how to reply to this email): On 14/03/2026 18:38, Junio C Hamano wrote:
> > remote: don't use Q_() when it is not needed
> > In this code path, both ours and theirs are already known to be
> positive integers, so ours + theirs will always be plural, never
> using the first variant given to Q_().
> > Just use _() with the plural string only.
There can be more than one form of the plural string though. The gettext manual has the following example of the Polish translation of "file" for different numbers of files [1]
1 plik
2,3,4 pliki
5-21 plików
22-24 pliki
25-31 plików
ngettext() handles that correctly, translating a single string without an associated count will not.
Thanks
Phillip
[1] https://www.gnu.org/software/gettext/manual/gettext.html#Additional-functions-for-plural-forms |
|
User |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): Phillip Wood <phillip.wood123@gmail.com> writes:
> There can be more than one form of the plural string though. The gettext
> manual has the following example of the Polish translation of "file" for
> different numbers of files [1]
>
> 1 plik
> 2,3,4 pliki
> 5-21 plików
> 22-24 pliki
> 25-31 plików
>
> ngettext() handles that correctly, translating a single string without
> an associated count will not.
That is a very interesting example, and a valid reason to have me
retract the #leftoverbits that led to the patch being discussed.
But wouldn't that lead to an awkward conclusion, i.e., hits from
"git grep '[^Q]_("[^"]*%[id]' \*.c" are potential bugs that need to
be updated to use ngettext().
Of course, we need to exclude messages like "the error code %d was
returned" and "you have a bug on line %d", but there seem to be real
errors in randomly selected hits from the "git grep" output, e.g.,
add-patch.c: _("Split into %d hunks."),
archive-zip.c: return error(_("path too long (%d chars, SHA1: %s): %s"),
builtin/checkout.c: die(_("'%s' matched multiple (%d) remote tracking branches"),
builtin/credential-store.c: die_errno(_("unable to get credential storage lock in %d ms"), timeout_ms);
builtin/describe.c: _("found %i tags; gave up search at %s\n"),
builtin/fsck.c: fprintf_ln(stderr, _("Checking connectivity (%d objects)"), max);
You can notice that I started from 'a' and stopped very early in 'b'
;-).
Thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cc: Phillip Wood phillip.wood123@gmail.com