Comment by chrismorgan
2 years ago
Going through the sorts of conflicts it solves, and limitations in that, I find it claiming that in some insertions, order doesn’t matter <https://mergiraf.org/conflicts.html#neighbouring-insertions-...>.
I really don’t like that. At the language level, order may not matter, but quite frequently in such cases the order does matter, insofar as almost every human would put the two things in a particular order; or where there is a particular convention active. If you automatically merge the two sides in a different order from that, doing it automatically has become harmful.
My clearest example: take Base `struct Foo; struct Bar;`, then between these two items, Left inserts `impl Foo { }`, Right inserts `struct Baz;`. To the computer, the difference doesn’t matter, but merging it as `struct Foo; struct Baz; impl Foo { } struct Bar;` is obviously bad to a human. This is the problem: it’s handling language syntax semantics, but can’t be aware of logical semantics. (Hope you can grasp what I’m trying to convey, not sure of the best words.) Left was not inserting something between Foo and Bar, it was attaching something to the end of Foo. Whereas Right was probably inserting something between Foo and Bar—but maybe even it was inserting something before Bar. You perceive that these are all different things, logically.
Another example where this will quickly go wrong: in CSS rulesets, some will sort the declarations by property name lexicographically, some by property name length (seriously, it’s frequently so pretty), some will group by different types of property… you can’t know.
Not only that, the order of fields in a Java class does matter despite what that link claims. It's common to use Lombok to automatically generate constructors, and "the order of the parameters match the order in which the fields appear in your class."
https://projectlombok.org/features/constructor
The first two kinds of conflicts that Mergiraf handles looks somewhat dangerous to me when handled by a computer.
https://mergiraf.org/conflicts.html
Lombok is an interesting example, but yes, just with reflection you can already get order-dependent behaviors as the docs note. I've been thinking about giving users more control over this commutativity, but it's not clear to me what it should look like. A strict mode where commutativity is disabled entirely? The ability to disable certain commutative parents?
Code that uses Lombok features which change classes (rather than subclassing them) might have a high degree of similarity to Java, but it's not Java.
Yes, that's definitely something that could be refined, for instance by specifying that only children of specific types can be reordered together: https://codeberg.org/mergiraf/mergiraf/issues/6
I sort my CSS by property type (display, size, position, etc). Takes a while to get used to it but it definitely speeds up my ability to do CSS surgery.
Similarly, I've been wanting to make an autoformatter that reorders CSS properties into these categories: https://9elements.com/css-rule-order/ (box model, positioning, typography, etc)
I think it's the most useful CSS organization method I've found yet.
You might be interested in https://github.com/hudochenkov/stylelint-order
This is what I've always done naturally for the 20+ years I've been writing CSS. The only difference is that I put animation at the end, probably because it came much later than the rest.
Yeah, it really helps out, and I just head each section with a comment for fast grepping and grokking. My system combines a lot of things into the Display category which could be split out but I think I've struck a good enough balance.
I am very excited about this tool despite agreeing with what you wrote here.
The reason for that is that most of the time when I'm resolving huge numbers merge conflicts... I don't give a shit about details like field order. I just want to get some code that's functionally correct at p<0.05 so I can figure out what performance characteristics my old feature branch would have if I resurrected it. Or I want to kick off the slow integration tests ASAP. 9/10 times it's that kind of thing.
The 1/10 times where I'm like "OK now I actually wanna merge this code, I have to look over the resolutions, I will upload them to Gerrit/GitHub and do a self-review" I am more than happy to spend 20 minutes correcting order etc. Or I'll happily just switch this tool off and do a totally manual merge.
So yeah I think it just comes down to usecase.
I want to add another problem here. <https://mergiraf.org/adding-a-language.html#add-commutative-...>:
> To let Mergiraf reorder using statements to fix conflicts, we actually need to specify that they can be reordered with any of their siblings (any other child of their parent in the syntax tree).
That’s too coarse-grained. No idea about C♯, but languages could impose a rule that imports must come before anything else—long ago Rust had such a rule, for example. So it might be that within your compilation_unit node, only its using_directive children are commutative, and only among themselves.
Otherwise (lapsing into Rust syntax for convenience), with Base `use A; struct X…`, Left `use A; use B; struct X…` and Right `use A; struct Y; struct X…`, you could end up with the invalid `use A; struct Y; use B; struct X…`.
> No idea about C♯
C#'s using aren't imports, and order indeed doesn't matter.
Yes maybe, but these issues are true for Git's native merge algorithms too. It isn't perfect either.
As soon as you do any merge you're accepting that there might be edits you don't agree with.
Could some of this be mitigated by running a prettifier post-merge?
It's definitely something I would recommend in general, but I'm not sure if it would solve this particular problem (reordering blocks is perhaps a bit bold for a prettifier).
Maybe prettifier was the wrong word. I've definitely used code formatting tools that offer sorting of certain syntax elements as a feature. (Python imports in PyCharm springs to mind)
But surely Mergiraf has some opinion about the order when it doesn’t matter, right? Like structs before impls, in your example.
For now, you let it reorder every child within a given node type, which felt expressive enough to me in most cases, but I agree it would be good to refine that: https://codeberg.org/mergiraf/mergiraf/issues/6
Looking at the nice demo, I think just defaulting to asking for confirmation if there is ambiguity, instead of dazzling the user with `mergiraf solve` magic would help; there is already a `merigraf review`. Then, a confirm prompt, an option to undo the resolution completely, or just do it on a file-by-file basis (with help what command to run next).