Hi Antonin
On 18/01/2024 22:09, Antonin Delpeuch via GitGitGadget wrote:
From: Antonin Delpeuch <antonin@xxxxxxxxxxx>
Custom merge drivers need access to the names of the revisions they
are working on, so that the merge conflict markers they introduce
can refer to those revisions. The placeholders '%S', '%X' and '%Y'
are introduced to this end.
Thanks for working on this, I think it is a useful improvement. I guess
'%X' and '%Y' are no worse than the existing '%A' and '%B' but I do
wonder if we want to take the opportunity to switch to more descriptive
names for the various parameters passed to the custom merge strategy. We
do do this by supporting %(label:ours) modeled after the format
specifiers used by other commands such as "git log" and "git for-each-ref".
[...]
+will be stored via placeholder `%P`. Additionally, the names of the
+common ancestor revision (`%S`), of the current revision (`%X`) and
+of the other branch (`%Y`) can also be supplied. Those are short > +revision names, optionally joined with the paths of the file in each
+revision. Those paths are only present if they differ and are separated
+from the revision by a colon.
It might be simpler to just call these the "conflict marker labels"
without tying ourselves to a particular format. Something like
The conflict labels to be used for the common ancestor, local head
and other head can be passed by using '%(label:base)',
'%(label:ours)' and '%(label:theirs) respectively.
@@ -222,6 +222,12 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
Not part of this patch but I noticed that we're passing the filenames
for '%A' etc. unquoted which is a bit scary.
strbuf_addf(&cmd, "%d", marker_size);
else if (skip_prefix(format, "P", &format))
sq_quote_buf(&cmd, path);
+ else if (skip_prefix(format, "S", &format))
+ sq_quote_buf(&cmd, orig_name);
I think you can avoid the SIGSEV problem you mentioned in your other
email by changing this to
sq_quote_buf(&cmd, orig_name ? orig_name, "");
That would make sure the labels we pass match the ones used by the
internal merge.
Best Wishes
Phillip
