On Wed, Feb 02 2022, Elijah Newren wrote:
> On Wed, Feb 2, 2022 at 1:32 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>
>> "Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>>
>> > @@ -450,7 +451,11 @@ static int real_merge(struct merge_tree_options *o,
>> > merge_get_conflicted_files(&result, &conflicted_files);
>> > for (i = 0; i < conflicted_files.nr; i++) {
>> > const char *name = conflicted_files.items[i].string;
>> > - if (last && !strcmp(last, name))
>> > + struct stage_info *c = conflicted_files.items[i].util;
>> > + if (!o->exclude_modes_oids_stages)
>> > + printf("%06o %s %d\t",
>> > + c->mode, oid_to_hex(&c->oid), c->stage);
>> > + else if (last && !strcmp(last, name))
>> > continue;
>> > write_name_quoted_relative(
>> > name, prefix, stdout, line_termination);
>>
>> OK. The addition (and disabling of the deduping) is quite trivial.
>> We do not even have to worry about line termination since the extra
>> pieces of info are prepended to the pathname. Nice.
>>
>> > @@ -485,6 +490,10 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
>> > N_("do a trivial merge only"), 't'),
>> > OPT_BOOL(0, "messages", &o.show_messages,
>> > N_("also show informational/conflict messages")),
>> > + OPT_BOOL_F('l', "exclude-modes-oids-stages",
>> > + &o.exclude_modes_oids_stages,
>> > + N_("list conflicted files without modes/oids/stages"),
>> > + PARSE_OPT_NONEG),
>>
>> Why does "-l" give shorter output than without it? "-l" strongly
>> hints a longer output than without, at least to me. Just wondering
>> if this will not become a source of confusion to future scripting
>> users.
>
> Here's another example where I was struggling with naming. Something
> like ls-tree's `--name-only` would have been nice, but I was worried
> it'd be confusing since it only affected the conflicted info section
> and does not suppress the printing of the toplevel tree or the
> informational messages sections. And the name
> --exclude-modes-oids-stages was long enough that I wanted a short flag
> for it, and just used the first letter of the description ("list
> conflicted files..."). I'm happy to change either the long or the
> short name for this flag if anyone has suggestions.
There's always sidestepping it by replacing it with a --format :)
Anyway, I'd mentioned that in an earlier review in
<220124.864k5tigto.gmgdl@xxxxxxxxxxxxxxxxxxx>. FWIW here's an experiment
to do that that I polished up (mostly copied from the ls-tree WIP code
I'd written already).
I don't know if it will ever be useful, or if you think it's
worthwhile/simpler, but in either case I think in doing this I spotted
the following issues or otherwise noted inconsistencies in the pre-image:
The docs say that "<stage> <path>" is SP-separated, but it's
actually TAB-separated, the rest is SP-separated.
* That you de-dupe --exclude-modes-oids-stages is a bit of a hidden feature,
but argubly initiative. Should it by optional? In any case my formatting
experiment makes it optional, since it then needs to be generalized to de-dupe
after we've formatted.
* Perhaps we should support --abbrev as ls-tree does? The below diff shows
it's easy enough.
* The dance you have with sed-ing out the hash in the tests could be made much
easier with "sed 1d <out >actual" and --no-messages for some existing tests.
diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
index 6a2ed475106..e906d1dc9bf 100644
--- a/Documentation/git-merge-tree.txt
+++ b/Documentation/git-merge-tree.txt
@@ -44,10 +44,9 @@ OPTIONS
newline. Also begin the messages section with a NUL character
instead of a newline. See OUTPUT below for more information.
---exclude-oids-and-modes::
- Instead of writing a list of (mode, oid, stage, path) tuples
- to output for conflicted files, just provide a list of
- filenames with conflicts.
+--conflict-format::
+ Override the default "%(objectmode) %(objectname)
+ %(stage)%x09%(path)" format.
--[no-]messages::
Write any informational messages such as "Auto-merging <path>"
@@ -89,13 +88,13 @@ Conflicted file info
This is a sequence of lines with the format
- <mode> <object> <stage> <filename>
+ %(objectmode) %(objectname) %(stage)%x09%(path)
The filename will be quoted as explained for the configuration
-variable `core.quotePath` (see linkgit:git-config[1]). However, if
-the `--exclude-oids-and-modes` option is passed, the mode, object, and
-stage will be omitted. If `-z` is passed, the "lines" are terminated
-by a NUL character instead of a newline character.
+variable `core.quotePath` (see linkgit:git-config[1]).
+
+If `-z` is passed, the "lines" are terminated by a NUL character
+instead of a newline character.
Informational messages
~~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 58c0ddc5a32..14fed95a8ce 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -395,9 +395,64 @@ struct merge_tree_options {
int mode;
int allow_unrelated_histories;
int show_messages;
- int exclude_modes_oids_stages;
+ const char *conflict_format;
+ int unique_conflicts;
+ int abbrev;
};
+struct expand_conflict_data {
+ const char *prefix;
+ struct string_list_item *item;
+ struct strbuf *scratch;
+ int abbrev;
+ struct strbuf *sb_tmp;
+};
+static size_t expand_conflict_format(struct strbuf *sb,
+ const char *start,
+ void *context)
+{
+ struct expand_conflict_data *data = context;
+ struct string_list_item *item = data->item;
+ struct stage_info *info = item->util;
+ const char *end;
+ const char *p;
+ size_t len;
+
+ len = strbuf_expand_literal_cb(sb, start, NULL);
+ if (len)
+ return len;
+
+ if (*start != '(')
+ die(_("bad format as of '%s'"), start);
+ end = strchr(start + 1, ')');
+ if (!end)
+ die(_("format element '%s' does not end in ')'"), start);
+ len = end - start + 1;
+
+ if (skip_prefix(start, "(objectmode)", &p)) {
+ strbuf_addf(sb, "%06o", info->mode);
+ } else if (skip_prefix(start, "(objectname)", &p)) {
+ strbuf_addstr(sb, find_unique_abbrev(&info->oid, data->abbrev));
+ } else if (skip_prefix(start, "(stage)", &p)) {
+ strbuf_addf(sb, "%d", info->stage);
+ } else if (skip_prefix(start, "(path)", &p)) {
+ const char *name = item->string;
+
+ if (data->prefix)
+ name = relative_path(name, data->prefix, data->scratch);
+ strbuf_addstr(sb, name);
+
+ strbuf_reset(data->sb_tmp);
+ /* The relative_path() function resets "scratch" */
+
+ } else {
+ unsigned int errlen = (unsigned long)len;
+ die(_("bad format specifier %%%.*s"), errlen, start);
+ }
+
+ return len;
+}
+
static int real_merge(struct merge_tree_options *o,
const char *branch1, const char *branch2,
const char *prefix)
@@ -446,23 +501,43 @@ static int real_merge(struct merge_tree_options *o,
puts(oid_to_hex(&result.tree->object.oid));
if (!result.clean) {
struct string_list conflicted_files = STRING_LIST_INIT_NODUP;
- const char *last = NULL;
- int i;
+ struct string_list_item *item;
+ char *last = NULL;
+ struct strbuf sb = STRBUF_INIT;
+ struct strbuf tmp = STRBUF_INIT;
merge_get_conflicted_files(&result, &conflicted_files);
- for (i = 0; i < conflicted_files.nr; i++) {
- const char *name = conflicted_files.items[i].string;
- struct stage_info *c = conflicted_files.items[i].util;
- if (!o->exclude_modes_oids_stages)
- printf("%06o %s %d\t",
- c->mode, oid_to_hex(&c->oid), c->stage);
- else if (last && !strcmp(last, name))
+ for_each_string_list_item(item, &conflicted_files) {
+ struct expand_conflict_data ctx = {
+ .prefix = prefix,
+ .item = item,
+ .abbrev = o->abbrev,
+ .scratch = &sb,
+ .sb_tmp = &tmp,
+ };
+
+ strbuf_expand(&sb, o->conflict_format, expand_conflict_format, &ctx);
+ strbuf_addch(&sb, line_termination);
+
+ if (o->unique_conflicts && last && !strcmp(last, sb.buf)) {
+ free(last);
+ last = strbuf_detach(&sb, NULL);
continue;
- write_name_quoted_relative(
- name, prefix, stdout, line_termination);
- last = name;
+ }
+
+ fwrite(sb.buf, sb.len, 1, stdout);
+
+ if (o->unique_conflicts) {
+ free(last);
+ last = strbuf_detach(&sb, NULL);
+ } else {
+ strbuf_reset(&sb);
+ }
}
string_list_clear(&conflicted_files, 1);
+ strbuf_release(&sb);
+ strbuf_release(&tmp);
+ free(last);
}
if (o->show_messages) {
putchar(line_termination);
@@ -474,7 +549,11 @@ static int real_merge(struct merge_tree_options *o,
int cmd_merge_tree(int argc, const char **argv, const char *prefix)
{
- struct merge_tree_options o = { .show_messages = -1 };
+ struct merge_tree_options o = {
+ .show_messages = -1,
+ .conflict_format = "%(objectmode) %(objectname) %(stage)%x09%(path)",
+ .unique_conflicts = 1,
+ };
int expected_remaining_argc;
int original_argc;
@@ -493,14 +572,15 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
N_("also show informational/conflict messages")),
OPT_SET_INT('z', NULL, &line_termination,
N_("separate paths with the NUL character"), '\0'),
- OPT_BOOL_F('l', "exclude-modes-oids-stages",
- &o.exclude_modes_oids_stages,
- N_("list conflicted files without modes/oids/stages"),
- PARSE_OPT_NONEG),
+ OPT_STRING(0, "conflict-format", &o.conflict_format, N_("format"),
+ N_("specify a custom format to use for conflicted files")),
+ OPT_BOOL(0, "unique-conflicts", &o.unique_conflicts,
+ N_("omit duplicate --conflict-format lines")),
OPT_BOOL_F(0, "allow-unrelated-histories",
&o.allow_unrelated_histories,
N_("allow merging unrelated histories"),
PARSE_OPT_NONEG),
+ OPT__ABBREV(&o.abbrev),
OPT_END()
};
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index 4de089d976d..e6354b2d284 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -93,7 +93,7 @@ test_expect_success 'Barf on too many arguments' '
'
test_expect_success 'test conflict notices and such' '
- test_expect_code 1 git merge-tree --write-tree --exclude-modes-oids-stages side1 side2 >out &&
+ test_expect_code 1 git merge-tree --write-tree --conflict-format="%(path)" side1 side2 >out &&
sed -e "s/[0-9a-f]\{40,\}/HASH/g" out >actual &&
# Expected results:
@@ -115,8 +115,35 @@ test_expect_success 'test conflict notices and such' '
test_cmp expect actual
'
+test_expect_success 'merge-tree --unique-conflicts is the default' '
+ test_expect_code 1 git merge-tree --write-tree --conflict-format="%(path)" --no-messages side1 side2 >out &&
+ sed 1d <out >actual &&
+ cat >expect <<-\EOF &&
+ greeting
+ whatever~side1
+ EOF
+ test_cmp expect actual &&
+
+ test_expect_code 1 git merge-tree --write-tree --conflict-format="%(path)" --no-messages side1 side2 >out2 &&
+ sed 1d <out2 >actual2 &&
+ test_cmp actual actual2
+'
+
+test_expect_success 'merge-tree --no-unique-conflicts' '
+ test_expect_code 1 git merge-tree --write-tree --conflict-format="%(path)" --no-unique-conflicts --no-messages side1 side2 >out &&
+ sed 1d <out >actual &&
+ cat >expect <<-\EOF &&
+ greeting
+ greeting
+ greeting
+ whatever~side1
+ whatever~side1
+ EOF
+ test_cmp expect actual
+'
+
test_expect_success 'Just the conflicted files without the messages' '
- test_expect_code 1 git merge-tree --write-tree --no-messages --exclude-modes-oids-stages side1 side2 >out &&
+ test_expect_code 1 git merge-tree --write-tree --no-messages --conflict-format="%(path)" side1 side2 >out &&
sed -e "s/[0-9a-f]\{40,\}/HASH/g" out >actual &&
test_write_lines HASH greeting whatever~side1 >expect &&
