Re: [PATCH v4 2/2] revision: implement `git log --merge` also for rebase/cherry-pick/revert

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Philippe

On 13/02/2024 13:27, Philippe Blain wrote:
Le 2024-02-12 à 06:02, Phillip Wood a écrit :
Hi Philippe
On 10/02/2024 23:35, Philippe Blain wrote:
From: Michael Lohmann <mi.al.lohmann@xxxxxxxxx>
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 2bf239ff03..5b4672c346 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -341,8 +341,10 @@ See also linkgit:git-reflog[1].
   Under `--pretty=reference`, this information will not be shown at all.
     --merge::
-    After a failed merge, show refs that touch files having a
-    conflict and don't exist on all heads to merge.
+    Show commits touching conflicted paths in the range `HEAD...$OTHER`,
+    where `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,
+    `CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works
+    when the index has unmerged entries.

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.

Yes, it took me a while to understand what that meant. I think it is simply
describing the range of commits shown. If we substitute "refs" for "commits"
and switch the order of the sentence, it reads:

     After a failed merge, show commits that don't exist on all heads to merge
     and that touch files having a conflict.

So it's just describing (a bit awkwardly) the HEAD...MERGE_HEAD range.

Ah, that makes sense, thanks for explaining


+    for (i = 0; i < ARRAY_SIZE(other_head); i++)
+        if (!read_ref_full(other_head[i],
+                RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
+                oid, NULL)) {
+            if (is_null_oid(oid))
+                die("%s is a symbolic ref???", other_head[i]);

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?)

This bit was suggested by Junio upthread in <xmqqzfxa9usx.fsf@gitster.g>.
I'm not sure if the are other causes of null oid, as I don't know well this
part of the code.
I agree that a single '?' would be enough, but I'm not sure about marking
this for translation, I think maybe this situation would be best handled with
BUG() ?

I think it would be a bug for git to create MERGE_HEAD as a symbolic ref but when we read MERGE_HEAD and find it is a symbolic ref we don't know if git created it or some third-party script so I think we should just report an error.

Best Wishes

Phillip





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux