Hi Elijah, On Tue, 22 Feb 2022, Elijah Newren wrote: > Sidenote: Do you lump in binary merge conflicts with "non-semantic > merge conflicts"? You would by your definition, but I'm not sure it > matches. > > I tend to call things either content-based conflicts or path-based > conflicts, where content-based usually means textual-based but also > includes merges of binaries. I like "content-based conflicts". And no, I had not even thought about binary merge conflicts yet... > On Mon, Feb 21, 2022 at 2:46 AM Johannes Schindelin > <Johannes.Schindelin@xxxxxx> wrote: > > > > Concretely: while I am not currently aware of any web UI that allows > > to resolve simple rename/rename conflicts, it is easily conceivable > > how to implement such a thing. When that happens, we will need to be > > able to teach the server-side code to discern between the cases that > > can be handled in the web UI (trivial merge conflicts, trivial > > rename/rename conflicts) as compared to scenarios where the conflicts > > are just too complex. > > Um, I'm really worried about attempting to make the conflict notices > machine parseable. I don't like that idea at all, and I even tried to > rule that out already with my wording: > """ > In all cases, the > <Informational messages> section has the necessary info, though it is > not designed to be machine parseable. > """ > though maybe I should have been even more explicit. The restrictions > that those messages be stable is too rigid, I think. I also think > they're a poor way to communicate information to a higher level tool. > I would much rather us add some kind of additional return data > structures from merge ort and use them if we want extra info. Okay. I thought that we could keep the `CONFLICT (<type>)` constant enough to serve as such a machine-parseable thing. And then presenting `<path>NUL<message>NUL` could have served my use case well... > > Here's an excerpt from t4301: > > > > -- snip -- > > Auto-merging greeting > > CONFLICT (content): Merge conflict in greeting > > Auto-merging numbers > > CONFLICT (file/directory): directory in the way of whatever from side1; moving it to whatever~side1 instead. > > CONFLICT (modify/delete): whatever~side1 deleted in side2 and modified in side1. Version side1 of whatever~side1 left in tree. > > -- snap -- > > > > This is the complete set of messages provided in the `test conflict > > notices and such` test case. > > > > I immediately notice that every line contains at least one file name. > > Looking at https://github.com/git/git/blob/v2.35.1/merge-ort.c#L1899, it > > does not seem as if the file names are quoted: > > > > path_msg(opt, path, 1, _("Auto-merging %s"), path); > > > > (where `path` is used verbatim in a call to `merge_3way()` before that, > > i.e. it must not have been quoted) > > > > I would like to register a wish to ensure that file names with special > > characters (such as most notably line-feed characters) are quoted in these > > messages, so that a simple server-side parser can handle messages starting > > with `Auto-merging` and with `CONFLICT (content): Merge conflict in `, and > > "throw the hands up in the air" if any other message prefix is seen. > > > > Do you think we can switch to `sq_quote_buf_pretty()` for these messages? > > For the `Auto-merging` one, it would be trivial, but I fear that we will > > have to work a bit on the `path_msg()` function > > (https://github.com/git/git/blob/v2.35.1/merge-ort.c#L630-L649) because it > > accepts a variable list of arguments without any clue whether the > > arguments refer to paths or not. (And I would be loathe to switch _all_ > > callers to do the quoting themselves.) > > > > I see 28 calls to that function, and at least a couple that pass not only > > a path but also an OID (e.g. > > https://github.com/git/git/blob/v2.35.1/merge-ort.c#L1611-L1613). > > > > We could of course be sloppy and pass even OIDs through > > `sq_quote_buf_pretty()` in `path_msg()`, knowing that there won't be any > > special characters in them, but it gets more complicated e.g. in > > https://github.com/git/git/blob/v2.35.1/merge-ort.c#L1648-L1651, where we > > pass an `strbuf` that contains a somewhat free-form commit message. > > > > I guess we could still pass those through `sq_quote_buf_pretty()`, even if > > they are not paths, to ensure that there are no special characters in the > > machine-parseable lines. > > > > What do you think? > > Switching to single quoting paths as a matter of style might make > sense, but only if we go through and change every caller to do so so > that we can make sure it applies to all paths. And only paths and not > OIDs. Yes, that sounds unappealing. > But I'm going to reserve the right in merge-ort to modify, add, or > delete any of those messages passed to path_msg(), which might wreak > havoc on your attempts to parse those strings. I think they're a bad > form for communicating information to a script or program, and trying > to transform them into such risks making them suboptimal at > communicating info to humans. These messages should optimize the > latter, and if we want something for the former, it should probably be > a new independent bit of info. Makes sense. So we need something in addition to those messages. Ciao, Dscho
