{"thread":{"id":"60897","subject":"[PATCH v4 1/2] revision: ensure MERGE_HEAD is a ref in prepare_show_merge","startedAt":"2024-02-10T23:35:32Z","lastAt":"2024-03-02T15:35:24Z","messageCount":26,"participants":["Philippe Blain","Johannes Sixt","Phillip Wood","Junio C Hamano","Jean-Noël Avila","phillip.wood123@gmail.com"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"488376","messageId":"20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v4-1-3bc9e62808f4@gmail.com","threadId":"60897","inReplyTo":"20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v4-0-3bc9e62808f4@gmail.com","subject":"[PATCH v4 1/2] revision: ensure MERGE_HEAD is a ref in prepare_show_merge","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-10T23:35:12Z","receivedAt":"2024-02-10T23:35:32Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Michael Lohmann \n\nThis is done to\n(1) ensure MERGE_HEAD is a ref,\n(2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()\n(3) error out when MERGE_HEAD is a symref.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Michael Lohmann \nSigned-off-by: Junio C Hamano \n---\n revision.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 2424c9bd67..aa4c4dc778 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1973,8 +1973,12 @@ static void prepare_show_merge(struct rev_info *revs)\n \tif (repo_get_oid(the_repository, \"HEAD\", &oid))\n \t\tdie(\"--merge without HEAD?\");\n \thead = lookup_commit_or_die(&oid, \"HEAD\");\n-\tif (repo_get_oid(the_repository, \"MERGE_HEAD\", &oid))\n+\tif (read_ref_full(\"MERGE_HEAD\",\n+\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t&oid, NULL))\n \t\tdie(\"--merge without MERGE_HEAD?\");\n+\tif (is_null_oid(&oid))\n+\t\tdie(\"MERGE_HEAD is a symbolic ref???\");\n \tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n \tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n\n-- \n2.39.1\n\n"},{"id":"488377","messageId":"20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v4-0-3bc9e62808f4@gmail.com","threadId":"60897","inReplyTo":"20240117081405.14012-1-mi.al.lohmann@gmail.com","subject":"[PATCH v4 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-10T23:35:11Z","receivedAt":"2024-02-10T23:35:32Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Changes in v4:\n- Added a commit message for 2/2 detailing the use case and summarizing the discussion in the thread\n- Adjusted the documentation of the option\n\nLink to v3: https://lore.kernel.org/r/20240117081405.14012-1-mi.al.lohmann@gmail.com\n\nRange-diff vs v3:\n\n1: 37405be1a3 = 1: 37405be1a3 revision: ensure MERGE_HEAD is a ref in prepare_show_merge\n2: de080a628c ! 2: 6ac1608809 revision: implement `git log --merge` also for rebase/cherry_pick/revert\n @@ Metadata\n Author: Michael Lohmann \n\n ## Commit message ##\n - revision: implement `git log --merge` also for rebase/cherry_pick/revert\n + revision: implement `git log --merge` also for rebase/cherry-pick/revert\n\n + 'git log' learned in ae3e5e1ef2 (git log -p --merge [[--] paths...],\n + 2006-07-03) to show commits touching conflicted files in the range\n + HEAD...MERGE_HEAD, an addition documented in d249b45547 (Document\n + rev-list's option --merge, 2006-08-04).\n +\n + It can be useful to look at the commit history to understand what lead\n + to merge conflicts also for other mergy operations besides merges, like\n + cherry-pick, revert and rebase.\n +\n + For rebases, an interesting range to look at is HEAD...REBASE_HEAD,\n + since the conflicts are usually caused by how the code changed\n + differently on HEAD since REBASE_HEAD forked from it.\n +\n + For cherry-picks and revert, it is less clear that\n + HEAD...CHERRY_PICK_HEAD and HEAD...REVERT_HEAD are indeed interesting\n + ranges, since these commands are about applying or unapplying a single\n + (or a few, for cherry-pick) commit(s) on top of HEAD. However, conflicts\n + encountered during these operations can indeed be caused by changes\n + introduced in preceding commits on both sides of the history.\n +\n + Adjust the code in prepare_show_merge so it constructs the range\n + HEAD...$OTHER for each of OTHER={MERGE_HEAD, CHERRY_PICK_HEAD,\n + REVERT_HEAD or REBASE_HEAD}. Note that we try these pseudorefs in order,\n + so keep REBASE_HEAD last since the three other operations can be\n + performed during a rebase. Note also that in the uncommon case where\n + $OTHER and HEAD do not share a common ancestor, this will show the\n + complete histories of both sides since their root commits, which is the\n + same behaviour as currently happens in that case for HEAD and\n + MERGE_HEAD.\n +\n + Adjust the documentation of this option accordingly.\n +\n + Co-authored-by: Philippe Blain \n Co-authored-by: Johannes Sixt \n + Signed-off-by: Philippe Blain \n Signed-off-by: Michael Lohmann \n [jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\n Signed-off-by: Junio C Hamano \n\n + ## Documentation/gitk.txt ##\n +@@ Documentation/gitk.txt: linkgit:git-rev-list[1] for a complete list.\n +\n + --merge::\n +\n +-\tAfter an attempt to merge stops with conflicts, show the commits on\n +-\tthe history between two branches (i.e. the HEAD and the MERGE_HEAD)\n +-\tthat modify the conflicted files and do not exist on all the heads\n +-\tbeing merged.\n ++\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n ++\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n ++\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n ++\twhen the index has unmerged entries.\n +\n + --left-right::\n +\n +\n + ## Documentation/rev-list-options.txt ##\n +@@ Documentation/rev-list-options.txt: See also linkgit:git-reflog[1].\n + Under `--pretty=reference`, this information will not be shown at all.\n +\n + --merge::\n +-\tAfter a failed merge, show refs that touch files having a\n +-\tconflict and don't exist on all heads to merge.\n ++\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n ++\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n ++\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n ++\twhen the index has unmerged entries.\n +\n + --boundary::\n + \tOutput excluded boundary commits. Boundary commits are\n +\n ## revision.c ##\n @@ revision.c: static void add_pending_commit_list(struct rev_info *revs,\n \t}\n\n---\nMichael Lohmann (2):\n revision: ensure MERGE_HEAD is a ref in prepare_show_merge\n revision: implement `git log --merge` also for rebase/cherry-pick/revert\n\n Documentation/gitk.txt | 8 ++++----\n Documentation/rev-list-options.txt | 6 ++++--\n revision.c | 27 +++++++++++++++++++++++----\n 3 files changed, 31 insertions(+), 10 deletions(-)\n---\nbase-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5\nchange-id: 20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-05bd8e8797db\n\n"},{"id":"488378","messageId":"20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v4-2-3bc9e62808f4@gmail.com","threadId":"60897","inReplyTo":"20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v4-0-3bc9e62808f4@gmail.com","subject":"[PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-10T23:35:13Z","receivedAt":"2024-02-10T23:35:33Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Michael Lohmann \n\n'git log' learned in ae3e5e1ef2 (git log -p --merge [[--] paths...],\n2006-07-03) to show commits touching conflicted files in the range\nHEAD...MERGE_HEAD, an addition documented in d249b45547 (Document\nrev-list's option --merge, 2006-08-04).\n\nIt can be useful to look at the commit history to understand what lead\nto merge conflicts also for other mergy operations besides merges, like\ncherry-pick, revert and rebase.\n\nFor rebases, an interesting range to look at is HEAD...REBASE_HEAD,\nsince the conflicts are usually caused by how the code changed\ndifferently on HEAD since REBASE_HEAD forked from it.\n\nFor cherry-picks and revert, it is less clear that\nHEAD...CHERRY_PICK_HEAD and HEAD...REVERT_HEAD are indeed interesting\nranges, since these commands are about applying or unapplying a single\n(or a few, for cherry-pick) commit(s) on top of HEAD. However, conflicts\nencountered during these operations can indeed be caused by changes\nintroduced in preceding commits on both sides of the history.\n\nAdjust the code in prepare_show_merge so it constructs the range\nHEAD...$OTHER for each of OTHER={MERGE_HEAD, CHERRY_PICK_HEAD,\nREVERT_HEAD or REBASE_HEAD}. Note that we try these pseudorefs in order,\nso keep REBASE_HEAD last since the three other operations can be\nperformed during a rebase. Note also that in the uncommon case where\n$OTHER and HEAD do not share a common ancestor, this will show the\ncomplete histories of both sides since their root commits, which is the\nsame behaviour as currently happens in that case for HEAD and\nMERGE_HEAD.\n\nAdjust the documentation of this option accordingly.\n\nCo-authored-by: Philippe Blain \nCo-authored-by: Johannes Sixt \nSigned-off-by: Philippe Blain \nSigned-off-by: Michael Lohmann \n[jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\nSigned-off-by: Junio C Hamano \n---\n Documentation/gitk.txt | 8 ++++----\n Documentation/rev-list-options.txt | 6 ++++--\n revision.c | 31 +++++++++++++++++++++++--------\n 3 files changed, 31 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\nindex c2213bb77b..80ff4e149a 100644\n--- a/Documentation/gitk.txt\n+++ b/Documentation/gitk.txt\n@@ -63,10 +63,10 @@ linkgit:git-rev-list[1] for a complete list.\n \n --merge::\n \n-\tAfter an attempt to merge stops with conflicts, show the commits on\n-\tthe history between two branches (i.e. the HEAD and the MERGE_HEAD)\n-\tthat modify the conflicted files and do not exist on all the heads\n-\tbeing merged.\n+\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n+\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n+\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n+\twhen the index has unmerged entries.\n \n --left-right::\n \ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 2bf239ff03..5b4672c346 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -341,8 +341,10 @@ See also linkgit:git-reflog[1].\n Under `--pretty=reference`, this information will not be shown at all.\n \n --merge::\n-\tAfter a failed merge, show refs that touch files having a\n-\tconflict and don't exist on all heads to merge.\n+\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n+\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n+\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n+\twhen the index has unmerged entries.\n \n --boundary::\n \tOutput excluded boundary commits. Boundary commits are\ndiff --git a/revision.c b/revision.c\nindex aa4c4dc778..36dc2f94f7 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1961,11 +1961,31 @@ static void add_pending_commit_list(struct rev_info *revs,\n \t}\n }\n \n+static const char *lookup_other_head(struct object_id *oid)\n+{\n+\tint i;\n+\tstatic const char *const other_head[] = {\n+\t\t\"MERGE_HEAD\", \"CHERRY_PICK_HEAD\", \"REVERT_HEAD\", \"REBASE_HEAD\"\n+\t};\n+\n+\tfor (i = 0; i < ARRAY_SIZE(other_head); i++)\n+\t\tif (!read_ref_full(other_head[i],\n+\t\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\toid, NULL)) {\n+\t\t\tif (is_null_oid(oid))\n+\t\t\t\tdie(\"%s is a symbolic ref???\", other_head[i]);\n+\t\t\treturn other_head[i];\n+\t\t}\n+\n+\tdie(\"--merge without MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD?\");\n+}\n+\n static void prepare_show_merge(struct rev_info *revs)\n {\n \tstruct commit_list *bases;\n \tstruct commit *head, *other;\n \tstruct object_id oid;\n+\tconst char *other_name;\n \tconst char **prune = NULL;\n \tint i, prune_num = 1; /* counting terminating NULL */\n \tstruct index_state *istate = revs->repo->index;\n@@ -1973,15 +1993,10 @@ static void prepare_show_merge(struct rev_info *revs)\n \tif (repo_get_oid(the_repository, \"HEAD\", &oid))\n \t\tdie(\"--merge without HEAD?\");\n \thead = lookup_commit_or_die(&oid, \"HEAD\");\n-\tif (read_ref_full(\"MERGE_HEAD\",\n-\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t&oid, NULL))\n-\t\tdie(\"--merge without MERGE_HEAD?\");\n-\tif (is_null_oid(&oid))\n-\t\tdie(\"MERGE_HEAD is a symbolic ref???\");\n-\tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n+\tother_name = lookup_other_head(&oid);\n+\tother = lookup_commit_or_die(&oid, other_name);\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n-\tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n+\tadd_pending_object(revs, &other->object, other_name);\n \tbases = repo_get_merge_bases(the_repository, head, other);\n \tadd_rev_cmdline_list(revs, bases, REV_CMD_MERGE_BASE, UNINTERESTING | BOTTOM);\n \tadd_pending_commit_list(revs, bases, UNINTERESTING | BOTTOM);\n\n-- \n2.39.1\n\n"},{"id":"488383","messageId":"2cf557e9-bf48-4bf3-be24-c1eeaa887418@kdbg.org","threadId":"60897","inReplyTo":"20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v4-2-3bc9e62808f4@gmail.com","subject":"Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-02-11T08:34:29Z","receivedAt":"2024-02-11T08:35:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Thank you for stepping in and resubmitting with an extended commit\nmessage and documentation!\n\nAm 11.02.24 um 00:35 schrieb Philippe Blain:\n> From: Michael Lohmann \n> \n> 'git log' learned in ae3e5e1ef2 (git log -p --merge [[--] paths...],\n> 2006-07-03) to show commits touching conflicted files in the range\n> HEAD...MERGE_HEAD, an addition documented in d249b45547 (Document\n> rev-list's option --merge, 2006-08-04).\n> \n> It can be useful to look at the commit history to understand what lead\n> to merge conflicts also for other mergy operations besides merges, like\n> cherry-pick, revert and rebase.\n> \n> For rebases, an interesting range to look at is HEAD...REBASE_HEAD,\n> since the conflicts are usually caused by how the code changed\n> differently on HEAD since REBASE_HEAD forked from it.\n> \n> For cherry-picks and revert, it is less clear that\n> HEAD...CHERRY_PICK_HEAD and HEAD...REVERT_HEAD are indeed interesting\n> ranges, since these commands are about applying or unapplying a single\n> (or a few, for cherry-pick) commit(s) on top of HEAD. However, conflicts\n> encountered during these operations can indeed be caused by changes\n> introduced in preceding commits on both sides of the history.\n\nI very much agree. Thank you for spelling it out!\n\n> Adjust the code in prepare_show_merge so it constructs the range\n> HEAD...$OTHER for each of OTHER={MERGE_HEAD, CHERRY_PICK_HEAD,\n> REVERT_HEAD or REBASE_HEAD}. Note that we try these pseudorefs in order,\n> so keep REBASE_HEAD last since the three other operations can be\n> performed during a rebase. Note also that in the uncommon case where\n> $OTHER and HEAD do not share a common ancestor, this will show the\n> complete histories of both sides since their root commits, which is the\n> same behaviour as currently happens in that case for HEAD and\n> MERGE_HEAD.\n\nWell explained!\n\n> \n> Adjust the documentation of this option accordingly.\n> \n> Co-authored-by: Philippe Blain \n> Co-authored-by: Johannes Sixt \n> Signed-off-by: Philippe Blain \n> Signed-off-by: Michael Lohmann \n> [jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\n> Signed-off-by: Junio C Hamano \n\nSigned-off-by trailers should occur in temporal order. Therefore, when\nyou pick up a commit and resend it, you should keep existing\nSigned-off-by and add yours last.\n\n> ---\n> Documentation/gitk.txt | 8 ++++----\n> Documentation/rev-list-options.txt | 6 ++++--\n> revision.c | 31 +++++++++++++++++++++++--------\n> 3 files changed, 31 insertions(+), 14 deletions(-)\n> \n> diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\n> index c2213bb77b..80ff4e149a 100644\n> --- a/Documentation/gitk.txt\n> +++ b/Documentation/gitk.txt\n> @@ -63,10 +63,10 @@ linkgit:git-rev-list[1] for a complete list.\n> \n> --merge::\n> \n> -\tAfter an attempt to merge stops with conflicts, show the commits on\n> -\tthe history between two branches (i.e. the HEAD and the MERGE_HEAD)\n> -\tthat modify the conflicted files and do not exist on all the heads\n> -\tbeing merged.\n> +\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n> +\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n> +\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n> +\twhen the index has unmerged entries.\n\nUnfortunately, this patch does not help gitk. Gitk has its own logic to\ntreat --merge and needs its own patch. This hunk should not be part of\nthis patch.\n\n> \n> --left-right::\n> \n> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n> index 2bf239ff03..5b4672c346 100644\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -341,8 +341,10 @@ See also linkgit:git-reflog[1].\n> Under `--pretty=reference`, this information will not be shown at all.\n> \n> --merge::\n> -\tAfter a failed merge, show refs that touch files having a\n> -\tconflict and don't exist on all heads to merge.\n> +\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n> +\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n> +\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n> +\twhen the index has unmerged entries.\n\nGood. I used --left-right to check that the direction is indeed\nHEAD...$OTHER and not $OTHER...HEAD.\n\n-- Hannes\n\n"},{"id":"488398","messageId":"1c258037-cb08-5fbc-d473-743a60cd8eab@gmail.com","threadId":"60897","inReplyTo":"2cf557e9-bf48-4bf3-be24-c1eeaa887418@kdbg.org","subject":"Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-11T16:43:39Z","receivedAt":"2024-02-11T16:43:41Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Johannes,\n\nLe 2024-02-11 à 03:34, Johannes Sixt a écrit :\n\n>> Adjust the documentation of this option accordingly.\n>>\n>> Co-authored-by: Philippe Blain \n>> Co-authored-by: Johannes Sixt \n>> Signed-off-by: Philippe Blain \n>> Signed-off-by: Michael Lohmann \n>> [jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\n>> Signed-off-by: Junio C Hamano \n> \n> Signed-off-by trailers should occur in temporal order. Therefore, when\n> you pick up a commit and resend it, you should keep existing\n> Signed-off-by and add yours last.\n\nThank you, I did not know that. I guess Junio should be kept last though ?\nOr maybe I should remove Junio's sign-off if I send a new version of the \npatch ?\n\nI'll resend with corrected order.\n\n\nBy the way, Michael put you as co-author but did not add your signed-off-by...\n\n\n>> ---\n>> Documentation/gitk.txt | 8 ++++----\n>> Documentation/rev-list-options.txt | 6 ++++--\n>> revision.c | 31 +++++++++++++++++++++++--------\n>> 3 files changed, 31 insertions(+), 14 deletions(-)\n>>\n>> diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\n>> index c2213bb77b..80ff4e149a 100644\n>> --- a/Documentation/gitk.txt\n>> +++ b/Documentation/gitk.txt\n>> @@ -63,10 +63,10 @@ linkgit:git-rev-list[1] for a complete list.\n>> \n>> --merge::\n>> \n>> -\tAfter an attempt to merge stops with conflicts, show the commits on\n>> -\tthe history between two branches (i.e. the HEAD and the MERGE_HEAD)\n>> -\tthat modify the conflicted files and do not exist on all the heads\n>> -\tbeing merged.\n>> +\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n>> +\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n>> +\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n>> +\twhen the index has unmerged entries.\n> \n> Unfortunately, this patch does not help gitk. Gitk has its own logic to\n> treat --merge and needs its own patch. This hunk should not be part of\n> this patch.\n\nAh, you are right. I assumed it just used rev-list under the hood, but it's \nnot the case for this flag. I'll remove that hunk.\n\n\nThanks,\nPhilippe.\n"},{"id":"488406","messageId":"529c7b42-c606-408e-b6a3-fe189c28db9b@kdbg.org","threadId":"60897","inReplyTo":"1c258037-cb08-5fbc-d473-743a60cd8eab@gmail.com","subject":"Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-02-11T17:59:09Z","receivedAt":"2024-02-11T17:59:27Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11.02.24 um 17:43 schrieb Philippe Blain:\n> Hi Johannes,\n> \n> Le 2024-02-11 à 03:34, Johannes Sixt a écrit :\n> \n>>> Adjust the documentation of this option accordingly.\n>>>\n>>> Co-authored-by: Philippe Blain \n>>> Co-authored-by: Johannes Sixt \n>>> Signed-off-by: Philippe Blain \n>>> Signed-off-by: Michael Lohmann \n>>> [jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\n>>> Signed-off-by: Junio C Hamano \n>>\n>> Signed-off-by trailers should occur in temporal order. Therefore, when\n>> you pick up a commit and resend it, you should keep existing\n>> Signed-off-by and add yours last.\n> \n> Thank you, I did not know that. I guess Junio should be kept last though ?\n> Or maybe I should remove Junio's sign-off if I send a new version of the \n> patch ?\n\nYou should *not* remove Junio's Signed-off-by, because the patch went\nthrough his hands before you picked it up. Then you add your own\nsign-off below. Later, Junio will sign it off again.\n\n> I'll resend with corrected order.\n> \n> By the way, Michael put you as co-author but did not add your signed-off-by...\n\nThis is fine and sufficient. Micheal used some of my ideas, but I didn't\ntake part in the patch submission process.\n\n-- Hannes\n\n"},{"id":"488441","messageId":"c5d60b5b-3181-4bb7-a7f8-eb97474526d7@gmail.com","threadId":"60897","inReplyTo":"20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v4-2-3bc9e62808f4@gmail.com","subject":"Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-12T11:02:56Z","receivedAt":"2024-02-12T11:02:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Philippe\n\nOn 10/02/2024 23:35, Philippe Blain wrote:\n> From: Michael Lohmann \n> \n> 'git log' learned in ae3e5e1ef2 (git log -p --merge [[--] paths...],\n> 2006-07-03) to show commits touching conflicted files in the range\n> HEAD...MERGE_HEAD, an addition documented in d249b45547 (Document\n> rev-list's option --merge, 2006-08-04).\n> \n> It can be useful to look at the commit history to understand what lead\n> to merge conflicts also for other mergy operations besides merges, like\n> cherry-pick, revert and rebase.\n> \n> For rebases, an interesting range to look at is HEAD...REBASE_HEAD,\n> since the conflicts are usually caused by how the code changed\n> differently on HEAD since REBASE_HEAD forked from it.\n> \n> For cherry-picks and revert, it is less clear that\n> HEAD...CHERRY_PICK_HEAD and HEAD...REVERT_HEAD are indeed interesting\n> ranges, since these commands are about applying or unapplying a single\n> (or a few, for cherry-pick) commit(s) on top of HEAD. However, conflicts\n> encountered during these operations can indeed be caused by changes\n> introduced in preceding commits on both sides of the history.\n\nI tend to think that there isn't much difference between rebase and \ncherry-pick here - they are both cherry-picking commits and it is \nperfectly possible to rebase a branch onto an unrelated upstream. The \nimportant part for me is that we're showing these commits because even \nthough they aren't part of the 3-way merge they are relevant for \ninvestigating where any merge conflicts come from.\n\nFor revert I'd argue that the only sane use is reverting an ancestor of \nHEAD but maybe I'm missing something. In that case REVERT_HEAD...HEAD is \nthe same as REVERT_HEAD..HEAD so it shows the changes since the commit \nthat is being reverted which will be the ones causing the conflict.\n\n> Adjust the code in prepare_show_merge so it constructs the range\n> HEAD...$OTHER for each of OTHER={MERGE_HEAD, CHERRY_PICK_HEAD,\n> REVERT_HEAD or REBASE_HEAD}. Note that we try these pseudorefs in order,\n> so keep REBASE_HEAD last since the three other operations can be\n> performed during a rebase. Note also that in the uncommon case where\n> $OTHER and HEAD do not share a common ancestor, this will show the\n> complete histories of both sides since their root commits, which is the\n> same behaviour as currently happens in that case for HEAD and\n> MERGE_HEAD.\n> \n> Adjust the documentation of this option accordingly.\n\nThanks for the comprehensive commit message.\n\n> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n> index 2bf239ff03..5b4672c346 100644\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -341,8 +341,10 @@ See also linkgit:git-reflog[1].\n> Under `--pretty=reference`, this information will not be shown at all.\n> \n> --merge::\n> -\tAfter a failed merge, show refs that touch files having a\n> -\tconflict and don't exist on all heads to merge.\n> +\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n> +\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n> +\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n> +\twhen the index has unmerged entries.\n\nDo you know what \"and don't exist on all heads to merge\" in the original \nis referring to? The new text doesn't mention anything that sounds like \nthat but I don't understand what the original was trying to say.\n\nIt might be worth adding a sentence explaining when this option is useful.\n\n This option can be used to show the commits that are relevant\n when resolving conflicts from a 3-way merge\n\nor something like that.\n\n> --boundary::\n> \tOutput excluded boundary commits. Boundary commits are\n> diff --git a/revision.c b/revision.c\n> index aa4c4dc778..36dc2f94f7 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1961,11 +1961,31 @@ static void add_pending_commit_list(struct rev_info *revs,\n> \t}\n> }\n> \n> +static const char *lookup_other_head(struct object_id *oid)\n> +{\n> +\tint i;\n> +\tstatic const char *const other_head[] = {\n> +\t\t\"MERGE_HEAD\", \"CHERRY_PICK_HEAD\", \"REVERT_HEAD\", \"REBASE_HEAD\"\n> +\t};\n> +\n> +\tfor (i = 0; i < ARRAY_SIZE(other_head); i++)\n> +\t\tif (!read_ref_full(other_head[i],\n> +\t\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> +\t\t\t\toid, NULL)) {\n> +\t\t\tif (is_null_oid(oid))\n> +\t\t\t\tdie(\"%s is a symbolic ref???\", other_head[i]);\n\nThis would benefit from being translated and I think one '?' would \nsuffice (I'm not sure we even need that - are there other possible \ncauses of a null oid here?)\n\n> +\t\t\treturn other_head[i];\n> +\t\t}\n> +\n> +\tdie(\"--merge without MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD?\");\n\nThis is not a question and would also benefit from translation. It might \nbe more helpful to say that \"--merge\" requires one of those pseudorefs.\n\nThanks for pick this series up and polishing it\n\nPhillip\n\n"},{"id":"488461","messageId":"xmqqmss55yhv.fsf@gitster.g","threadId":"60897","inReplyTo":"529c7b42-c606-408e-b6a3-fe189c28db9b@kdbg.org","subject":"Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-12T18:27:40Z","receivedAt":"2024-02-12T18:27:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 11.02.24 um 17:43 schrieb Philippe Blain:\n>> Hi Johannes,\n>> \n>> Le 2024-02-11 à 03:34, Johannes Sixt a écrit :\n>> \n>>>> Adjust the documentation of this option accordingly.\n>>>>\n>>>> Co-authored-by: Philippe Blain \n>>>> Co-authored-by: Johannes Sixt \n>>>> Signed-off-by: Philippe Blain \n>>>> Signed-off-by: Michael Lohmann \n>>>> [jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\n>>>> Signed-off-by: Junio C Hamano \n>>>\n>>> Signed-off-by trailers should occur in temporal order. Therefore, when\n>>> you pick up a commit and resend it, you should keep existing\n>>> Signed-off-by and add yours last.\n>> \n>> Thank you, I did not know that. I guess Junio should be kept last though ?\n>> Or maybe I should remove Junio's sign-off if I send a new version of the \n>> patch ?\n>\n> You should *not* remove Junio's Signed-off-by, because the patch went\n> through his hands before you picked it up. Then you add your own\n> sign-off below. Later, Junio will sign it off again.\n\nIn the meantime, this is how I tweaked while queuing.\n\n Co-authored-by: Johannes Sixt \n Signed-off-by: Michael Lohmann \n [jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\n Signed-off-by: Junio C Hamano \n [pb: greatly enhanced the log message]\n Signed-off-by: Philippe Blain \n Signed-off-by: Junio C Hamano \n"},{"id":"488508","messageId":"7a2a0ed5-f9dc-42dd-886b-457641b9bc79@gmail.com","threadId":"60897","inReplyTo":"20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v4-2-3bc9e62808f4@gmail.com","subject":"Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Jean-Noël Avila","fromEmail":"avila.jn@gmail.com","sentAt":"2024-02-13T08:33:51Z","receivedAt":"2024-02-13T08:33:54Z","isPatch":true,"sender":{"key":"jn.avila@free.fr","avatar":"https://avatars.githubusercontent.com/u/156172?v=4"},"body":"Le 11/02/2024 à 00:35, Philippe Blain a écrit :\n> From: Michael Lohmann \n> \n> 'git log' learned in ae3e5e1ef2 (git log -p --merge [[--] paths...],\n> 2006-07-03) to show commits touching conflicted files in the range\n> HEAD...MERGE_HEAD, an addition documented in d249b45547 (Document\n> rev-list's option --merge, 2006-08-04).\n> \n> It can be useful to look at the commit history to understand what lead\n> to merge conflicts also for other mergy operations besides merges, like\n> cherry-pick, revert and rebase.\n> \n> For rebases, an interesting range to look at is HEAD...REBASE_HEAD,\n> since the conflicts are usually caused by how the code changed\n> differently on HEAD since REBASE_HEAD forked from it.\n> \n> For cherry-picks and revert, it is less clear that\n> HEAD...CHERRY_PICK_HEAD and HEAD...REVERT_HEAD are indeed interesting\n> ranges, since these commands are about applying or unapplying a single\n> (or a few, for cherry-pick) commit(s) on top of HEAD. However, conflicts\n> encountered during these operations can indeed be caused by changes\n> introduced in preceding commits on both sides of the history.\n> \n> Adjust the code in prepare_show_merge so it constructs the range\n> HEAD...$OTHER for each of OTHER={MERGE_HEAD, CHERRY_PICK_HEAD,\n> REVERT_HEAD or REBASE_HEAD}. Note that we try these pseudorefs in order,\n> so keep REBASE_HEAD last since the three other operations can be\n> performed during a rebase. Note also that in the uncommon case where\n> $OTHER and HEAD do not share a common ancestor, this will show the\n> complete histories of both sides since their root commits, which is the\n> same behaviour as currently happens in that case for HEAD and\n> MERGE_HEAD.\n> \n> Adjust the documentation of this option accordingly.\n> \n> Co-authored-by: Philippe Blain \n> Co-authored-by: Johannes Sixt \n> Signed-off-by: Philippe Blain \n> Signed-off-by: Michael Lohmann \n> [jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/gitk.txt | 8 ++++----\n> Documentation/rev-list-options.txt | 6 ++++--\n> revision.c | 31 +++++++++++++++++++++++--------\n> 3 files changed, 31 insertions(+), 14 deletions(-)\n> \n> diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\n> index c2213bb77b..80ff4e149a 100644\n> --- a/Documentation/gitk.txt\n> +++ b/Documentation/gitk.txt\n> @@ -63,10 +63,10 @@ linkgit:git-rev-list[1] for a complete list.\n> \n> --merge::\n> \n> -\tAfter an attempt to merge stops with conflicts, show the commits on\n> -\tthe history between two branches (i.e. the HEAD and the MERGE_HEAD)\n> -\tthat modify the conflicted files and do not exist on all the heads\n> -\tbeing merged.\n> +\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n\nif $OTHER is a placeholder, why not use the placeholder notation \ninstead of a notation that could deceive the reader into thinking that \nthis is an actual environment variable?\n\n> +\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n> +\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n> +\twhen the index has unmerged entries.\n> \n\nThanks\n\n"},{"id":"488527","messageId":"8384d1dc-b6c4-b853-9bf6-3d7ccee86d12@gmail.com","threadId":"60897","inReplyTo":"7a2a0ed5-f9dc-42dd-886b-457641b9bc79@gmail.com","subject":"Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-13T13:14:55Z","receivedAt":"2024-02-13T13:14:57Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Jean-Nöel,\n\nLe 2024-02-13 à 03:33, Jean-Noël Avila a écrit :\n> Le 11/02/2024 à 00:35, Philippe Blain a écrit :\n>> From: Michael Lohmann \n>>\n>>\n>> diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\n>> index c2213bb77b..80ff4e149a 100644\n>> --- a/Documentation/gitk.txt\n>> +++ b/Documentation/gitk.txt\n>> @@ -63,10 +63,10 @@ linkgit:git-rev-list[1] for a complete list.\n>>     --merge::\n>>   -    After an attempt to merge stops with conflicts, show the commits on\n>> -    the history between two branches (i.e. the HEAD and the MERGE_HEAD)\n>> -    that modify the conflicted files and do not exist on all the heads\n>> -    being merged.\n>> +    Show commits touching conflicted paths in the range `HEAD...$OTHER`,\n> \n> if $OTHER is a placeholder, why not use the placeholder notation \n> instead of a notation that could deceive the reader into thinking that \n> this is an actual environment variable?\n\nGood point, I'll make that change.\nThanks!\nPhilippe.\n\n\n"},{"id":"488528","messageId":"790a3f11-5a8c-42f2-7a35-f2900c0299b4@gmail.com","threadId":"60897","inReplyTo":"c5d60b5b-3181-4bb7-a7f8-eb97474526d7@gmail.com","subject":"Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-13T13:27:20Z","receivedAt":"2024-02-13T13:27:22Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Phillip,\n\nLe 2024-02-12 à 06:02, Phillip Wood a écrit :\n> Hi Philippe\n> \n> On 10/02/2024 23:35, Philippe Blain wrote:\n>> From: Michael Lohmann \n>>\n>> 'git log' learned in ae3e5e1ef2 (git log -p --merge [[--] paths...],\n>> 2006-07-03) to show commits touching conflicted files in the range\n>> HEAD...MERGE_HEAD, an addition documented in d249b45547 (Document\n>> rev-list's option --merge, 2006-08-04).\n>>\n>> It can be useful to look at the commit history to understand what lead\n>> to merge conflicts also for other mergy operations besides merges, like\n>> cherry-pick, revert and rebase.\n>>\n>> For rebases, an interesting range to look at is HEAD...REBASE_HEAD,\n>> since the conflicts are usually caused by how the code changed\n>> differently on HEAD since REBASE_HEAD forked from it.\n>>\n>> For cherry-picks and revert, it is less clear that\n>> HEAD...CHERRY_PICK_HEAD and HEAD...REVERT_HEAD are indeed interesting\n>> ranges, since these commands are about applying or unapplying a single\n>> (or a few, for cherry-pick) commit(s) on top of HEAD. However, conflicts\n>> encountered during these operations can indeed be caused by changes\n>> introduced in preceding commits on both sides of the history.\n> \n> I tend to think that there isn't much difference between rebase and cherry-pick here - they are both cherry-picking commits and it is perfectly possible to rebase a branch onto an unrelated upstream. The important part for me is that we're showing these commits because even though they aren't part of the 3-way merge they are relevant for investigating where any merge conflicts come from.\n> \n> For revert I'd argue that the only sane use is reverting an ancestor of HEAD but maybe I'm missing something. In that case REVERT_HEAD...HEAD is the same as REVERT_HEAD..HEAD so it shows the changes since the commit that is being reverted which will be the ones causing the conflict.\n\nThanks, I can rework the wording from that angle.\n\n\n>> Adjust the code in prepare_show_merge so it constructs the range\n>> HEAD...$OTHER for each of OTHER={MERGE_HEAD, CHERRY_PICK_HEAD,\n>> REVERT_HEAD or REBASE_HEAD}. Note that we try these pseudorefs in order,\n>> so keep REBASE_HEAD last since the three other operations can be\n>> performed during a rebase. Note also that in the uncommon case where\n>> $OTHER and HEAD do not share a common ancestor, this will show the\n>> complete histories of both sides since their root commits, which is the\n>> same behaviour as currently happens in that case for HEAD and\n>> MERGE_HEAD.\n>>\n>> Adjust the documentation of this option accordingly.\n> \n> Thanks for the comprehensive commit message.\n> \n>> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n>> index 2bf239ff03..5b4672c346 100644\n>> --- a/Documentation/rev-list-options.txt\n>> +++ b/Documentation/rev-list-options.txt\n>> @@ -341,8 +341,10 @@ See also linkgit:git-reflog[1].\n>>   Under `--pretty=reference`, this information will not be shown at all.\n>>     --merge::\n>> -    After a failed merge, show refs that touch files having a\n>> -    conflict and don't exist on all heads to merge.\n>> +    Show commits touching conflicted paths in the range `HEAD...$OTHER`,\n>> +    where `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n>> +    `CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n>> +    when the index has unmerged entries.\n> \n> Do you know what \"and don't exist on all heads to merge\" in the original is referring to? The new text doesn't mention anything that sounds like that but I don't understand what the original was trying to say.\n\nYes, it took me a while to understand what that meant. I think it is simply\ndescribing the range of commits shown. If we substitute \"refs\" for \"commits\"\nand switch the order of the sentence, it reads:\n\n After a failed merge, show commits that don't exist on all heads to merge\n and that touch files having a conflict.\n\nSo it's just describing (a bit awkwardly) the HEAD...MERGE_HEAD range.\n\n> It might be worth adding a sentence explaining when this option is useful.\n> \n>     This option can be used to show the commits that are relevant\n>     when resolving conflicts from a 3-way merge\n> \n> or something like that.\n\nNice idea, I'll add that.\n\n> \n>>   --boundary::\n>>       Output excluded boundary commits. Boundary commits are\n>> diff --git a/revision.c b/revision.c\n>> index aa4c4dc778..36dc2f94f7 100644\n>> --- a/revision.c\n>> +++ b/revision.c\n>> @@ -1961,11 +1961,31 @@ static void add_pending_commit_list(struct rev_info *revs,\n>>       }\n>>   }\n>>   +static const char *lookup_other_head(struct object_id *oid)\n>> +{\n>> +    int i;\n>> +    static const char *const other_head[] = {\n>> +        \"MERGE_HEAD\", \"CHERRY_PICK_HEAD\", \"REVERT_HEAD\", \"REBASE_HEAD\"\n>> +    };\n>> +\n>> +    for (i = 0; i < ARRAY_SIZE(other_head); i++)\n>> +        if (!read_ref_full(other_head[i],\n>> +                RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n>> +                oid, NULL)) {\n>> +            if (is_null_oid(oid))\n>> +                die(\"%s is a symbolic ref???\", other_head[i]);\n> \n> This would benefit from being translated and I think one '?' would suffice (I'm not sure we even need that - are there other possible causes of a null oid here?)\n\nThis bit was suggested by Junio upthread in .\nI'm not sure if the are other causes of null oid, as I don't know well this \npart of the code.\nI agree that a single '?' would be enough, but I'm not sure about marking\nthis for translation, I think maybe this situation would be best handled with\nBUG() ?\n\n>> +            return other_head[i];\n>> +        }\n>> +\n>> +    die(\"--merge without MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD?\");\n> \n> This is not a question and would also benefit from translation. It might be more helpful to say that \"--merge\" requires one of those pseudorefs.\n\nYes, I agree. I'll tweak that.\n\n> Thanks for pick this series up and polishing it\n> \n> Phillip\n> \n\nThanks,\n\nPhilippe.\n"},{"id":"488634","messageId":"568f9c07-0b2c-4d4f-9d35-aeeb6e1e29c4@gmail.com","threadId":"60897","inReplyTo":"790a3f11-5a8c-42f2-7a35-f2900c0299b4@gmail.com","subject":"Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-14T11:02:35Z","receivedAt":"2024-02-14T11:02:36Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Philippe\n\nOn 13/02/2024 13:27, Philippe Blain wrote:\n> Le 2024-02-12 à 06:02, Phillip Wood a écrit :\n>> Hi Philippe\n>> On 10/02/2024 23:35, Philippe Blain wrote:\n>>> From: Michael Lohmann \n>>> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n>>> index 2bf239ff03..5b4672c346 100644\n>>> --- a/Documentation/rev-list-options.txt\n>>> +++ b/Documentation/rev-list-options.txt\n>>> @@ -341,8 +341,10 @@ See also linkgit:git-reflog[1].\n>>>   Under `--pretty=reference`, this information will not be shown at all.\n>>>     --merge::\n>>> -    After a failed merge, show refs that touch files having a\n>>> -    conflict and don't exist on all heads to merge.\n>>> +    Show commits touching conflicted paths in the range `HEAD...$OTHER`,\n>>> +    where `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n>>> +    `CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n>>> +    when the index has unmerged entries.\n>>\n>> Do you know what \"and don't exist on all heads to merge\" in the original \n> is referring to? The new text doesn't mention anything that sounds like \n>that but I don't understand what the original was trying to say.\n> \n> Yes, it took me a while to understand what that meant. I think it is simply\n> describing the range of commits shown. If we substitute \"refs\" for \"commits\"\n> and switch the order of the sentence, it reads:\n> \n> After a failed merge, show commits that don't exist on all heads to merge\n> and that touch files having a conflict.\n> \n> So it's just describing (a bit awkwardly) the HEAD...MERGE_HEAD range.\n\nAh, that makes sense, thanks for explaining\n\n\n>>> +    for (i = 0; i < ARRAY_SIZE(other_head); i++)\n>>> +        if (!read_ref_full(other_head[i],\n>>> +                RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n>>> +                oid, NULL)) {\n>>> +            if (is_null_oid(oid))\n>>> +                die(\"%s is a symbolic ref???\", other_head[i]);\n>>\n>> This would benefit from being translated and I think one '?' would suffice \n>> (I'm not sure we even need that - are there other possible causes of a null \n>> oid here?)\n> \n> This bit was suggested by Junio upthread in .\n> I'm not sure if the are other causes of null oid, as I don't know well this\n> part of the code.\n> I agree that a single '?' would be enough, but I'm not sure about marking\n> this for translation, I think maybe this situation would be best handled with\n> BUG() ?\n\nI think it would be a bug for git to create MERGE_HEAD as a symbolic ref \nbut when we read MERGE_HEAD and find it is a symbolic ref we don't know \nif git created it or some third-party script so I think we should just \nreport an error.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"489332","messageId":"20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-0-af1ef2d9e44d@gmail.com","threadId":"60897","inReplyTo":"20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v4-0-3bc9e62808f4@gmail.com","subject":"[PATCH v5 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-25T21:56:15Z","receivedAt":"2024-02-25T21:56:37Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Changes in v5:\n- Marked error messages for translation and tweaked them as suggested by Phillip\n- Reworded the message of 2/2 as suggested by Phillip\n- Removed the change to gitk's doc in 2/2 as pointed out by Johannes\n- Fixed the trailers in 2/2\n- Improved the doc in 2/2 as suggested by Phillip and Jean-Noël\n\nChanges in v4:\n- Added a commit message for 2/2 detailing the use case and summarizing the discussion in the thread\n- Adjusted the documentation of the option\n\n---\nMichael Lohmann (2):\n revision: ensure MERGE_HEAD is a ref in prepare_show_merge\n revision: implement `git log --merge` also for rebase/cherry-pick/revert\n\n Documentation/rev-list-options.txt | 7 +++++--\n revision.c | 27 +++++++++++++++++++++++----\n 2 files changed, 28 insertions(+), 6 deletions(-)\n---\nbase-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5\nchange-id: 20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-05bd8e8797db\n\nRange-diff versus v4:\n\n1: 37405be1a3 ! 1: c9536431d1 revision: ensure MERGE_HEAD is a ref in prepare_show_merge\n @@ Commit message\n Helped-by: Junio C Hamano \n Signed-off-by: Michael Lohmann \n Signed-off-by: Junio C Hamano \n + Signed-off-by: Philippe Blain \n \n ## revision.c ##\n @@ revision.c: static void prepare_show_merge(struct rev_info *revs)\n @@ revision.c: static void prepare_show_merge(struct rev_info *revs)\n +\t\t\t&oid, NULL))\n \t\tdie(\"--merge without MERGE_HEAD?\");\n +\tif (is_null_oid(&oid))\n -+\t\tdie(\"MERGE_HEAD is a symbolic ref???\");\n ++\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n \tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n \tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n2: 6ac1608809 ! 2: 1641c4be81 revision: implement `git log --merge` also for rebase/cherry-pick/revert\n @@ Commit message\n to merge conflicts also for other mergy operations besides merges, like\n cherry-pick, revert and rebase.\n \n - For rebases, an interesting range to look at is HEAD...REBASE_HEAD,\n - since the conflicts are usually caused by how the code changed\n - differently on HEAD since REBASE_HEAD forked from it.\n + For rebases and cherry-picks, an interesting range to look at is\n + HEAD...{REBASE_HEAD,CHERRY_PICK_HEAD}, since even if all the commits\n + included in that range are not directly part of the 3-way merge,\n + conflicts encountered during these operations can indeed be caused by\n + changes introduced in preceding commits on both sides of the history.\n \n - For cherry-picks and revert, it is less clear that\n - HEAD...CHERRY_PICK_HEAD and HEAD...REVERT_HEAD are indeed interesting\n - ranges, since these commands are about applying or unapplying a single\n - (or a few, for cherry-pick) commit(s) on top of HEAD. However, conflicts\n - encountered during these operations can indeed be caused by changes\n - introduced in preceding commits on both sides of the history.\n + For revert, as we are (most likely) reversing changes from a previous\n + commit, an appropriate range is REVERT_HEAD..HEAD, which is equivalent\n + to REVERT_HEAD...HEAD and to HEAD...REVERT_HEAD, if we keep HEAD and its\n + parents on the left side of the range.\n \n - Adjust the code in prepare_show_merge so it constructs the range\n - HEAD...$OTHER for each of OTHER={MERGE_HEAD, CHERRY_PICK_HEAD,\n - REVERT_HEAD or REBASE_HEAD}. Note that we try these pseudorefs in order,\n - so keep REBASE_HEAD last since the three other operations can be\n - performed during a rebase. Note also that in the uncommon case where\n - $OTHER and HEAD do not share a common ancestor, this will show the\n - complete histories of both sides since their root commits, which is the\n - same behaviour as currently happens in that case for HEAD and\n - MERGE_HEAD.\n + As such, adjust the code in prepare_show_merge so it constructs the\n + range HEAD...$OTHER for OTHER={MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD\n + or REBASE_HEAD}. Note that we try these pseudorefs in order, so keep\n + REBASE_HEAD last since the three other operations can be performed\n + during a rebase. Note also that in the uncommon case where $OTHER and\n + HEAD do not share a common ancestor, this will show the complete\n + histories of both sides since their root commits, which is the same\n + behaviour as currently happens in that case for HEAD and MERGE_HEAD.\n \n Adjust the documentation of this option accordingly.\n \n - Co-authored-by: Philippe Blain \n Co-authored-by: Johannes Sixt \n - Signed-off-by: Philippe Blain \n + Co-authored-by: Philippe Blain \n Signed-off-by: Michael Lohmann \n [jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\n Signed-off-by: Junio C Hamano \n -\n - ## Documentation/gitk.txt ##\n -@@ Documentation/gitk.txt: linkgit:git-rev-list[1] for a complete list.\n - \n - --merge::\n - \n --\tAfter an attempt to merge stops with conflicts, show the commits on\n --\tthe history between two branches (i.e. the HEAD and the MERGE_HEAD)\n --\tthat modify the conflicted files and do not exist on all the heads\n --\tbeing merged.\n -+\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n -+\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n -+\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n -+\twhen the index has unmerged entries.\n - \n - --left-right::\n - \n + Signed-off-by: Philippe Blain \n \n ## Documentation/rev-list-options.txt ##\n @@ Documentation/rev-list-options.txt: See also linkgit:git-reflog[1].\n @@ Documentation/rev-list-options.txt: See also linkgit:git-reflog[1].\n --merge::\n -\tAfter a failed merge, show refs that touch files having a\n -\tconflict and don't exist on all heads to merge.\n -+\tShow commits touching conflicted paths in the range `HEAD...$OTHER`,\n -+\twhere `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,\n ++\tShow commits touching conflicted paths in the range `HEAD...`,\n ++\twhere `` is the first existing pseudoref in `MERGE_HEAD`,\n +\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n -+\twhen the index has unmerged entries.\n ++\twhen the index has unmerged entries. This option can be used to show\n ++\trelevant commits when resolving conflicts from a 3-way merge.\n \n --boundary::\n \tOutput excluded boundary commits. Boundary commits are\n @@ revision.c: static void add_pending_commit_list(struct rev_info *revs,\n +\t\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n +\t\t\t\toid, NULL)) {\n +\t\t\tif (is_null_oid(oid))\n -+\t\t\t\tdie(\"%s is a symbolic ref???\", other_head[i]);\n ++\t\t\t\tdie(_(\"%s is a symbolic ref?\"), other_head[i]);\n +\t\t\treturn other_head[i];\n +\t\t}\n +\n -+\tdie(\"--merge without MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD?\");\n ++\tdie(_(\"--merge requires one of the pseudorefs MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD\"));\n +}\n +\n static void prepare_show_merge(struct rev_info *revs)\n @@ revision.c: static void prepare_show_merge(struct rev_info *revs)\n -\t\t\t&oid, NULL))\n -\t\tdie(\"--merge without MERGE_HEAD?\");\n -\tif (is_null_oid(&oid))\n --\t\tdie(\"MERGE_HEAD is a symbolic ref???\");\n +-\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n -\tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n +\tother_name = lookup_other_head(&oid);\n +\tother = lookup_commit_or_die(&oid, other_name);\n\n"},{"id":"489333","messageId":"20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-1-af1ef2d9e44d@gmail.com","threadId":"60897","inReplyTo":"20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-0-af1ef2d9e44d@gmail.com","subject":"[PATCH v5 1/2] revision: ensure MERGE_HEAD is a ref in prepare_show_merge","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-25T21:56:16Z","receivedAt":"2024-02-25T21:56:38Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Michael Lohmann \n\nThis is done to\n(1) ensure MERGE_HEAD is a ref,\n(2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()\n(3) error out when MERGE_HEAD is a symref.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Michael Lohmann \nSigned-off-by: Junio C Hamano \nSigned-off-by: Philippe Blain \n---\n revision.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 2424c9bd67..ee26988cc6 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1973,8 +1973,12 @@ static void prepare_show_merge(struct rev_info *revs)\n \tif (repo_get_oid(the_repository, \"HEAD\", &oid))\n \t\tdie(\"--merge without HEAD?\");\n \thead = lookup_commit_or_die(&oid, \"HEAD\");\n-\tif (repo_get_oid(the_repository, \"MERGE_HEAD\", &oid))\n+\tif (read_ref_full(\"MERGE_HEAD\",\n+\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t&oid, NULL))\n \t\tdie(\"--merge without MERGE_HEAD?\");\n+\tif (is_null_oid(&oid))\n+\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n \tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n \tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n\n-- \n2.39.1\n\n"},{"id":"489334","messageId":"20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-2-af1ef2d9e44d@gmail.com","threadId":"60897","inReplyTo":"20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-0-af1ef2d9e44d@gmail.com","subject":"[PATCH v5 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-25T21:56:17Z","receivedAt":"2024-02-25T21:56:39Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Michael Lohmann \n\n'git log' learned in ae3e5e1ef2 (git log -p --merge [[--] paths...],\n2006-07-03) to show commits touching conflicted files in the range\nHEAD...MERGE_HEAD, an addition documented in d249b45547 (Document\nrev-list's option --merge, 2006-08-04).\n\nIt can be useful to look at the commit history to understand what lead\nto merge conflicts also for other mergy operations besides merges, like\ncherry-pick, revert and rebase.\n\nFor rebases and cherry-picks, an interesting range to look at is\nHEAD...{REBASE_HEAD,CHERRY_PICK_HEAD}, since even if all the commits\nincluded in that range are not directly part of the 3-way merge,\nconflicts encountered during these operations can indeed be caused by\nchanges introduced in preceding commits on both sides of the history.\n\nFor revert, as we are (most likely) reversing changes from a previous\ncommit, an appropriate range is REVERT_HEAD..HEAD, which is equivalent\nto REVERT_HEAD...HEAD and to HEAD...REVERT_HEAD, if we keep HEAD and its\nparents on the left side of the range.\n\nAs such, adjust the code in prepare_show_merge so it constructs the\nrange HEAD...$OTHER for OTHER={MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD\nor REBASE_HEAD}. Note that we try these pseudorefs in order, so keep\nREBASE_HEAD last since the three other operations can be performed\nduring a rebase. Note also that in the uncommon case where $OTHER and\nHEAD do not share a common ancestor, this will show the complete\nhistories of both sides since their root commits, which is the same\nbehaviour as currently happens in that case for HEAD and MERGE_HEAD.\n\nAdjust the documentation of this option accordingly.\n\nCo-authored-by: Johannes Sixt \nCo-authored-by: Philippe Blain \nSigned-off-by: Michael Lohmann \n[jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\nSigned-off-by: Junio C Hamano \nSigned-off-by: Philippe Blain \n---\n Documentation/rev-list-options.txt | 7 +++++--\n revision.c | 31 +++++++++++++++++++++++--------\n 2 files changed, 28 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 2bf239ff03..9ce7a5eedc 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -341,8 +341,11 @@ See also linkgit:git-reflog[1].\n Under `--pretty=reference`, this information will not be shown at all.\n \n --merge::\n-\tAfter a failed merge, show refs that touch files having a\n-\tconflict and don't exist on all heads to merge.\n+\tShow commits touching conflicted paths in the range `HEAD...`,\n+\twhere `` is the first existing pseudoref in `MERGE_HEAD`,\n+\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n+\twhen the index has unmerged entries. This option can be used to show\n+\trelevant commits when resolving conflicts from a 3-way merge.\n \n --boundary::\n \tOutput excluded boundary commits. Boundary commits are\ndiff --git a/revision.c b/revision.c\nindex ee26988cc6..a90a6f861b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1961,11 +1961,31 @@ static void add_pending_commit_list(struct rev_info *revs,\n \t}\n }\n \n+static const char *lookup_other_head(struct object_id *oid)\n+{\n+\tint i;\n+\tstatic const char *const other_head[] = {\n+\t\t\"MERGE_HEAD\", \"CHERRY_PICK_HEAD\", \"REVERT_HEAD\", \"REBASE_HEAD\"\n+\t};\n+\n+\tfor (i = 0; i < ARRAY_SIZE(other_head); i++)\n+\t\tif (!read_ref_full(other_head[i],\n+\t\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\toid, NULL)) {\n+\t\t\tif (is_null_oid(oid))\n+\t\t\t\tdie(_(\"%s is a symbolic ref?\"), other_head[i]);\n+\t\t\treturn other_head[i];\n+\t\t}\n+\n+\tdie(_(\"--merge requires one of the pseudorefs MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD\"));\n+}\n+\n static void prepare_show_merge(struct rev_info *revs)\n {\n \tstruct commit_list *bases;\n \tstruct commit *head, *other;\n \tstruct object_id oid;\n+\tconst char *other_name;\n \tconst char **prune = NULL;\n \tint i, prune_num = 1; /* counting terminating NULL */\n \tstruct index_state *istate = revs->repo->index;\n@@ -1973,15 +1993,10 @@ static void prepare_show_merge(struct rev_info *revs)\n \tif (repo_get_oid(the_repository, \"HEAD\", &oid))\n \t\tdie(\"--merge without HEAD?\");\n \thead = lookup_commit_or_die(&oid, \"HEAD\");\n-\tif (read_ref_full(\"MERGE_HEAD\",\n-\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t&oid, NULL))\n-\t\tdie(\"--merge without MERGE_HEAD?\");\n-\tif (is_null_oid(&oid))\n-\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n-\tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n+\tother_name = lookup_other_head(&oid);\n+\tother = lookup_commit_or_die(&oid, other_name);\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n-\tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n+\tadd_pending_object(revs, &other->object, other_name);\n \tbases = repo_get_merge_bases(the_repository, head, other);\n \tadd_rev_cmdline_list(revs, bases, REV_CMD_MERGE_BASE, UNINTERESTING | BOTTOM);\n \tadd_pending_commit_list(revs, bases, UNINTERESTING | BOTTOM);\n\n-- \n2.39.1\n\n"},{"id":"489347","messageId":"xmqqa5nnj10v.fsf@gitster.g","threadId":"60897","inReplyTo":"20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-2-af1ef2d9e44d@gmail.com","subject":"Re: [PATCH v5 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-26T04:35:28Z","receivedAt":"2024-02-26T04:35:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n> +\tfor (i = 0; i < ARRAY_SIZE(other_head); i++)\n> +\t\tif (!read_ref_full(other_head[i],\n> +\t\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> +\t\t\t\toid, NULL)) {\n> +\t\t\tif (is_null_oid(oid))\n> +\t\t\t\tdie(_(\"%s is a symbolic ref?\"), other_head[i]);\n> +\t\t\treturn other_head[i];\n> +\t\t}\n> +\n> +\tdie(_(\"--merge requires one of the pseudorefs MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD\"));\n> +}\n\nJust a minor nit, but reacting to recent \"passive-aggressive\"\nmessage change in another thread, perhaps we should stop asking a\nrhetorical question like the new message and instead state what we\ndetected and what we consider is an error condition as a fact in\nthem.\n\nThe last die() in the above helper function used to be such a\nrhetorical question \"--merge without HEAD?\" but now it reads much\nbetter. The one about symbolic ref is new in this series, and we\ncan avoid making it rhetorical from the get go. Perhaps \"%s exists\nbut it is a symbolic ref\" or something?\n"},{"id":"489390","messageId":"c46a0a4f-63dc-404a-8ee2-64a34f474c5f@gmail.com","threadId":"60897","inReplyTo":"20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-1-af1ef2d9e44d@gmail.com","subject":"Re: [PATCH v5 1/2] revision: ensure MERGE_HEAD is a ref in prepare_show_merge","fromName":"Jean-Noël Avila","fromEmail":"avila.jn@gmail.com","sentAt":"2024-02-26T17:22:50Z","receivedAt":"2024-02-26T17:22:53Z","isPatch":true,"sender":{"key":"jn.avila@free.fr","avatar":"https://avatars.githubusercontent.com/u/156172?v=4"},"body":"Hello,\n\nLe 25/02/2024 à 22:56, Philippe Blain a écrit :\n> From: Michael Lohmann \n> \n> This is done to\n> (1) ensure MERGE_HEAD is a ref,\n> (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()\n> (3) error out when MERGE_HEAD is a symref.\n> \n> Helped-by: Junio C Hamano \n> Signed-off-by: Michael Lohmann \n> Signed-off-by: Junio C Hamano \n> Signed-off-by: Philippe Blain \n> ---\n> revision.c | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/revision.c b/revision.c\n> index 2424c9bd67..ee26988cc6 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1973,8 +1973,12 @@ static void prepare_show_merge(struct rev_info *revs)\n> \tif (repo_get_oid(the_repository, \"HEAD\", &oid))\n> \t\tdie(\"--merge without HEAD?\");\n> \thead = lookup_commit_or_die(&oid, \"HEAD\");\n> -\tif (repo_get_oid(the_repository, \"MERGE_HEAD\", &oid))\n> +\tif (read_ref_full(\"MERGE_HEAD\",\n> +\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> +\t\t\t&oid, NULL))\n> \t\tdie(\"--merge without MERGE_HEAD?\");\n> +\tif (is_null_oid(&oid))\n> +\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n\nFollowing the thread about being less passive-aggressive, maybe this\ncould be rephrased in an assertive mood.\n\nBy the way, this string is translatable, but not the one 2 lines above.\nWhat is the policy around translation?\n\n> \tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n> \tadd_pending_object(revs, &head->object, \"HEAD\");\n> \tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n> \n\nThanks.\n"},{"id":"489395","messageId":"d60ba27a-ac65-842a-9efe-a163480363d5@gmail.com","threadId":"60897","inReplyTo":"xmqqa5nnj10v.fsf@gitster.g","subject":"Re: [PATCH v5 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-26T17:43:52Z","receivedAt":"2024-02-26T17:43:55Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Junio,\n\nLe 2024-02-25 à 23:35, Junio C Hamano a écrit :\n> Philippe Blain writes:\n> \n>> +\tfor (i = 0; i < ARRAY_SIZE(other_head); i++)\n>> +\t\tif (!read_ref_full(other_head[i],\n>> +\t\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n>> +\t\t\t\toid, NULL)) {\n>> +\t\t\tif (is_null_oid(oid))\n>> +\t\t\t\tdie(_(\"%s is a symbolic ref?\"), other_head[i]);\n>> +\t\t\treturn other_head[i];\n>> +\t\t}\n>> +\n>> +\tdie(_(\"--merge requires one of the pseudorefs MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD\"));\n>> +}\n> \n> Just a minor nit, but reacting to recent \"passive-aggressive\"\n> message change in another thread, perhaps we should stop asking a\n> rhetorical question like the new message and instead state what we\n> detected and what we consider is an error condition as a fact in\n> them.\n> \n> The last die() in the above helper function used to be such a\n> rhetorical question \"--merge without HEAD?\" but now it reads much\n> better. The one about symbolic ref is new in this series, and we\n> can avoid making it rhetorical from the get go. Perhaps \"%s exists\n> but it is a symbolic ref\" or something?\n\nOk, I can make that change. I agree we should maybe keep these rethorical \nquestions to 'BUG' calls...\n\nThanks,\nPhilippe.\n"},{"id":"489398","messageId":"13c882d4-8f61-1ea5-8949-fdde16bcfc5b@gmail.com","threadId":"60897","inReplyTo":"c46a0a4f-63dc-404a-8ee2-64a34f474c5f@gmail.com","subject":"Re: [PATCH v5 1/2] revision: ensure MERGE_HEAD is a ref in prepare_show_merge","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-26T17:54:21Z","receivedAt":"2024-02-26T17:54:24Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Jean-Noël,\n\nLe 2024-02-26 à 12:22, Jean-Noël Avila a écrit :\n> Hello,\n> \n> Le 25/02/2024 à 22:56, Philippe Blain a écrit :\n>> From: Michael Lohmann \n>>\n>> This is done to\n>> (1) ensure MERGE_HEAD is a ref,\n>> (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()\n>> (3) error out when MERGE_HEAD is a symref.\n>>\n>> Helped-by: Junio C Hamano \n>> Signed-off-by: Michael Lohmann \n>> Signed-off-by: Junio C Hamano \n>> Signed-off-by: Philippe Blain \n>> ---\n>> revision.c | 6 +++++-\n>> 1 file changed, 5 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/revision.c b/revision.c\n>> index 2424c9bd67..ee26988cc6 100644\n>> --- a/revision.c\n>> +++ b/revision.c\n>> @@ -1973,8 +1973,12 @@ static void prepare_show_merge(struct rev_info *revs)\n>> \tif (repo_get_oid(the_repository, \"HEAD\", &oid))\n>> \t\tdie(\"--merge without HEAD?\");\n>> \thead = lookup_commit_or_die(&oid, \"HEAD\");\n>> -\tif (repo_get_oid(the_repository, \"MERGE_HEAD\", &oid))\n>> +\tif (read_ref_full(\"MERGE_HEAD\",\n>> +\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n>> +\t\t\t&oid, NULL))\n>> \t\tdie(\"--merge without MERGE_HEAD?\");\n>> +\tif (is_null_oid(&oid))\n>> +\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n> \n> Following the thread about being less passive-aggressive, maybe this\n> could be rephrased in an assertive mood.\n\nYes, Junio suggested the same in .\n\n> \n> By the way, this string is translatable, but not the one 2 lines above.\n> What is the policy around translation?\n\nMy understanding is that new error messages should be translated, but here the patch\nis not touching the message \"--merge without HEAD?\" so I would think\nit is OK to avoid changing these lines to mark it for translation.\nBut, I could make that change in a preparatory patch (and rephrase it \nat the same time). \n\nThanks, \nPhilippe.\n"},{"id":"489497","messageId":"ff5a3954-4e7a-42b7-988e-f306b45918bf@gmail.com","threadId":"60897","inReplyTo":"20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-0-af1ef2d9e44d@gmail.com","subject":"Re: [PATCH v5 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-27T14:00:22Z","receivedAt":"2024-02-27T14:00:25Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 25/02/2024 21:56, Philippe Blain wrote:\n> Changes in v5:\n> - Marked error messages for translation and tweaked them as suggested by Phillip\n> - Reworded the message of 2/2 as suggested by Phillip\n> - Removed the change to gitk's doc in 2/2 as pointed out by Johannes\n> - Fixed the trailers in 2/2\n> - Improved the doc in 2/2 as suggested by Phillip and Jean-Noël\n\nThese changes look good, thanks for making them. I agree with the other \nreviewers that it would be nice to improve the wording of the error \nmessage when we find a symbolic ref. Everything else looks good to me.\n\nThanks\n\nPhillip\n\n"},{"id":"489559","messageId":"xmqqmsrlsqty.fsf@gitster.g","threadId":"60897","inReplyTo":"ff5a3954-4e7a-42b7-988e-f306b45918bf@gmail.com","subject":"Re: [PATCH v5 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-27T18:30:01Z","receivedAt":"2024-02-27T18:30:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 25/02/2024 21:56, Philippe Blain wrote:\n>> Changes in v5:\n>> - Marked error messages for translation and tweaked them as suggested by Phillip\n>> - Reworded the message of 2/2 as suggested by Phillip\n>> - Removed the change to gitk's doc in 2/2 as pointed out by Johannes\n>> - Fixed the trailers in 2/2\n>> - Improved the doc in 2/2 as suggested by Phillip and Jean-Noël\n>\n> These changes look good, thanks for making them. I agree with the\n> other reviewers that it would be nice to improve the wording of the\n> error message when we find a symbolic ref. Everything else looks good\n> to me.\n>\n> Thanks\n>\n> Phillip\n\nThanks for a review. Queued.\n"},{"id":"489611","messageId":"20240228-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v6-0-8ec34c052b39@gmail.com","threadId":"60897","inReplyTo":"20240225-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v5-0-af1ef2d9e44d@gmail.com","subject":"[PATCH v6 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-28T13:54:52Z","receivedAt":"2024-02-28T13:55:05Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Changes in v6:\n- Changed the error message added in 1/2 and adjusted in 2/2 to avoid a rhetorical\nquestion\n\nChanges in v5:\n- Marked error messages for translation and tweaked them as suggested by Phillip\n- Reworded the message of 2/2 as suggested by Phillip\n- Removed the change to gitk's doc in 2/2 as pointed out by Johannes\n- Fixed the trailers in 2/2\n- Improved the doc in 2/2 as suggested by Phillip and Jean-Noël\n\nChanges in v4:\n- Added a commit message for 2/2 detailing the use case and summarizing the discussion in the thread\n- Adjusted the documentation of the option\n\n---\nMichael Lohmann (2):\n revision: ensure MERGE_HEAD is a ref in prepare_show_merge\n revision: implement `git log --merge` also for rebase/cherry-pick/revert\n\n Documentation/rev-list-options.txt | 7 +++++--\n revision.c | 27 +++++++++++++++++++++++----\n 2 files changed, 28 insertions(+), 6 deletions(-)\n---\nbase-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0\nchange-id: 20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-05bd8e8797db\n\nRange-diff versus v5:\n\n1: c9536431d1 ! 1: 363657561c revision: ensure MERGE_HEAD is a ref in prepare_show_merge\n @@ Commit message\n (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()\n (3) error out when MERGE_HEAD is a symref.\n \n + Note that we avoid marking the new error message for translation as it\n + will be done in the next commit when the message is generalized to other\n + special refs.\n +\n Helped-by: Junio C Hamano \n Signed-off-by: Michael Lohmann \n Signed-off-by: Junio C Hamano \n @@ revision.c: static void prepare_show_merge(struct rev_info *revs)\n +\t\t\t&oid, NULL))\n \t\tdie(\"--merge without MERGE_HEAD?\");\n +\tif (is_null_oid(&oid))\n -+\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n ++\t\tdie(_(\"MERGE_HEAD exists but is a symbolic ref\"));\n \tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n \tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n2: 1641c4be81 ! 2: 749abadc04 revision: implement `git log --merge` also for rebase/cherry-pick/revert\n @@ revision.c: static void add_pending_commit_list(struct rev_info *revs,\n +\t\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n +\t\t\t\toid, NULL)) {\n +\t\t\tif (is_null_oid(oid))\n -+\t\t\t\tdie(_(\"%s is a symbolic ref?\"), other_head[i]);\n ++\t\t\t\tdie(_(\"%s exists but is a symbolic ref\"), other_head[i]);\n +\t\t\treturn other_head[i];\n +\t\t}\n +\n @@ revision.c: static void prepare_show_merge(struct rev_info *revs)\n -\t\t\t&oid, NULL))\n -\t\tdie(\"--merge without MERGE_HEAD?\");\n -\tif (is_null_oid(&oid))\n --\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n +-\t\tdie(_(\"MERGE_HEAD exists but is a symbolic ref\"));\n -\tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n +\tother_name = lookup_other_head(&oid);\n +\tother = lookup_commit_or_die(&oid, other_name);\n\n"},{"id":"489612","messageId":"20240228-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v6-1-8ec34c052b39@gmail.com","threadId":"60897","inReplyTo":"20240228-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v6-0-8ec34c052b39@gmail.com","subject":"[PATCH v6 1/2] revision: ensure MERGE_HEAD is a ref in prepare_show_merge","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-28T13:54:53Z","receivedAt":"2024-02-28T13:55:06Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Michael Lohmann \n\nThis is done to\n(1) ensure MERGE_HEAD is a ref,\n(2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()\n(3) error out when MERGE_HEAD is a symref.\n\nNote that we avoid marking the new error message for translation as it\nwill be done in the next commit when the message is generalized to other\nspecial refs.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Michael Lohmann \nSigned-off-by: Junio C Hamano \nSigned-off-by: Philippe Blain \n---\n revision.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 2424c9bd67..df775f74d0 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1973,8 +1973,12 @@ static void prepare_show_merge(struct rev_info *revs)\n \tif (repo_get_oid(the_repository, \"HEAD\", &oid))\n \t\tdie(\"--merge without HEAD?\");\n \thead = lookup_commit_or_die(&oid, \"HEAD\");\n-\tif (repo_get_oid(the_repository, \"MERGE_HEAD\", &oid))\n+\tif (read_ref_full(\"MERGE_HEAD\",\n+\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t&oid, NULL))\n \t\tdie(\"--merge without MERGE_HEAD?\");\n+\tif (is_null_oid(&oid))\n+\t\tdie(_(\"MERGE_HEAD exists but is a symbolic ref\"));\n \tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n \tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n\n-- \n2.39.1\n\n"},{"id":"489613","messageId":"20240228-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v6-2-8ec34c052b39@gmail.com","threadId":"60897","inReplyTo":"20240228-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v6-0-8ec34c052b39@gmail.com","subject":"[PATCH v6 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-28T13:54:54Z","receivedAt":"2024-02-28T13:55:07Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Michael Lohmann \n\n'git log' learned in ae3e5e1ef2 (git log -p --merge [[--] paths...],\n2006-07-03) to show commits touching conflicted files in the range\nHEAD...MERGE_HEAD, an addition documented in d249b45547 (Document\nrev-list's option --merge, 2006-08-04).\n\nIt can be useful to look at the commit history to understand what lead\nto merge conflicts also for other mergy operations besides merges, like\ncherry-pick, revert and rebase.\n\nFor rebases and cherry-picks, an interesting range to look at is\nHEAD...{REBASE_HEAD,CHERRY_PICK_HEAD}, since even if all the commits\nincluded in that range are not directly part of the 3-way merge,\nconflicts encountered during these operations can indeed be caused by\nchanges introduced in preceding commits on both sides of the history.\n\nFor revert, as we are (most likely) reversing changes from a previous\ncommit, an appropriate range is REVERT_HEAD..HEAD, which is equivalent\nto REVERT_HEAD...HEAD and to HEAD...REVERT_HEAD, if we keep HEAD and its\nparents on the left side of the range.\n\nAs such, adjust the code in prepare_show_merge so it constructs the\nrange HEAD...$OTHER for OTHER={MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD\nor REBASE_HEAD}. Note that we try these pseudorefs in order, so keep\nREBASE_HEAD last since the three other operations can be performed\nduring a rebase. Note also that in the uncommon case where $OTHER and\nHEAD do not share a common ancestor, this will show the complete\nhistories of both sides since their root commits, which is the same\nbehaviour as currently happens in that case for HEAD and MERGE_HEAD.\n\nAdjust the documentation of this option accordingly.\n\nCo-authored-by: Johannes Sixt \nCo-authored-by: Philippe Blain \nSigned-off-by: Michael Lohmann \n[jc: tweaked in j6t's precedence fix that tries REBASE_HEAD last]\nSigned-off-by: Junio C Hamano \nSigned-off-by: Philippe Blain \n---\n Documentation/rev-list-options.txt | 7 +++++--\n revision.c | 31 +++++++++++++++++++++++--------\n 2 files changed, 28 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex a583b52c61..29e7b50bcf 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -341,8 +341,11 @@ See also linkgit:git-reflog[1].\n Under `--pretty=reference`, this information will not be shown at all.\n \n --merge::\n-\tAfter a failed merge, show refs that touch files having a\n-\tconflict and don't exist on all heads to merge.\n+\tShow commits touching conflicted paths in the range `HEAD...`,\n+\twhere `` is the first existing pseudoref in `MERGE_HEAD`,\n+\t`CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works\n+\twhen the index has unmerged entries. This option can be used to show\n+\trelevant commits when resolving conflicts from a 3-way merge.\n \n --boundary::\n \tOutput excluded boundary commits. Boundary commits are\ndiff --git a/revision.c b/revision.c\nindex df775f74d0..d2ebdd045a 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1961,11 +1961,31 @@ static void add_pending_commit_list(struct rev_info *revs,\n \t}\n }\n \n+static const char *lookup_other_head(struct object_id *oid)\n+{\n+\tint i;\n+\tstatic const char *const other_head[] = {\n+\t\t\"MERGE_HEAD\", \"CHERRY_PICK_HEAD\", \"REVERT_HEAD\", \"REBASE_HEAD\"\n+\t};\n+\n+\tfor (i = 0; i < ARRAY_SIZE(other_head); i++)\n+\t\tif (!read_ref_full(other_head[i],\n+\t\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\toid, NULL)) {\n+\t\t\tif (is_null_oid(oid))\n+\t\t\t\tdie(_(\"%s exists but is a symbolic ref\"), other_head[i]);\n+\t\t\treturn other_head[i];\n+\t\t}\n+\n+\tdie(_(\"--merge requires one of the pseudorefs MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD\"));\n+}\n+\n static void prepare_show_merge(struct rev_info *revs)\n {\n \tstruct commit_list *bases;\n \tstruct commit *head, *other;\n \tstruct object_id oid;\n+\tconst char *other_name;\n \tconst char **prune = NULL;\n \tint i, prune_num = 1; /* counting terminating NULL */\n \tstruct index_state *istate = revs->repo->index;\n@@ -1973,15 +1993,10 @@ static void prepare_show_merge(struct rev_info *revs)\n \tif (repo_get_oid(the_repository, \"HEAD\", &oid))\n \t\tdie(\"--merge without HEAD?\");\n \thead = lookup_commit_or_die(&oid, \"HEAD\");\n-\tif (read_ref_full(\"MERGE_HEAD\",\n-\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n-\t\t\t&oid, NULL))\n-\t\tdie(\"--merge without MERGE_HEAD?\");\n-\tif (is_null_oid(&oid))\n-\t\tdie(_(\"MERGE_HEAD exists but is a symbolic ref\"));\n-\tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n+\tother_name = lookup_other_head(&oid);\n+\tother = lookup_commit_or_die(&oid, other_name);\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n-\tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n+\tadd_pending_object(revs, &other->object, other_name);\n \tbases = repo_get_merge_bases(the_repository, head, other);\n \tadd_rev_cmdline_list(revs, bases, REV_CMD_MERGE_BASE, UNINTERESTING | BOTTOM);\n \tadd_pending_commit_list(revs, bases, UNINTERESTING | BOTTOM);\n\n-- \n2.39.1\n\n"},{"id":"489614","messageId":"3875a0dc-6ef2-4bd3-9852-c180043464c3@gmail.com","threadId":"60897","inReplyTo":"20240228-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v6-0-8ec34c052b39@gmail.com","subject":"Re: [PATCH v6 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-28T14:40:02Z","receivedAt":"2024-02-28T14:40:09Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Philippe\n\nOn 28/02/2024 13:54, Philippe Blain wrote:\n> Range-diff versus v5:\n> \n> 1: c9536431d1 ! 1: 363657561c revision: ensure MERGE_HEAD is a ref in prepare_show_merge\n> @@ Commit message\n> (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()\n> (3) error out when MERGE_HEAD is a symref.\n> \n> + Note that we avoid marking the new error message for translation as it\n> + will be done in the next commit when the message is generalized to other\n> + special refs.\n\nLooking at the change below, the new message is in fact marked for \ntranslation. I don't think this matters (other than the commit message \nbeing confusing) as the translators will only see the final version of \nthe massage.\n\n> Helped-by: Junio C Hamano \n> Signed-off-by: Michael Lohmann \n> Signed-off-by: Junio C Hamano \n> @@ revision.c: static void prepare_show_merge(struct rev_info *revs)\n> +\t\t\t&oid, NULL))\n> \t\tdie(\"--merge without MERGE_HEAD?\");\n> +\tif (is_null_oid(&oid))\n> -+\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n> ++\t\tdie(_(\"MERGE_HEAD exists but is a symbolic ref\"));\n\nThe new message is marked with _(...) so will be translated.\n\n> \tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n> \tadd_pending_object(revs, &head->object, \"HEAD\");\n> \tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n> 2: 1641c4be81 ! 2: 749abadc04 revision: implement `git log --merge` also for rebase/cherry-pick/revert\n> @@ revision.c: static void add_pending_commit_list(struct rev_info *revs,\n> +\t\t\t\tRESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n> +\t\t\t\toid, NULL)) {\n> +\t\t\tif (is_null_oid(oid))\n> -+\t\t\t\tdie(_(\"%s is a symbolic ref?\"), other_head[i]);\n> ++\t\t\t\tdie(_(\"%s exists but is a symbolic ref\"), other_head[i]);\n\nThe second patch updates the message and this new version retains the _(...)\n\nBest Wishes\n\nPhillip\n\n> +\t\t\treturn other_head[i];\n> +\t\t}\n> +\n> @@ revision.c: static void prepare_show_merge(struct rev_info *revs)\n> -\t\t\t&oid, NULL))\n> -\t\tdie(\"--merge without MERGE_HEAD?\");\n> -\tif (is_null_oid(&oid))\n> --\t\tdie(_(\"MERGE_HEAD is a symbolic ref?\"));\n> +-\t\tdie(_(\"MERGE_HEAD exists but is a symbolic ref\"));\n> -\tother = lookup_commit_or_die(&oid, \"MERGE_HEAD\");\n> +\tother_name = lookup_other_head(&oid);\n> +\tother = lookup_commit_or_die(&oid, other_name);\n> \n"},{"id":"489771","messageId":"7d21027a-3b5d-dcef-7803-ce41be0a948c@gmail.com","threadId":"60897","inReplyTo":"3875a0dc-6ef2-4bd3-9852-c180043464c3@gmail.com","subject":"Re: [PATCH v6 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-03-02T15:35:22Z","receivedAt":"2024-03-02T15:35:24Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Phillip,\n\nLe 2024-02-28 à 09:40, phillip.wood123@gmail.com a écrit :\n> Hi Philippe\n> \n> On 28/02/2024 13:54, Philippe Blain wrote:\n>> Range-diff versus v5:\n>>\n>> 1:  c9536431d1 ! 1:  363657561c revision: ensure MERGE_HEAD is a ref in prepare_show_merge\n>>      @@ Commit message\n>>           (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()\n>>           (3) error out when MERGE_HEAD is a symref.\n>>            +    Note that we avoid marking the new error message for translation as it\n>>      +    will be done in the next commit when the message is generalized to other\n>>      +    special refs.\n> \n> Looking at the change below, the new message is in fact marked for translation. I don't think this matters (other than the commit message being confusing) as the translators will only see the final version of the massage.\n\nOops, you are right, I wanted to do what I wrote but ended up marking it also in 1/2.\nJunio already merged it to next, so I guess it's gonna stay that way.\n\nThanks for your review,\nPhilippe.\n"}]}