Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to translate diff message #1045

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jordimas
Copy link
Contributor

@jordimas jordimas commented Jul 12, 2021

cc: Bagas Sanjaya bagasdotme@gmail.com
cc: Ævar Arnfjörð Bjarmason avarab@gmail.com

@gitgitgadget-git
Copy link

There is an issue in commit e182d95:
Commit checks stopped - the message is too short

Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, except of course for the commit message ;-)

@gitgitgadget-git
Copy link

There are issues in commit 48b686f:
The first line must be separated from the rest by an empty line
Commit not signed off

@gitgitgadget-git
Copy link

There is an issue in commit b7e578f:
The first line must be separated from the rest by an empty line

@gitgitgadget-git
Copy link

There is an issue in commit 0676e24:
The first line must be separated from the rest by an empty line

Allows to translate the diff messages shown when the
user commits, indicating the number of insertions,
deletions and files changed.

Signed-off-by: Jordi Mas <jmas@softcatala.org>
@jordimas
Copy link
Contributor Author

Looks good, except of course for the commit message ;-)

Fixed! Thanks

@dscho
Copy link
Member

dscho commented Jul 13, 2021

/submit time?

@jordimas
Copy link
Contributor Author

jordimas commented Jul 13, 2021

/submit time?

My understanding is that the next step is to write a comment with /submit', correct? Thanks First time I do this

@jordimas jordimas closed this Jul 13, 2021
@jordimas jordimas reopened this Jul 13, 2021
@dscho
Copy link
Member

dscho commented Jul 13, 2021

/submit time?

My understanding is that the next step is to write a comment with /submit', correct? Thanks First time I do this

Yes. See #1035 (comment) for more details.

@jordimas
Copy link
Contributor Author

/preview

@gitgitgadget-git
Copy link

Preview email sent as pull.1045.git.git.1626177345699.gitgitgadget@gmail.com

@jordimas
Copy link
Contributor Author

/submit

@gitgitgadget-git
Copy link

Submitted as pull.1045.git.git.1626177676801.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git pr-git-1045/jordimas/localize-msg-v1

To fetch this version to local tag pr-git-1045/jordimas/localize-msg-v1:

git fetch --no-tags https://github.com/gitgitgadget/git tag pr-git-1045/jordimas/localize-msg-v1

@gitgitgadget-git
Copy link

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Jordi Mas via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Subject: Re: [PATCH] l10n: allows to translate diff messages

"allow to translate" would be the right phrasing, but it is too
vague to say "diff messages".  You are only marking 3 messages for
translation when there are probably a handful more.  

You need to explain which ones, not just vague "diff messages".

I think you are focusing on the words on the "git diff --stat" and
"git diff --shortstat" summary line, so

    i18n: mark "git diff --[short]stat" summary for translation

perhaps?  And remember, i18n is the act of making the code capable
of being translated, while l10n is the act of actually translating
what i18n prepared into a particular language.  Here, i18n is more
appropriate.

> From: Jordi Mas <jmas@softcatala.org>
>
> Allows to translate the diff messages shown when the
> user commits, indicating the number of insertions,
> deletions and files changed.

Our log message first explain the current status/behaviour of the
code (and highlight the problem in it), and then gives an order to
the codebase to "be like so" in imperative mood.  Perhaps like this:

    "git diff --[short]stat" summarizes the number of paths touched
    and lines inserted and deleted.  Mark this message for
    translation.

Note that I didn't explicitly say what's wrong with the current
behaviour, but "mark for translation" as the order given to the
codebase, it should be clear enough from the context that the
problem is that it is not marked for translation.

Having said all that, I am not sure if this patch stop at a sensible
point.  At least "git diff --summary" output should also be updated
to match, as they often go hand-in-hand, no?

Also, when showing "git diff --stat" for a binary file, we'd say

	$path | Bin 1234 -> 1236 bytes

I wonder if that should also be translated.

While I am at it, there are a handful things that should NEVER be
translated in the "git diff" output.

> diff --git a/diff.c b/diff.c
> index 52c791574b..29db9ce079 100644

Between "diff --git" and "--- a/$path", there are metainfo lines
that are read by "git apply" (including this "index" line).  They
should never be translated.

Thanks.

@gitgitgadget-git
Copy link

On the Git mailing list, Bagas Sanjaya wrote (reply to this):

On 14/07/21 06.11, Junio C Hamano wrote:
> Also, when showing "git diff --stat" for a binary file, we'd say
> 
> 	$path | Bin 1234 -> 1236 bytes
> 
> I wonder if that should also be translated.
> 

I think that above should also be translated, for consistency with diff 
for regular text file.

-- 
An old man doll... just what I always wanted! - Clara

@gitgitgadget-git
Copy link

User Bagas Sanjaya <bagasdotme@gmail.com> has been added to the cc: list.

@gitgitgadget-git
Copy link

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Tue, Jul 13 2021, Junio C Hamano wrote:

> "Jordi Mas via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> Subject: Re: [PATCH] l10n: allows to translate diff messages
>
> "allow to translate" would be the right phrasing, but it is too
> vague to say "diff messages".  You are only marking 3 messages for
> translation when there are probably a handful more.  
>
> You need to explain which ones, not just vague "diff messages".
>
> I think you are focusing on the words on the "git diff --stat" and
> "git diff --shortstat" summary line, so
>
>     i18n: mark "git diff --[short]stat" summary for translation
>
> perhaps?  And remember, i18n is the act of making the code capable
> of being translated, while l10n is the act of actually translating
> what i18n prepared into a particular language.  Here, i18n is more
> appropriate.

What i18n v.s. l10n means is quite the side-discussion, I have not heard
it used like this before, but rather as e.g. defined at :
https://www.w3.org/International/questions/qa-i18n

I.e. in git.git's case that all our gettext-ization would fall under
i18n, but l10n are generally a step beyond that, e.g. if "git status"
output entries were sorted by locale, fsck and other progress.c users
used localized numeric formatting etc.

IOW I agree that i18n is more appropriate here, I just hadn't heard
i18n/l10n used in the way that you describe.

@gitgitgadget-git
Copy link

User Ævar Arnfjörð Bjarmason <avarab@gmail.com> has been added to the cc: list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants