{"thread":{"id":"58298","subject":"[RFC/PATCH] sequencer: do not translate reflog messages","startedAt":"2022-08-12T15:46:33Z","lastAt":"2022-08-22T16:12:38Z","messageCount":34,"participants":["Michael J Gruber","Junio C Hamano","Phillip Wood","Johannes Schindelin","Ævar Arnfjörð Bjarmason","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"461122","messageId":"b8ab40b2b0e3e5d762b414329ad2f4552f935d28.1660318162.git.git@grubix.eu","threadId":"58298","inReplyTo":null,"subject":"[RFC/PATCH] sequencer: do not translate reflog messages","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2022-08-12T15:38:40Z","receivedAt":"2022-08-12T15:46:33Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Traditionally, reflog messages were never translated, in particular not\non storage.\n\nDue to the switch of more parts of git to the sequencer, old changes in\nthe sequencer code may lead to recent changes in git's behaviour. E.g.:\nc28cbc5ea6 (\"sequencer: mark action_name() for translation\", 2016-10-21)\nmarked several uses of `action_name()` for translation. Recently, this\nlead to a partially translated reflog:\n\n`rebase: fast-forward` is translated (e.g. in de to `Rebase: Vorspulen`)\nwhereas other reflog entries such as `rebase (pick):` remain\nuntranslated as they should be.\n\nChange the relevant line in the sequencer so that this reflog entry\nremains untranslated, as well.\n\nSigned-off-by: Michael J Gruber <git@grubix.eu>\n---\nThe patch also changes `action_name()` not to translate the names. This\nmakes no difference for `rebase: fast-forward` (I don't quite grok why\nso far) but in any case, the callers mark the result of `action_name()`\n(or do not mark it) so that the result itself should not be translated.\nThe full test suite passes either way.\n\nRFC for my lack of full grasp of the relevant code paths.\n\n sequencer.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 5f22b7cd37..b456489590 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -395,11 +395,11 @@ static const char *action_name(const struct replay_opts *opts)\n {\n \tswitch (opts->action) {\n \tcase REPLAY_REVERT:\n-\t\treturn N_(\"revert\");\n+\t\treturn \"revert\";\n \tcase REPLAY_PICK:\n-\t\treturn N_(\"cherry-pick\");\n+\t\treturn \"cherry-pick\";\n \tcase REPLAY_INTERACTIVE_REBASE:\n-\t\treturn N_(\"rebase\");\n+\t\treturn \"rebase\";\n \t}\n \tdie(_(\"unknown action: %d\"), opts->action);\n }\n@@ -575,7 +575,7 @@ static int fast_forward_to(struct repository *r,\n \tif (checkout_fast_forward(r, from, to, 1))\n \t\treturn -1; /* the callee should have complained already */\n \n-\tstrbuf_addf(&sb, _(\"%s: fast-forward\"), _(action_name(opts)));\n+\tstrbuf_addf(&sb, \"%s: fast-forward\", action_name(opts));\n \n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n-- \n2.37.1.671.g27f8e4a42a\n\n"},{"id":"461127","messageId":"xmqq8rntcr8x.fsf@gitster.g","threadId":"58298","inReplyTo":"b8ab40b2b0e3e5d762b414329ad2f4552f935d28.1660318162.git.git@grubix.eu","subject":"Re: [RFC/PATCH] sequencer: do not translate reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-12T17:21:18Z","receivedAt":"2022-08-12T17:21:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@grubix.eu> writes:\n\n> Traditionally, reflog messages were never translated, in particular not\n> on storage.\n\nTrue, and it must (unfortunately) stay to be the way, because tools\n(like @{-<n>} syntax) expect to be able to parse out what we write.\n\n> Due to the switch of more parts of git to the sequencer, old changes in\n> the sequencer code may lead to recent changes in git's behaviour. E.g.:\n> c28cbc5ea6 (\"sequencer: mark action_name() for translation\", 2016-10-21)\n> marked several uses of `action_name()` for translation. Recently, this\n> lead to a partially translated reflog:\n>\n> `rebase: fast-forward` is translated (e.g. in de to `Rebase: Vorspulen`)\n> whereas other reflog entries such as `rebase (pick):` remain\n> untranslated as they should be.\n>\n> Change the relevant line in the sequencer so that this reflog entry\n> remains untranslated, as well.\n\nGood move, I would have to say X-<.\n\nIn the longer term, we need to transition to a new version of reflog\nmessage, where \"git reflog\" output can (meaning: with an option) or\ndoes (meaning: by default) show localized message, but the internal\nmachinery as well as scripts can ask to see an untranslated message.\n\nWe would need to teach the reflog machinery to understand a reflog\nmessage specially formatted (e.g. with an unusual prefix like\n\"::v2::\"), from which both untranslated and translated messages can\nbe parsed out or generated.  Codepaths that write reflog messages\nmay need to be adjusted to send both versions to the ref machinery.\n\nLooking at recent reflog entries I happen to have in \"git reflog\n--format=\"%gs\" HEAD@{now}\"\n\n    checkout: moving from 219fe53025fdf5c3fb79d289a36eb2cad3f38a04 to master\n    checkout: moving from master to next^0\n    commit (amend): fsmonitor: option to allow fsmonitor to run against network-mounted repos\n    checkout: moving from d5eaf969c17c196268d9db7af50f6767ec3a3d0a to ed/fsmonitor-on-network-disk\n    am: fsmonitor: option to allow fsmonitor to run against network-mounted repos\n    merge @{-1}: Merge made by the 'ort' strategy.\n    checkout: moving from ll/disk-usage-humanise to seen\n    am: rev-list: support human-readable output for `--disk-usage`\n    checkout: moving from master to ll/disk-usage-humanise\n\none relatively easy way to do so may be to store the printf-like\nformat string, possibly limiting to %s and nothing else, e.g.\n\n    \"checkout: moving from %s to %s\"\n    \"am: %s\"\n    \"merge %s: Merge made by the '%s' strategy\"\n\ntogether with the parameters to fill in these %s blanks, as a\nN-tuple of strings, i.e.\n\n    (\"checkout: moving from %s to %s\",\n     \"219fe53025fdf5c3fb79d289a36eb2cad3f38a04\", \"master\")\n\nand then serialize them into a single long string (with that special\nprefix to allow us notice the format).\n\nBut I'll leave the details of how the new format can be made to\nallow storing raw and translated messages.  The review thread of\nthis patch is not a good place or time to discuss it.\n\nThanks.\n"},{"id":"461133","messageId":"333bbaa9-d484-7c20-90d6-e64edf8a8248@gmail.com","threadId":"58298","inReplyTo":"b8ab40b2b0e3e5d762b414329ad2f4552f935d28.1660318162.git.git@grubix.eu","subject":"Re: [RFC/PATCH] sequencer: do not translate reflog messages","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-08-12T19:21:04Z","receivedAt":"2022-08-12T19:21:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Michael\n\nOn 12/08/2022 16:38, Michael J Gruber wrote:\n> Traditionally, reflog messages were never translated, in particular not\n> on storage.\n> \n> Due to the switch of more parts of git to the sequencer, old changes in\n> the sequencer code may lead to recent changes in git's behaviour. E.g.:\n> c28cbc5ea6 (\"sequencer: mark action_name() for translation\", 2016-10-21)\n> marked several uses of `action_name()` for translation. Recently, this\n> lead to a partially translated reflog:\n> \n> `rebase: fast-forward` is translated (e.g. in de to `Rebase: Vorspulen`)\n> whereas other reflog entries such as `rebase (pick):` remain\n> untranslated as they should be.\n> \n> Change the relevant line in the sequencer so that this reflog entry\n> remains untranslated, as well.\n> \n> Signed-off-by: Michael J Gruber <git@grubix.eu>\n> ---\n> The patch also changes `action_name()` not to translate the names This > makes no difference for `rebase: fast-forward` (I don't quite grok why\n> so far) but in any case, the callers mark the result of `action_name()`\n> (or do not mark it) so that the result itself should not be translated.\n> The full test suite passes either way.\n> \n> RFC for my lack of full grasp of the relevant code paths.\n> \n>   sequencer.c | 8 ++++----\n>   1 file changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index 5f22b7cd37..b456489590 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -395,11 +395,11 @@ static const char *action_name(const struct replay_opts *opts)\n>   {\n>   \tswitch (opts->action) {\n>   \tcase REPLAY_REVERT:\n> -\t\treturn N_(\"revert\");\n> +\t\treturn \"revert\";\n>   \tcase REPLAY_PICK:\n> -\t\treturn N_(\"cherry-pick\");\n> +\t\treturn \"cherry-pick\";\n>   \tcase REPLAY_INTERACTIVE_REBASE:\n> -\t\treturn N_(\"rebase\");\n> +\t\treturn \"rebase\";\n\nRemoving the N_() stops these strings from being extracted for \ntranslation, but there are several callers left that are still using _() \nto get the (now non-existent) translated string. I only had a quick look \nbut I think we should remove the _() from all the callers of action_name().\n\nBest Wishes\n\nPhillip\n\n>   \t}\n>   \tdie(_(\"unknown action: %d\"), opts->action);\n>   }\n> @@ -575,7 +575,7 @@ static int fast_forward_to(struct repository *r,\n>   \tif (checkout_fast_forward(r, from, to, 1))\n>   \t\treturn -1; /* the callee should have complained already */\n>   \n> -\tstrbuf_addf(&sb, _(\"%s: fast-forward\"), _(action_name(opts)));\n> +\tstrbuf_addf(&sb, \"%s: fast-forward\", action_name(opts));\n>   \n>   \ttransaction = ref_transaction_begin(&err);\n>   \tif (!transaction ||\n"},{"id":"461150","messageId":"xmqqy1vt9ora.fsf@gitster.g","threadId":"58298","inReplyTo":"333bbaa9-d484-7c20-90d6-e64edf8a8248@gmail.com","subject":"Re: [RFC/PATCH] sequencer: do not translate reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-12T20:43:21Z","receivedAt":"2022-08-12T20:43:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Removing the N_() stops these strings from being extracted for\n> translation, but there are several callers left that are still using\n> _() to get the (now non-existent) translated string. I only had a\n> quick look but I think we should remove the _() from all the callers\n> of action_name().\n\nThanks, that's all correct.\n"},{"id":"461255","messageId":"92sr80s2-6311-p065-755s-61s28s543q6n@tzk.qr","threadId":"58298","inReplyTo":"xmqqy1vt9ora.fsf@gitster.g","subject":"Re: [RFC/PATCH] sequencer: do not translate reflog messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-15T20:20:52Z","receivedAt":"2022-08-16T00:04:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 12 Aug 2022, Junio C Hamano wrote:\n\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n> > Removing the N_() stops these strings from being extracted for\n> > translation, but there are several callers left that are still using\n> > _() to get the (now non-existent) translated string. I only had a\n> > quick look but I think we should remove the _() from all the callers\n> > of action_name().\n>\n> Thanks, that's all correct.\n\nI am afraid that it is not.\n\nIn https://github.com/git/git/blob/v2.37.2/sequencer.c#L502-L503, for\nexample, we use the value returned by `action_name()` in a translated\nmessage:\n\n\terror(_(\"your local changes would be overwritten by %s.\"),\n\t\t_(action_name(opts)));\n\nMichael, I am afraid that we need more nuance here.\n\nI do see that https://github.com/git/git/blob/v2.37.2/sequencer.c#L4316\ncalls `action_name()` without wrapping it in `_(...)`:\n\n\tsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n\nThis suggests to me that the proper solution will be to carefully vet\nwhich `_(action_name())` calls should drop the `_(...)` and which ones\nshould not, and to leave the `N_(...)` parts alone.\n\nThe affected calls seem to fall into these categories:\n\n- reflog (do _not_ translate the action name)\n\n- parameter of `error_resolve_conflict()` (do _not_ translate the\n  parameter)\n\n- error messages talking about `git <command>` (do _not_ translate the\n  action name)\n\n- error messages talking about the operation (_do_ translate the action\n  name)\n\nMy take on which lines need to be patched:\n\n- https://github.com/git/git/blob/v2.37.2/sequencer.c#L500\n- https://github.com/git/git/blob/v2.37.2/sequencer.c#L538\n- https://github.com/git/git/blob/v2.37.2/sequencer.c#L2384\n- https://github.com/git/git/blob/v2.37.2/sequencer.c#L2392\n- https://github.com/git/git/blob/v2.37.2/sequencer.c#L3715\n\nbut not\n\n- https://github.com/git/git/blob/v2.37.2/sequencer.c#L503\n- https://github.com/git/git/blob/v2.37.2/sequencer.c#L689\n\nCiao,\nDscho\n"},{"id":"461263","messageId":"870072d5-d220-09e7-684b-f9d7d8d59c93@gmail.com","threadId":"58298","inReplyTo":"92sr80s2-6311-p065-755s-61s28s543q6n@tzk.qr","subject":"Re: [RFC/PATCH] sequencer: do not translate reflog messages","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-08-16T08:59:35Z","receivedAt":"2022-08-16T09:50:07Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Dscho\n\nOn 15/08/2022 21:20, Johannes Schindelin wrote:\n> Hi Junio,\n> \n> On Fri, 12 Aug 2022, Junio C Hamano wrote:\n> \n>> Phillip Wood <phillip.wood123@gmail.com> writes:\n>>\n>>> Removing the N_() stops these strings from being extracted for\n>>> translation, but there are several callers left that are still using\n>>> _() to get the (now non-existent) translated string. I only had a\n>>> quick look but I think we should remove the _() from all the callers\n>>> of action_name().\n>>\n>> Thanks, that's all correct.\n> \n> I am afraid that it is not.\n> \n> In https://github.com/git/git/blob/v2.37.2/sequencer.c#L502-L503, for\n> example, we use the value returned by `action_name()` in a translated\n> message:\n> \n> \terror(_(\"your local changes would be overwritten by %s.\"),\n> \t\t_(action_name(opts)));\n\nIsn't this message using action_name() to get the name of the command \nthat the user ran? As that name is not localized when the user runs the \ncommand I don't see that we should be translating it (and playing \nsentence lego with the result) in this message. I think the same applies \nto the message at line 689 that you mention below.\n\nBest Wishes\n\nPhillip\n\n> Michael, I am afraid that we need more nuance here.\n> \n> I do see that https://github.com/git/git/blob/v2.37.2/sequencer.c#L4316\n> calls `action_name()` without wrapping it in `_(...)`:\n> \n> \tsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n> \n> This suggests to me that the proper solution will be to carefully vet\n> which `_(action_name())` calls should drop the `_(...)` and which ones\n> should not, and to leave the `N_(...)` parts alone.\n> \n> The affected calls seem to fall into these categories:\n> \n> - reflog (do _not_ translate the action name)\n> \n> - parameter of `error_resolve_conflict()` (do _not_ translate the\n>    parameter)\n> \n> - error messages talking about `git <command>` (do _not_ translate the\n>    action name)\n> \n> - error messages talking about the operation (_do_ translate the action\n>    name)\n> \n> My take on which lines need to be patched:\n> \n> - https://github.com/git/git/blob/v2.37.2/sequencer.c#L500\n> - https://github.com/git/git/blob/v2.37.2/sequencer.c#L538\n> - https://github.com/git/git/blob/v2.37.2/sequencer.c#L2384\n> - https://github.com/git/git/blob/v2.37.2/sequencer.c#L2392\n> - https://github.com/git/git/blob/v2.37.2/sequencer.c#L3715\n> \n> but not\n> \n> - https://github.com/git/git/blob/v2.37.2/sequencer.c#L503\n> - https://github.com/git/git/blob/v2.37.2/sequencer.c#L689\n> \n> Ciao,\n> Dscho\n"},{"id":"461276","messageId":"09rn6r61-38qo-4s1q-q7qq-p5onp6p87o44@tzk.qr","threadId":"58298","inReplyTo":"870072d5-d220-09e7-684b-f9d7d8d59c93@gmail.com","subject":"Re: [RFC/PATCH] sequencer: do not translate reflog messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-16T11:02:38Z","receivedAt":"2022-08-16T11:37:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Tue, 16 Aug 2022, Phillip Wood wrote:\n\n> On 15/08/2022 21:20, Johannes Schindelin wrote:\n>\n> > On Fri, 12 Aug 2022, Junio C Hamano wrote:\n> >\n> > > Phillip Wood <phillip.wood123@gmail.com> writes:\n> > >\n> > > > Removing the N_() stops these strings from being extracted for\n> > > > translation, but there are several callers left that are still using\n> > > > _() to get the (now non-existent) translated string. I only had a\n> > > > quick look but I think we should remove the _() from all the callers\n> > > > of action_name().\n> > >\n> > > Thanks, that's all correct.\n> >\n> > I am afraid that it is not.\n> >\n> > In https://github.com/git/git/blob/v2.37.2/sequencer.c#L502-L503, for\n> > example, we use the value returned by `action_name()` in a translated\n> > message:\n> >\n> >  error(_(\"your local changes would be overwritten by %s.\"),\n> >   _(action_name(opts)));\n>\n> Isn't this message using action_name() to get the name of the command that the\n> user ran? As that name is not localized when the user runs the command I don't\n> see that we should be translating it (and playing sentence lego with the\n> result) in this message. I think the same applies to the message at line 689\n> that you mention below.\n\nI do not believe that this error message talks about the command,\notherwise it would use \"`git %s`\" instead of \"%s\" here. Imagine, for a\nsecond, that Git was written in French and you preferred to read your\nerror messages in English, therefore set your locale, and you just issued\na `git retour`, would this error message read well for you?\n\n\terror: your local changes would be overwritten by retour.\n\nThat looks wrong to me. I could see us changing this to:\n\n\terror: your local changes would be overwritten by `git retour`.\n\nor to:\n\n\terror: your local changes would be overwritten by revert.\n\ni.e. either use \"`git %s`\" without translating, or keeping \"%s\" with the\ntranslated `action_name()`. But it would probably read better to have the\naction name localized (which is what I suggested).\n\nCiao,\nDscho\n"},{"id":"461413","messageId":"cover.1660828108.git.git@grubix.eu","threadId":"58298","inReplyTo":"09rn6r61-38qo-4s1q-q7qq-p5onp6p87o44@tzk.qr","subject":"[PATCH 0/4] sequencer: clarify translations","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2022-08-18T13:13:25Z","receivedAt":"2022-08-18T13:14:15Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Hi there,\n\nthanks for all your input to my RFC patch. I tried to summarize and pack\neverything up into this little series.\n\nA follow-up could (but does not have to) turn translated action names\ninto untranslated git command names in some places.\n\nCheers\nMichael\n\nMichael J Gruber (4):\n  sequencer: do not translate reflog messages\n  sequencer: do not translate parameters to error_resolve_conflict()\n  sequencer: do not translate command names\n  po: adjust README to code\n\n po/README.md |  2 +-\n sequencer.c  | 10 +++++-----\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\n-- \n2.37.2.596.g72ccb331cf\n\n"},{"id":"461414","messageId":"f1d4ee05af8f88bfdca94c5e2030228e8ad5610f.1660828108.git.git@grubix.eu","threadId":"58298","inReplyTo":"cover.1660828108.git.git@grubix.eu","subject":"[PATCH 3/4] sequencer: do not translate command names","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2022-08-18T13:13:28Z","receivedAt":"2022-08-18T13:14:15Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"When action_name is used to denote a command `git %s` do not translate\nsince command names are never translated.\n\nSuggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Michael J Gruber <git@grubix.eu>\n---\n sequencer.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 8b32b239b9..79dad522f5 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2422,7 +2422,7 @@ static int read_and_refresh_cache(struct repository *r,\n \tif (repo_read_index(r) < 0) {\n \t\trollback_lock_file(&index_lock);\n \t\treturn error(_(\"git %s: failed to read the index\"),\n-\t\t\t_(action_name(opts)));\n+\t\t\taction_name(opts));\n \t}\n \trefresh_index(r->index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);\n \n@@ -2430,7 +2430,7 @@ static int read_and_refresh_cache(struct repository *r,\n \t\tif (write_locked_index(r->index, &index_lock,\n \t\t\t\t       COMMIT_LOCK | SKIP_IF_UNCHANGED)) {\n \t\t\treturn error(_(\"git %s: failed to refresh the index\"),\n-\t\t\t\t_(action_name(opts)));\n+\t\t\t\taction_name(opts));\n \t\t}\n \t}\n \n-- \n2.37.2.596.g72ccb331cf\n\n"},{"id":"461415","messageId":"ea6c65c254bb08b20ea6c4d81200b847755b555c.1660828108.git.git@grubix.eu","threadId":"58298","inReplyTo":"cover.1660828108.git.git@grubix.eu","subject":"[PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2022-08-18T13:13:26Z","receivedAt":"2022-08-18T13:14:17Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Traditionally, reflog messages were never translated, in particular not\non storage.\n\nDue to the switch of more parts of git to the sequencer, old changes in\nthe sequencer code may lead to recent changes in git's behaviour. E.g.:\nc28cbc5ea6 (\"sequencer: mark action_name() for translation\", 2016-10-21)\nmarked several uses of `action_name()` for translation. Recently, this\nlead to a partially translated reflog:\n\n`rebase: fast-forward` is translated (e.g. in de to `Rebase: Vorspulen`)\nwhereas other reflog entries such as `rebase (pick):` remain\nuntranslated as they should be.\n\nChange the relevant line in the sequencer so that this reflog entry\nremains untranslated, as well.\n\nSigned-off-by: Michael J Gruber <git@grubix.eu>\n---\n sequencer.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 5f22b7cd37..51d75dfbe1 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -575,7 +575,7 @@ static int fast_forward_to(struct repository *r,\n \tif (checkout_fast_forward(r, from, to, 1))\n \t\treturn -1; /* the callee should have complained already */\n \n-\tstrbuf_addf(&sb, _(\"%s: fast-forward\"), _(action_name(opts)));\n+\tstrbuf_addf(&sb, \"%s: fast-forward\", action_name(opts));\n \n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n-- \n2.37.2.596.g72ccb331cf\n\n"},{"id":"461416","messageId":"4684d54aeb3e00c96ba581c824a04e47b7236db7.1660828108.git.git@grubix.eu","threadId":"58298","inReplyTo":"cover.1660828108.git.git@grubix.eu","subject":"[PATCH 2/4] sequencer: do not translate parameters to error_resolve_conflict()","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2022-08-18T13:13:27Z","receivedAt":"2022-08-18T13:14:19Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"`error_resolve_conflict()` checks the untranslated action_name\nparameter, so pass it as is.\n\nSuggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Michael J Gruber <git@grubix.eu>\n---\n sequencer.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 51d75dfbe1..8b32b239b9 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -537,7 +537,7 @@ static struct tree *empty_tree(struct repository *r)\n static int error_dirty_index(struct repository *repo, struct replay_opts *opts)\n {\n \tif (repo_read_index_unmerged(repo))\n-\t\treturn error_resolve_conflict(_(action_name(opts)));\n+\t\treturn error_resolve_conflict(action_name(opts));\n \n \terror(_(\"your local changes would be overwritten by %s.\"),\n \t\t_(action_name(opts)));\n@@ -3753,7 +3753,7 @@ static int do_reset(struct repository *r,\n \tinit_checkout_metadata(&unpack_tree_opts.meta, name, &oid, NULL);\n \n \tif (repo_read_index_unmerged(r)) {\n-\t\tret = error_resolve_conflict(_(action_name(opts)));\n+\t\tret = error_resolve_conflict(action_name(opts));\n \t\tgoto cleanup;\n \t}\n \n-- \n2.37.2.596.g72ccb331cf\n\n"},{"id":"461417","messageId":"e163c87b3efc1571cb3657df6459583af92f9f2b.1660828108.git.git@grubix.eu","threadId":"58298","inReplyTo":"cover.1660828108.git.git@grubix.eu","subject":"[PATCH 4/4] po: adjust README to code","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2022-08-18T13:13:29Z","receivedAt":"2022-08-18T13:14:21Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"When we talk about sequencer action names as such (as opposed to command\nnames) we do translate the action name. Adjust the po README to reflect\nthis and to match the code base.\n\nSigned-off-by: Michael J Gruber <git@grubix.eu>\n---\n po/README.md | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/po/README.md b/po/README.md\nindex 3e4f897d93..90b8455401 100644\n--- a/po/README.md\n+++ b/po/README.md\n@@ -273,7 +273,7 @@ General advice:\n \n   ```c\n   /* TRANSLATORS: %s will be \"revert\" or \"cherry-pick\" */\n-  die(_(\"%s: Unable to write new index file\"), action_name(opts));\n+  die(_(\"%s: Unable to write new index file\"), _(action_name(opts)));\n   ```\n \n We provide wrappers for C, Shell and Perl programs. Here's how they're\n-- \n2.37.2.596.g72ccb331cf\n\n"},{"id":"461428","messageId":"220818.86zgg18umf.gmgdl@evledraar.gmail.com","threadId":"58298","inReplyTo":"ea6c65c254bb08b20ea6c4d81200b847755b555c.1660828108.git.git@grubix.eu","subject":"Re: [PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-08-18T14:55:54Z","receivedAt":"2022-08-18T15:00:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Aug 18 2022, Michael J Gruber wrote:\n\n> Traditionally, reflog messages were never translated, in particular not\n> on storage.\n>\n> Due to the switch of more parts of git to the sequencer, old changes in\n> the sequencer code may lead to recent changes in git's behaviour. E.g.:\n> c28cbc5ea6 (\"sequencer: mark action_name() for translation\", 2016-10-21)\n> marked several uses of `action_name()` for translation. Recently, this\n> lead to a partially translated reflog:\n>\n> `rebase: fast-forward` is translated (e.g. in de to `Rebase: Vorspulen`)\n> whereas other reflog entries such as `rebase (pick):` remain\n> untranslated as they should be.\n>\n> Change the relevant line in the sequencer so that this reflog entry\n> remains untranslated, as well.\n>\n> Signed-off-by: Michael J Gruber <git@grubix.eu>\n> ---\n>  sequencer.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 5f22b7cd37..51d75dfbe1 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -575,7 +575,7 @@ static int fast_forward_to(struct repository *r,\n>  \tif (checkout_fast_forward(r, from, to, 1))\n>  \t\treturn -1; /* the callee should have complained already */\n>  \n> -\tstrbuf_addf(&sb, _(\"%s: fast-forward\"), _(action_name(opts)));\n> +\tstrbuf_addf(&sb, \"%s: fast-forward\", action_name(opts));\n>  \n>  \ttransaction = ref_transaction_begin(&err);\n>  \tif (!transaction ||\n\nI 95% agree with this direction, but the other 5% of me is thinking\n\"isn't this fine then? Let's keep it?\".\n\nI.e. from the very beginning we've really tried not to translate file\nformats and plumbing, to the point of having the (now removed) \"gettext\npoison\" facility to try to smoke out any such cases (but it wouldn't\nhave caught this one).\n\nWe've even done this to the point of not translating things like the\n\"revert\" template, even though that's an entirely \"soft\" file format as\nfar as anyone being able to rely on it goes.\n\nBut reflogs are local-only, if you're using Git in German isn't it\nuseful to you to have this messaging in German too? We don't \"push\" them\naround, and to the extent that there's shared environments they (should)\nensure LC_ALL=C if they care.\n\nOf course more useful would be if we wrote it in some language-agnostic\nformat and changed it on the fly, but perhaps we've inadvertently run an\nexperiment here that's shows us this is fine?\n\nWe do have some translated \"file format\" output already, notable\nwhatever we write into the \"gc.log\". Perhaps we should treat this the\nsame.\n\nI'm *not* noting the other 95% argument(s) for accepting this change,\njust playing devil's advocate for the 5% one :)\n\n\n"},{"id":"461430","messageId":"220818.86v8qp8uid.gmgdl@evledraar.gmail.com","threadId":"58298","inReplyTo":"4684d54aeb3e00c96ba581c824a04e47b7236db7.1660828108.git.git@grubix.eu","subject":"Re: [PATCH 2/4] sequencer: do not translate parameters to error_resolve_conflict()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-08-18T15:01:09Z","receivedAt":"2022-08-18T15:03:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Aug 18 2022, Michael J Gruber wrote:\n\n> `error_resolve_conflict()` checks the untranslated action_name\n> parameter, so pass it as is.\n>\n> Suggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> Signed-off-by: Michael J Gruber <git@grubix.eu>\n> ---\n>  sequencer.c | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 51d75dfbe1..8b32b239b9 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -537,7 +537,7 @@ static struct tree *empty_tree(struct repository *r)\n>  static int error_dirty_index(struct repository *repo, struct replay_opts *opts)\n>  {\n>  \tif (repo_read_index_unmerged(repo))\n> -\t\treturn error_resolve_conflict(_(action_name(opts)));\n> +\t\treturn error_resolve_conflict(action_name(opts));\n>  \n>  \terror(_(\"your local changes would be overwritten by %s.\"),\n>  \t\t_(action_name(opts)));\n> @@ -3753,7 +3753,7 @@ static int do_reset(struct repository *r,\n>  \tinit_checkout_metadata(&unpack_tree_opts.meta, name, &oid, NULL);\n>  \n>  \tif (repo_read_index_unmerged(r)) {\n> -\t\tret = error_resolve_conflict(_(action_name(opts)));\n> +\t\tret = error_resolve_conflict(action_name(opts));\n>  \t\tgoto cleanup;\n>  \t}\n\nPerhaps we should have the error_resolve_conflict() function take a\n\"enum replay_action\" instead? We could just do this more isolated\nchange, but perhaps that \"while-we're-at-it\" would be acceptable to\nreduce the risk of running with this particular set of scissors.\n\nThen we could note in a comment in that function that we do not want to\ntranslate the string we'd get from action_name()...\n"},{"id":"461431","messageId":"220818.86r11d8u8m.gmgdl@evledraar.gmail.com","threadId":"58298","inReplyTo":"e163c87b3efc1571cb3657df6459583af92f9f2b.1660828108.git.git@grubix.eu","subject":"Re: [PATCH 4/4] po: adjust README to code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-08-18T15:03:18Z","receivedAt":"2022-08-18T15:08:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Aug 18 2022, Michael J Gruber wrote:\n\n> When we talk about sequencer action names as such (as opposed to command\n> names) we do translate the action name. Adjust the po README to reflect\n> this and to match the code base.\n>\n> Signed-off-by: Michael J Gruber <git@grubix.eu>\n> ---\n>  po/README.md | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/po/README.md b/po/README.md\n> index 3e4f897d93..90b8455401 100644\n> --- a/po/README.md\n> +++ b/po/README.md\n> @@ -273,7 +273,7 @@ General advice:\n>  \n>    ```c\n>    /* TRANSLATORS: %s will be \"revert\" or \"cherry-pick\" */\n> -  die(_(\"%s: Unable to write new index file\"), action_name(opts));\n> +  die(_(\"%s: Unable to write new index file\"), _(action_name(opts)));\n>    ```\n>  \n>  We provide wrappers for C, Shell and Perl programs. Here's how they're\n\nIs the end-state of this series such that we do that anywhere? Perhaps\nthat's OK for an isolated fix, but it would really be preferred to avoid\nthe \"lego\" with:\n\n\tdie(action == REVERT ? _(\"revert: Unable to write new index file\") : ...);\n\nOr whatever.\n\nThe \"TRANSLATORS\" comment above the example you're modifying is now\ninaccurate, the \"%s\" will *not* be \"revert\" or \"cherry-pick\" in the\npost-image.\n\nI think the right thing here would be to grep our source for TRANSLATORS\ncomments that mention %s and replace this existing example with an\nentirely different one...\n"},{"id":"461433","messageId":"CAA19uiTDeVmUHRVd8JK+qLmwTCN_eiY49yEJERi1mLn9oU4hYA@mail.gmail.com","threadId":"58298","inReplyTo":"220818.86v8qp8uid.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/4] sequencer: do not translate parameters to error_resolve_conflict()","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2022-08-18T15:23:39Z","receivedAt":"2022-08-18T15:23:58Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Am Do., 18. Aug. 2022 um 17:02 Uhr schrieb Ævar Arnfjörð Bjarmason\n<avarab@gmail.com>:\n>\n>\n> On Thu, Aug 18 2022, Michael J Gruber wrote:\n>\n> > `error_resolve_conflict()` checks the untranslated action_name\n> > parameter, so pass it as is.\n> >\n> > Suggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> > Signed-off-by: Michael J Gruber <git@grubix.eu>\n> > ---\n> >  sequencer.c | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/sequencer.c b/sequencer.c\n> > index 51d75dfbe1..8b32b239b9 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -537,7 +537,7 @@ static struct tree *empty_tree(struct repository *r)\n> >  static int error_dirty_index(struct repository *repo, struct replay_opts *opts)\n> >  {\n> >       if (repo_read_index_unmerged(repo))\n> > -             return error_resolve_conflict(_(action_name(opts)));\n> > +             return error_resolve_conflict(action_name(opts));\n> >\n> >       error(_(\"your local changes would be overwritten by %s.\"),\n> >               _(action_name(opts)));\n> > @@ -3753,7 +3753,7 @@ static int do_reset(struct repository *r,\n> >       init_checkout_metadata(&unpack_tree_opts.meta, name, &oid, NULL);\n> >\n> >       if (repo_read_index_unmerged(r)) {\n> > -             ret = error_resolve_conflict(_(action_name(opts)));\n> > +             ret = error_resolve_conflict(action_name(opts));\n> >               goto cleanup;\n> >       }\n>\n> Perhaps we should have the error_resolve_conflict() function take a\n> \"enum replay_action\" instead? We could just do this more isolated\n> change, but perhaps that \"while-we're-at-it\" would be acceptable to\n> reduce the risk of running with this particular set of scissors.\n>\n> Then we could note in a comment in that function that we do not want to\n> translate the string we'd get from action_name()...\n\nRather than setting out to do that, I'd retract 2/3/4 just to get 1\ndone, which was my original motivation ... or switch git to C again as\nI did for a while in the past ...\n"},{"id":"461464","messageId":"xmqqpmgxnvkl.fsf@gitster.g","threadId":"58298","inReplyTo":"CAA19uiTDeVmUHRVd8JK+qLmwTCN_eiY49yEJERi1mLn9oU4hYA@mail.gmail.com","subject":"Re: [PATCH 2/4] sequencer: do not translate parameters to error_resolve_conflict()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-18T20:30:34Z","receivedAt":"2022-08-18T20:31:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@grubix.eu> writes:\n\n> Rather than setting out to do that, I'd retract 2/3/4 just to get 1\n> done, which was my original motivation ... or switch git to C again as\n> I did for a while in the past ...\n\nWhile I found 4/4 a bit questionable, these early three patches\nlooked eminently sensible to me.\n"},{"id":"461465","messageId":"xmqqilmpnvad.fsf@gitster.g","threadId":"58298","inReplyTo":"e163c87b3efc1571cb3657df6459583af92f9f2b.1660828108.git.git@grubix.eu","subject":"Re: [PATCH 4/4] po: adjust README to code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-18T20:36:42Z","receivedAt":"2022-08-18T20:36:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@grubix.eu> writes:\n\n> When we talk about sequencer action names as such (as opposed to command\n> names) we do translate the action name. Adjust the po README to reflect\n> this and to match the code base.\n>\n> Signed-off-by: Michael J Gruber <git@grubix.eu>\n> ---\n>  po/README.md | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/po/README.md b/po/README.md\n> index 3e4f897d93..90b8455401 100644\n> --- a/po/README.md\n> +++ b/po/README.md\n> @@ -273,7 +273,7 @@ General advice:\n>  \n>    ```c\n>    /* TRANSLATORS: %s will be \"revert\" or \"cherry-pick\" */\n> -  die(_(\"%s: Unable to write new index file\"), action_name(opts));\n> +  die(_(\"%s: Unable to write new index file\"), _(action_name(opts)));\n>    ```\n\nWhile \"revert\" and \"cherry-pick\" may have localized words in our po/\ndictionary, the message uses \"%s:\" placeholder to identify the Git\noperation that is reporting the problem, and the way the end-user\nwho is getting the message triggered the Git operation was by\nrunning a subcommand of \"git\", isn't it?  \n\nIsn't it confusing for a user who typed \"git revert\" to see an error\nfrom _(\"revert\")?  _(\"Unable to write new index file\") is perfectly\nfine, though.\n"},{"id":"461511","messageId":"ef8b4536322eebd2bed53157f43349e9158631ae.1660894946.git.git@grubix.eu","threadId":"58298","inReplyTo":"xmqqilmpnvad.fsf@gitster.g","subject":"[PATCH 4/4 v2] sequencer: spell out command names and do not translate them","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2022-08-19T07:50:48Z","receivedAt":"2022-08-19T07:50:57Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"When we talk about sequencer action names as such we do translate the\naction name. In all cases, we talk about the like-named git command\nname, though, which is not translated.\n\nIn order to make the correspondence clearer, reword those error messages\nto use the (untranslated) git command name, and adjust the po README to\nmatch the code base.\n\nSigned-off-by: Michael J Gruber <git@grubix.eu>\n---\nI guess there are two extreme views regarding these cases (in terms of\nhow much to translate) and a few in between. v2 here implements the\none of these. As a result, we don't need to N_()-mark the action names\nany more unless I'm overlooking something. I'm holding this back until\nthe consensus is clear.\n\nOverall, we are not consistent with the prefixes in our error messages\n(command or not) nor the capitalisation. One could say that at the point\nof an error/die worse has gone wrong than the wording, of course ;)\n\n po/README.md | 2 +-\n sequencer.c  | 8 ++++----\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/po/README.md b/po/README.md\nindex 3e4f897d93..7b7ad24412 100644\n--- a/po/README.md\n+++ b/po/README.md\n@@ -273,7 +273,7 @@ General advice:\n \n   ```c\n   /* TRANSLATORS: %s will be \"revert\" or \"cherry-pick\" */\n-  die(_(\"%s: Unable to write new index file\"), action_name(opts));\n+  die(_(\"git %s: unable to write new index file\"), action_name(opts));\n   ```\n \n We provide wrappers for C, Shell and Perl programs. Here's how they're\ndiff --git a/sequencer.c b/sequencer.c\nindex 79dad522f5..c26dc46268 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -539,8 +539,8 @@ static int error_dirty_index(struct repository *repo, struct replay_opts *opts)\n \tif (repo_read_index_unmerged(repo))\n \t\treturn error_resolve_conflict(action_name(opts));\n \n-\terror(_(\"your local changes would be overwritten by %s.\"),\n-\t\t_(action_name(opts)));\n+\terror(_(\"git %s: your local changes would be overwritten\"),\n+\t\taction_name(opts)));\n \n \tif (advice_enabled(ADVICE_COMMIT_BEFORE_MERGE))\n \t\tadvise(_(\"commit your changes or stash them to proceed.\"));\n@@ -725,8 +725,8 @@ static int do_recursive_merge(struct repository *r,\n \t\t * TRANSLATORS: %s will be \"revert\", \"cherry-pick\" or\n \t\t * \"rebase\".\n \t\t */\n-\t\treturn error(_(\"%s: Unable to write new index file\"),\n-\t\t\t_(action_name(opts)));\n+\t\treturn error(_(\"git %s: unable to write new index file\"),\n+\t\t\taction_name(opts));\n \n \tif (!clean)\n \t\tappend_conflicts_hint(r->index, msgbuf,\n-- \n2.37.2.653.g5b2587383a\n\n"},{"id":"461521","messageId":"6oqr69o7-qsps-sr86-o4r9-16r7no9n5424@tzk.qr","threadId":"58298","inReplyTo":"220818.86zgg18umf.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-19T09:25:05Z","receivedAt":"2022-08-19T09:25:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Thu, 18 Aug 2022, Ævar Arnfjörð Bjarmason wrote:\n\n> On Thu, Aug 18 2022, Michael J Gruber wrote:\n>\n> > Traditionally, reflog messages were never translated, in particular not\n> > on storage.\n> >\n> > Due to the switch of more parts of git to the sequencer, old changes in\n> > the sequencer code may lead to recent changes in git's behaviour. E.g.:\n> > c28cbc5ea6 (\"sequencer: mark action_name() for translation\", 2016-10-21)\n> > marked several uses of `action_name()` for translation. Recently, this\n> > lead to a partially translated reflog:\n> >\n> > `rebase: fast-forward` is translated (e.g. in de to `Rebase: Vorspulen`)\n> > whereas other reflog entries such as `rebase (pick):` remain\n> > untranslated as they should be.\n> >\n> > Change the relevant line in the sequencer so that this reflog entry\n> > remains untranslated, as well.\n> >\n> > Signed-off-by: Michael J Gruber <git@grubix.eu>\n> > ---\n> >  sequencer.c | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/sequencer.c b/sequencer.c\n> > index 5f22b7cd37..51d75dfbe1 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -575,7 +575,7 @@ static int fast_forward_to(struct repository *r,\n> >  \tif (checkout_fast_forward(r, from, to, 1))\n> >  \t\treturn -1; /* the callee should have complained already */\n> >\n> > -\tstrbuf_addf(&sb, _(\"%s: fast-forward\"), _(action_name(opts)));\n> > +\tstrbuf_addf(&sb, \"%s: fast-forward\", action_name(opts));\n> >\n> >  \ttransaction = ref_transaction_begin(&err);\n> >  \tif (!transaction ||\n>\n> I 95% agree with this direction, but the other 5% of me is thinking\n> \"isn't this fine then? Let's keep it?\".\n\nNo, it's not fine, we mustn't keep it, because we expect Git itself to\nparse the reflog.\n\nCiao,\nJohannes\n"},{"id":"461522","messageId":"06s6r3s7-27nn-1o9s-1n7p-5413284r8740@tzk.qr","threadId":"58298","inReplyTo":"220818.86v8qp8uid.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/4] sequencer: do not translate parameters to error_resolve_conflict()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-19T09:26:35Z","receivedAt":"2022-08-19T09:26:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Thu, 18 Aug 2022, Ævar Arnfjörð Bjarmason wrote:\n\n> On Thu, Aug 18 2022, Michael J Gruber wrote:\n>\n> > `error_resolve_conflict()` checks the untranslated action_name\n> > parameter, so pass it as is.\n> >\n> > Suggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> > Signed-off-by: Michael J Gruber <git@grubix.eu>\n> > ---\n> >  sequencer.c | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/sequencer.c b/sequencer.c\n> > index 51d75dfbe1..8b32b239b9 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -537,7 +537,7 @@ static struct tree *empty_tree(struct repository *r)\n> >  static int error_dirty_index(struct repository *repo, struct replay_opts *opts)\n> >  {\n> >  \tif (repo_read_index_unmerged(repo))\n> > -\t\treturn error_resolve_conflict(_(action_name(opts)));\n> > +\t\treturn error_resolve_conflict(action_name(opts));\n> >\n> >  \terror(_(\"your local changes would be overwritten by %s.\"),\n> >  \t\t_(action_name(opts)));\n> > @@ -3753,7 +3753,7 @@ static int do_reset(struct repository *r,\n> >  \tinit_checkout_metadata(&unpack_tree_opts.meta, name, &oid, NULL);\n> >\n> >  \tif (repo_read_index_unmerged(r)) {\n> > -\t\tret = error_resolve_conflict(_(action_name(opts)));\n> > +\t\tret = error_resolve_conflict(action_name(opts));\n> >  \t\tgoto cleanup;\n> >  \t}\n>\n> Perhaps we should have the error_resolve_conflict() function take a\n> \"enum replay_action\" instead?\n\nWe could do that. We could also just delete the sequencer code. It's just\nthat both are a bad idea.\n\nCiao,\nJohannes\n"},{"id":"461523","messageId":"o4op5qqo-206p-on30-49q7-n1qp4859q0n7@tzk.qr","threadId":"58298","inReplyTo":"ef8b4536322eebd2bed53157f43349e9158631ae.1660894946.git.git@grubix.eu","subject":"Re: [PATCH 4/4 v2] sequencer: spell out command names and do not translate them","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-19T09:30:31Z","receivedAt":"2022-08-19T09:30:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael & Junio,\n\nOn Fri, 19 Aug 2022, Michael J Gruber wrote:\n\n> When we talk about sequencer action names as such we do translate the\n> action name. In all cases, we talk about the like-named git command\n> name, though, which is not translated.\n>\n> In order to make the correspondence clearer, reword those error messages\n> to use the (untranslated) git command name, and adjust the po README to\n> match the code base.\n>\n> Signed-off-by: Michael J Gruber <git@grubix.eu>\n> ---\n> I guess there are two extreme views regarding these cases (in terms of\n> how much to translate) and a few in between. v2 here implements the\n> one of these. As a result, we don't need to N_()-mark the action names\n> any more unless I'm overlooking something. I'm holding this back until\n> the consensus is clear.\n\nThank you for being careful.\n\nIn general, I would like to leave the decision whether or not to mention\nthe _English_ word for an operation (or whether to treat the error\nmessage's prefix as a short-hand for the Git command) to the l10n\nmaintainer, so that things can be consistent between translations.\n\nCiao,\nDscho\n\n> Overall, we are not consistent with the prefixes in our error messages\n> (command or not) nor the capitalisation. One could say that at the point\n> of an error/die worse has gone wrong than the wording, of course ;)\n>\n>  po/README.md | 2 +-\n>  sequencer.c  | 8 ++++----\n>  2 files changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/po/README.md b/po/README.md\n> index 3e4f897d93..7b7ad24412 100644\n> --- a/po/README.md\n> +++ b/po/README.md\n> @@ -273,7 +273,7 @@ General advice:\n>\n>    ```c\n>    /* TRANSLATORS: %s will be \"revert\" or \"cherry-pick\" */\n> -  die(_(\"%s: Unable to write new index file\"), action_name(opts));\n> +  die(_(\"git %s: unable to write new index file\"), action_name(opts));\n>    ```\n>\n>  We provide wrappers for C, Shell and Perl programs. Here's how they're\n> diff --git a/sequencer.c b/sequencer.c\n> index 79dad522f5..c26dc46268 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -539,8 +539,8 @@ static int error_dirty_index(struct repository *repo, struct replay_opts *opts)\n>  \tif (repo_read_index_unmerged(repo))\n>  \t\treturn error_resolve_conflict(action_name(opts));\n>\n> -\terror(_(\"your local changes would be overwritten by %s.\"),\n> -\t\t_(action_name(opts)));\n> +\terror(_(\"git %s: your local changes would be overwritten\"),\n> +\t\taction_name(opts)));\n>\n>  \tif (advice_enabled(ADVICE_COMMIT_BEFORE_MERGE))\n>  \t\tadvise(_(\"commit your changes or stash them to proceed.\"));\n> @@ -725,8 +725,8 @@ static int do_recursive_merge(struct repository *r,\n>  \t\t * TRANSLATORS: %s will be \"revert\", \"cherry-pick\" or\n>  \t\t * \"rebase\".\n>  \t\t */\n> -\t\treturn error(_(\"%s: Unable to write new index file\"),\n> -\t\t\t_(action_name(opts)));\n> +\t\treturn error(_(\"git %s: unable to write new index file\"),\n> +\t\t\taction_name(opts));\n>\n>  \tif (!clean)\n>  \t\tappend_conflicts_hint(r->index, msgbuf,\n> --\n> 2.37.2.653.g5b2587383a\n>\n>\n"},{"id":"461524","messageId":"2p150404-o6r0-4p10-o0s4-orso00o6n369@tzk.qr","threadId":"58298","inReplyTo":"cover.1660828108.git.git@grubix.eu","subject":"Re: [PATCH 0/4] sequencer: clarify translations","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-19T09:32:14Z","receivedAt":"2022-08-19T09:32:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\nOn Thu, 18 Aug 2022, Michael J Gruber wrote:\n\n> thanks for all your input to my RFC patch. I tried to summarize and pack\n> everything up into this little series.\n>\n> A follow-up could (but does not have to) turn translated action names\n> into untranslated git command names in some places.\n\nThank you for persisting on this, and on behalf of the core Git reviewers\nI would like to apologize for the hornets' nest.\n\nI offer my ACK to this iteration of the patch series, with or without the\nv2 of patch 4/4.\n\nCiao,\nDscho\n"},{"id":"461539","messageId":"CAA19uiQhgxKDM8LJq-os=KuxmwVOt2eJ_pkpj0eLBU=E0MYLRQ@mail.gmail.com","threadId":"58298","inReplyTo":"2p150404-o6r0-4p10-o0s4-orso00o6n369@tzk.qr","subject":"Re: [PATCH 0/4] sequencer: clarify translations","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2022-08-19T10:19:10Z","receivedAt":"2022-08-19T10:19:29Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Am Fr., 19. Aug. 2022 um 11:32 Uhr schrieb Johannes Schindelin\n<Johannes.Schindelin@gmx.de>:\n>\n> Hi Michael,\n>\n> On Thu, 18 Aug 2022, Michael J Gruber wrote:\n>\n> > thanks for all your input to my RFC patch. I tried to summarize and pack\n> > everything up into this little series.\n> >\n> > A follow-up could (but does not have to) turn translated action names\n> > into untranslated git command names in some places.\n>\n> Thank you for persisting on this, and on behalf of the core Git reviewers\n> I would like to apologize for the hornets' nest.\n\nNo need to. After all, this thoroughness is a huge part of what makes\ngit into what it is.\n\nIt's also why I'ven been participating much less: simply for lack of\ntime. That thoroughness takes time, sometimes much more than expected\n(and can be frustrating, even in the very technical-physical meaning).\nBut discussions here are always about the best solution (not \"whose\"\nsolution \"wins\"), and that is why I'm always happy to be back for a\nbit.\n\n> I offer my ACK to this iteration of the patch series, with or without the\n> v2 of patch 4/4.\n\nThanks!\n\nPersonally, I care only about the consistent reflog, which currently\nmeans untranslated. Not that I look at the reflog all the time, but\n...\n\nMichael\n"},{"id":"461556","messageId":"220819.86o7wg6zci.gmgdl@evledraar.gmail.com","threadId":"58298","inReplyTo":"6oqr69o7-qsps-sr86-o4r9-16r7no9n5424@tzk.qr","subject":"Re: [PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-08-19T15:12:43Z","receivedAt":"2022-08-19T15:13:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Aug 19 2022, Johannes Schindelin wrote:\n\n> Hi Ævar,\n>\n> On Thu, 18 Aug 2022, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Thu, Aug 18 2022, Michael J Gruber wrote:\n>>\n>> > Traditionally, reflog messages were never translated, in particular not\n>> > on storage.\n>> >\n>> > Due to the switch of more parts of git to the sequencer, old changes in\n>> > the sequencer code may lead to recent changes in git's behaviour. E.g.:\n>> > c28cbc5ea6 (\"sequencer: mark action_name() for translation\", 2016-10-21)\n>> > marked several uses of `action_name()` for translation. Recently, this\n>> > lead to a partially translated reflog:\n>> >\n>> > `rebase: fast-forward` is translated (e.g. in de to `Rebase: Vorspulen`)\n>> > whereas other reflog entries such as `rebase (pick):` remain\n>> > untranslated as they should be.\n>> >\n>> > Change the relevant line in the sequencer so that this reflog entry\n>> > remains untranslated, as well.\n>> >\n>> > Signed-off-by: Michael J Gruber <git@grubix.eu>\n>> > ---\n>> >  sequencer.c | 2 +-\n>> >  1 file changed, 1 insertion(+), 1 deletion(-)\n>> >\n>> > diff --git a/sequencer.c b/sequencer.c\n>> > index 5f22b7cd37..51d75dfbe1 100644\n>> > --- a/sequencer.c\n>> > +++ b/sequencer.c\n>> > @@ -575,7 +575,7 @@ static int fast_forward_to(struct repository *r,\n>> >  \tif (checkout_fast_forward(r, from, to, 1))\n>> >  \t\treturn -1; /* the callee should have complained already */\n>> >\n>> > -\tstrbuf_addf(&sb, _(\"%s: fast-forward\"), _(action_name(opts)));\n>> > +\tstrbuf_addf(&sb, \"%s: fast-forward\", action_name(opts));\n>> >\n>> >  \ttransaction = ref_transaction_begin(&err);\n>> >  \tif (!transaction ||\n>>\n>> I 95% agree with this direction, but the other 5% of me is thinking\n>> \"isn't this fine then? Let's keep it?\".\n>\n> No, it's not fine, we mustn't keep it, because we expect Git itself to\n> parse the reflog.\n\nDoesn't that also mean that the relevant functionality is now also (and\nstill?) broken on any repository where these translations ended up\non-disk?\n"},{"id":"461585","messageId":"xmqqfshsm8z1.fsf@gitster.g","threadId":"58298","inReplyTo":"06s6r3s7-27nn-1o9s-1n7p-5413284r8740@tzk.qr","subject":"Re: [PATCH 2/4] sequencer: do not translate parameters to error_resolve_conflict()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-19T17:36:18Z","receivedAt":"2022-08-19T17:56:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> Perhaps we should have the error_resolve_conflict() function take a\n>> \"enum replay_action\" instead?\n>\n> We could do that. We could also just delete the sequencer code. It's just\n> that both are a bad idea.\n\nSorry, but I do not quite understand this comment.  You may think\nsome parts of the sequencer code are a bad idea but I think overall\nit is eminently useful and usable enough that it does not make sense\nto \"just delete the sequencer code\"---if there are things we find\nbad ideas in there, we should fix them instead, no?\n\nIn any case, can you keep the conversation more civil?  I have to\nsay that between you two, you may by no means be the only one who is\nunnecessarily abrasive, but if you do not understand why the other\nside suggests a solution you feel you do not like, you can ask more\nconstructively why they think it is a good idea, without assuming\nthat they are doing so only to block you.  Or explain why you think\nit is a bad idea by showing the consequences of their solution, e.g.\n\"there are 20 callsites, among which only 1 has the enum readily\navailable so it would be a lot of churn to give the other 19 the\nenum, even though the error helper may become simpler with a single\nswitch() statement if we allow it to take an enum.\" or something (I\nknow this function is called only from very few places, so 1 out of\n20 is a totally made-up reasoning that would not apply in this case,\nbut you get the idea).\n\nThanks.\n\n"},{"id":"461612","messageId":"xmqq8rnkklon.fsf@gitster.g","threadId":"58298","inReplyTo":"220819.86o7wg6zci.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-19T20:44:40Z","receivedAt":"2022-08-19T20:44:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Doesn't that also mean that the relevant functionality is now also (and\n> still?) broken on any repository where these translations ended up\n> on-disk?\n\nIt may, but the first response to that problem is not to make the\nbreakage in repositires worse by keep adding unparseable data to\nthem.\n"},{"id":"461623","messageId":"220819.864jy853qc.gmgdl@evledraar.gmail.com","threadId":"58298","inReplyTo":"xmqq8rnkklon.fsf@gitster.g","subject":"Re: [PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-08-19T21:13:21Z","receivedAt":"2022-08-19T21:21:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Aug 19 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>> Doesn't that also mean that the relevant functionality is now also (and\n>> still?) broken on any repository where these translations ended up\n>> on-disk?\n>\n> It may, but the first response to that problem is not to make the\n> breakage in repositires worse by keep adding unparseable data to\n> them.\n\n*nod*, but where is that breakage specifically? I don't see where we're\nparsing this message out again. I tried to test it out with the below\n(making the message as un-helpful as possible). All our tests pass, but\nof course our coverage may just be lacking...\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 5f22b7cd377..9e039e26b5a 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -391,19 +391,24 @@ int sequencer_remove_state(struct replay_opts *opts)\n \treturn ret;\n }\n \n-static const char *action_name(const struct replay_opts *opts)\n+static const char *action_name_1(const struct replay_opts *opts, int revert)\n {\n \tswitch (opts->action) {\n \tcase REPLAY_REVERT:\n-\t\treturn N_(\"revert\");\n+\t\treturn revert ? N_(\"trever\") : N_(\"revert\");\n \tcase REPLAY_PICK:\n-\t\treturn N_(\"cherry-pick\");\n+\t\treturn revert ? N_(\"kcip-yrrehc\") : N_(\"cherry-pick\");\n \tcase REPLAY_INTERACTIVE_REBASE:\n-\t\treturn N_(\"rebase\");\n+\t\treturn revert ? N_(\"esaber\") : N_(\"rebase\");\n \t}\n \tdie(_(\"unknown action: %d\"), opts->action);\n }\n \n+static const char *action_name(const struct replay_opts *opts)\n+{\n+\treturn action_name_1(opts, 0);\n+}\n+\n struct commit_message {\n \tchar *parent_label;\n \tchar *label;\n@@ -575,7 +580,7 @@ static int fast_forward_to(struct repository *r,\n \tif (checkout_fast_forward(r, from, to, 1))\n \t\treturn -1; /* the callee should have complained already */\n \n-\tstrbuf_addf(&sb, _(\"%s: fast-forward\"), _(action_name(opts)));\n+\tstrbuf_addf(&sb, _(\"drawrof-tsaf: %s\"), _(action_name_1(opts, 1)));\n \n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n"},{"id":"461634","messageId":"xmqq4jy7kg8e.fsf@gitster.g","threadId":"58298","inReplyTo":"220819.864jy853qc.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-19T22:42:25Z","receivedAt":"2022-08-19T22:42:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Fri, Aug 19 2022, Junio C Hamano wrote:\n>\n>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>>\n>>> Doesn't that also mean that the relevant functionality is now also (and\n>>> still?) broken on any repository where these translations ended up\n>>> on-disk?\n>>\n>> It may, but the first response to that problem is not to make the\n>> breakage in repositires worse by keep adding unparseable data to\n>> them.\n>\n> *nod*, but where is that breakage specifically?\n\nSet your LANG to something other than C and then run \"git reflog\"\nafter running sequencer operations, and you'll see the same breakage\nthat motivated Michael to send this patch set, I think.\n\n"},{"id":"461637","messageId":"220820.86v8qn4xea.gmgdl@evledraar.gmail.com","threadId":"58298","inReplyTo":"xmqq4jy7kg8e.fsf@gitster.g","subject":"Re: [PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-08-19T23:33:56Z","receivedAt":"2022-08-19T23:38:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Aug 19 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>> On Fri, Aug 19 2022, Junio C Hamano wrote:\n>>\n>>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>>>\n>>>> Doesn't that also mean that the relevant functionality is now also (and\n>>>> still?) broken on any repository where these translations ended up\n>>>> on-disk?\n>>>\n>>> It may, but the first response to that problem is not to make the\n>>> breakage in repositires worse by keep adding unparseable data to\n>>> them.\n>>\n>> *nod*, but where is that breakage specifically?\n>\n> Set your LANG to something other than C and then run \"git reflog\"\n> after running sequencer operations, and you'll see the same breakage\n> that motivated Michael to send this patch set, I think.\n\nYes, I can see how and what we write to the reflog. But in order for\nthis to cause anything other than cosmetic breakage we'd need more than\nthat.\n\nOr what do we mean by breakage here?\n\nThat it's broken because we intended for these to be LC_ALL=C, but they\nweren't? Fair enough, but that's got a smaller scope.\n\nOr that it's broken because we expected to not only write \"rebase:\nfast-forward\" into the reflog, but to parse that out again, or to\ne.g. parse the \"rebase\" part of it out as a command-name. I haven't\nfound *those* bits yet.\n\nOf course we also have to worry about third-party software that expected\nLC_ALL=C breaking. I'm just wondering if we have some code in git.git\nthat would also be similarly broken.\n\nBecause if we do it wouldn't be that hard to just hardcode all the\ntranslations we shipped at that time in some array in the C code, and\nnot only parse out a \"rebase: fast-forward\", but also the German\netc. equivalent.\n\n"},{"id":"461652","messageId":"YwChr17RntWnoNok@coredump.intra.peff.net","threadId":"58298","inReplyTo":"220819.864jy853qc.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-08-20T08:56:15Z","receivedAt":"2022-08-20T08:56:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 19, 2022 at 11:13:21PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> \n> On Fri, Aug 19 2022, Junio C Hamano wrote:\n> \n> > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n> >\n> >> Doesn't that also mean that the relevant functionality is now also (and\n> >> still?) broken on any repository where these translations ended up\n> >> on-disk?\n> >\n> > It may, but the first response to that problem is not to make the\n> > breakage in repositires worse by keep adding unparseable data to\n> > them.\n> \n> *nod*, but where is that breakage specifically? I don't see where we're\n> parsing this message out again. I tried to test it out with the below\n> (making the message as un-helpful as possible). All our tests pass, but\n> of course our coverage may just be lacking...\n\nI'm not sure if you mean \"where are we parsing this sequencer message\nspecifically\", or if you're just asking where we parse reflog messages\nat all. If the latter, try interpret_nth_prior_checkout() and its helper\ngrab_nth_branch_switch().\n\nAs far as I know, that's the only one we parse, so the answer for\n_these_ messages is: nowhere.\n\nI'm not sure if you're proposing to leave the \"checkout\" message\nuntranslated, but translate everything else. If so, I'm not sure how I\nfeel about that. On the one hand, it could help people who want the\ntranslation. On the other hand, it sounds like a maintainability\nnightmare. ;)\n\n-Peff\n"},{"id":"461662","messageId":"xmqq5yimipd0.fsf@gitster.g","threadId":"58298","inReplyTo":"YwChr17RntWnoNok@coredump.intra.peff.net","subject":"Re: [PATCH 1/4] sequencer: do not translate reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-20T21:20:27Z","receivedAt":"2022-08-20T21:20:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I'm not sure if you mean \"where are we parsing this sequencer message\n> specifically\", or if you're just asking where we parse reflog messages\n> at all. If the latter, try interpret_nth_prior_checkout() and its helper\n> grab_nth_branch_switch().\n>\n> As far as I know, that's the only one we parse, so the answer for\n> _these_ messages is: nowhere.\n\nUnless translation in some language of these messages looks similar\nto what 'nth-prior' wants to find.  So the answer really is \"asking\nif somebody parses _these_ messages is pointless\" ;-)\n\nI outlined one possible approach to allow translat{able,ed} reflog\nmessages without breaking 'nth-prior' and would allow us add more\ncode to mechanically parse them if we wanted to elsewhere in the\nthread, by the way.  I do not plan to work on it soon, but without\ndoing something like that first, letting translated messages\nrandomly into reflog is asking for trouble, I am afraid.\n"},{"id":"461754","messageId":"oqq42q11-3031-91or-no50-p68q85po1492@tzk.qr","threadId":"58298","inReplyTo":"xmqqfshsm8z1.fsf@gitster.g","subject":"Re: [PATCH 2/4] sequencer: do not translate parameters to error_resolve_conflict()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-22T13:53:56Z","receivedAt":"2022-08-22T13:54:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\n[Michael, I do not consider what I wrote below relevant for your patch\nseries, you may ignore it if you want]\n\nOn Fri, 19 Aug 2022, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> >> Perhaps we should have the error_resolve_conflict() function take a\n> >> \"enum replay_action\" instead?\n> >\n> > We could do that. We could also just delete the sequencer code. It's just\n> > that both are a bad idea.\n>\n> Sorry, but I do not quite understand this comment.\n\nI expected a seasoned reviewer to offer such a suggestion only after\nlooking up (or remembering) how `error_resolve_conflict()` is defined, and\nwhere, and where its callers are.\n\nAfter all, many suggestions that come to mind during a review turn out to\nbe a bad idea when considering them carefully, and if that can be\ndetermined before the mail is sent, everybody wins back some time.\n\nIn this instance, `error_resolve_conflict()` is declared in `advice.h`.\nThe suggestion to use a sequencer-specific data type there sounds...\ncontroversial. But okay, maybe there are good reasons to suggest that.\n\nLet's look at the callers. Two callers in `sequencer.c`. Okay, maybe it\nmakes a bit more sense. But one caller in `advice.c`? Let's dig deeper.\n\nThat caller in `advice.c` is `die_resolve_conflict()`, which is called in\nthe built-ins `commit`, `merge-recursive`, `merge` and `pull`.\n\nThose callers have nothing to do with the sequencer, therefore it is a bad\nidea to suggest using a sequencer-specific data type in that call chain.\n\nFrom my perspective, that is enough to retire the suggestion.\n\nWhen I wrote what I wrote, I thought that it was a pretty quick thing to\ndetermine, so quick that I really expected to not see such a suggestion on\nthe mailing list in the first place.\n\nIn hindsight, I understand that you would have had to look at the code,\nand not just at the patch, to see this. And therefore it is probably not\nquite as obvious as I thought. I did not expect new contributors to be\nable to analyze this quickly, but a Git mailing list regular, yes.\n\nFor my flippant response, I apologize.\n\nAs for the suggestion I criticized: I stand by my assessment. It is not a\ngood idea, and it was not necessary to send it out before doing a cursory\nsanity check. We want code contribution to have a high quality, and the\ncode reviews should meet at least the same bar.\n\nCiao,\nDscho\n\n"},{"id":"461774","messageId":"xmqqv8qkdzpu.fsf@gitster.g","threadId":"58298","inReplyTo":"oqq42q11-3031-91or-no50-p68q85po1492@tzk.qr","subject":"Re: [PATCH 2/4] sequencer: do not translate parameters to error_resolve_conflict()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-22T16:12:29Z","receivedAt":"2022-08-22T16:12:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> ... We want code contribution to have a high quality, and the\n> code reviews should meet at least the same bar.\n\nI like that one.  Ævar is not alone, but many of us often throw an\nunrelated \"observation\" into a review thread that is a total\ntangent.  While I do not think it is necessarily a bad thing,\nbecause these tangential discussions often turn into separate idea\nthat lead to improvements, we should learn to (1) mark a tangent\nclearly as such and (2) keep the quality of the tangent reasonably\nhigh.\n\nThanks.\n"}]}