7 ms·
Mergiraf: a syntax-aware merge driver for Git
- chrismorgan 2y agoGoing 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-and-deletions-of-elements-whose-order-does-not-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.
- andybak 2y agoCould some of this be mitigated by running a prettifier post-merge?
- wetneb 2y agoIt'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).
- andybak 2y agoMaybe 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)
- CGamesPlay 2y agoBut surely Mergiraf has some opinion about the order when it doesn’t matter, right? Like structs before impls, in your example.
- wetneb 2y agoFor 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 https://codeberg.org/mergiraf/mergiraf/issues/6
- zeotroph 2y agoLooking 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).
- soulofmischief 2y agoI 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.
- wonger_ 2y agoSimilarly, I've been wanting to make an autoformatter that reorders CSS properties into these categories: https://9elements.com/css-rule-order/ 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.
- fallingsquirrel 2y agoYou might be interested in https://github.com/hudochenkov/stylelint-order https://github.com/hudochenkov/stylelint-order
- p4bl0 2y agoThis 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.
- soulofmischief 2y agoYeah, 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.
- wetneb 2y agoYes, 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 https://codeberg.org/mergiraf/mergiraf/issues/6
- chrismorgan 2y agoI want to add another problem here. <https://mergiraf.org/adding-a-language.html#add-commutative-parents:~:text=To%20let%20Mergiraf%20reorder,in%20the%20syntax%20tree). 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…`.
- tokinonagare 2y ago> No idea about C♯ C#'s using aren't imports, and order indeed doesn't matter.
- soraminazuki 2y agoNot 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 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 https://mergiraf.org/conflicts.html
- wetneb 2y agoLombok 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?
- andrewaylett 2y agoCode 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.
- IshKebab 2y agoYes 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.
- bjackman 2y agoI 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.
- nathell 2y ago‘Why the giraffe? Two reasons. First, it can see farther due to its height; second, it has one of the biggest hearts of all land mammals. Besides, its ossicones make you believe it listens to you when you look at it.’ – My NVC teacher Kudos for the nonviolence. :)
- aeonik 2y agoFyi, giraffes are wild animals, and can be very violent and territorial.
- mnsc 2y agoI'm very literal minded and many metaphors irk me because they require a unspoken shared agreement of only considering the positive aspects that match and ignore the inconvenient ones. Like all sports metaphors used in work related activities. But still, don't skip nvc just because you can't accept a violent animal as a symbol for non violent communication. It has to much value for us literal minded people.
- aeonik 2y agoI'm all for minimizing violence and harm, but violence is a necessary part of life. I just watched a surgeon slice open a woman and save her life and the baby. Blood was everywhere, it was pretty violent. Nothing wrong with it. Violence != Malice Mergiraf looks pretty awesome though.
- computerfriend 2y agoI agree with your first and last point, but surgery != violence.
- aeonik 2y agoI disagree, but we are quickly getting deep into semantics. Surgery is not a harmless procedure. But the surgeon must decide that the patient is more likely to benefit overall from it.
- lucasoshiro 2y agoHappy to see something being developed for merge drivers, they are a underrated Git feature that could save a lot since the standard three-way merge of file contents is not aware of the language and can create some problems. For example, if you have this valid Python code: x = input() if x == 'x': print('foo') print('bar') If you delete the first print in a branch, delete the other print in another branch, then merge the two branches, you'll have this: x = input() if x == 'x': Both branches delete a portion of the code inside the if block, leaving it only with a whitespace. In Python it is not a valid code, as they empty scopes need to be declared with pass. I installed Mergiraf to see if it can solve this situation, but sadly, it doesn't support Python...
- wetneb 2y agoI tried your example but git does create a conflict in my case - but maybe I misunderstood the scenario. Python support can likely be done (I would be thrilled if someone made a PR for it), but I don't know if there is a lot of potential for solving conflicts there: imports can have side effects, function arguments are complicated with the mixture of positional and keyword arguments, decorators are effectful… it seems to me that there is a lot of sensitivity to order in many places.
- masklinn 2y agoFWIW your examples are very unclear as they use text formatting, you need to indent lines by 4 spaces (with an empty line before and after) for a code block e.g. if a == b: print("x") versus if a == b: print("x")
- chrismorgan 2y agoThe minimum required is actually only two spaces.
- deleted 2y ago[deleted]
- pknopf 2y agoCan LLMs help here?
- wetneb 2y agoThere are attempts to do that, such as https://arxiv.org/abs/2111.11904 https://arxiv.org/abs/2111.11904 or https://arxiv.org/abs/2109.00084 https://arxiv.org/abs/2109.00084. I couldn't find any open source implementation though. I'm personally not so keen to use LLMs for merging: I want it to be quick and predictable.
- leonheld 2y agoI'll certainly give it a try. Another tool I've been using (with varied degree of success) to enhance my git life is https://github.com/tummychow/git-absorb https://github.com/tummychow/git-absorb. If both of these worked flawlessly or maybe even officially incorporated in git, I'd be very happy.
- ksynwa 2y agoThis sounds like what jujutsu's workflow is like by default
- leonheld 2y agoI've heard of jujutsu, but I'm kinda over learning new tooling, specially experimental one. If it's not a drop-in replacement that makes my life better (like these easy "git plugins"), I'm basically not using.
- fallingsquirrel 2y agofwiw you can mix git and jj commands on the same repo and everything will work fine. If you want to stick with 99% git commands, and use jj only as a replacement for git-absorb, you can. For this particular use case, jj will handle merges/tags descended from the fixed commit gracefully, and I don't think git-absorb handles that.
- stouset 2y agoIt is a drop-in replacement for git and completely compatible. Your coworkers do not need to know you're using a different tool. And you can continue to use git commands directly if you want.
- deleted 2y ago[deleted]
- dochtman 2y agogit absorb is great, use it all the time.
- jappgar 2y agoThis seems like a really cool idea that would help with a scenario I encounter a lot with conflicts related to auto-formatting. Sometimes a small change can lead to a lot of whitespace changes below (in functional chains in js, for example). Can this also detect some scenarios where semantic conflicts (but not line conflicts) arise, usually due to moved code? I don't know the exact circumstances when this happens, but occasionally you can have e.g a function defined twice after two branches both move the same function elsewhere.
- wetneb 2y agoYes, see this example: https://mergiraf.org/conflicts.html#line-based-merges https://mergiraf.org/conflicts.html#line-based-merges
- donatj 2y agoNeat idea for sure. Language support is pretty limited right now, hopefully there's support for more in the works.
- Game_Ender 2y agoThe tool has an excellent architecture section [0] that goes into how it works under the hood. It stands out to me that a complex tool has an overview to this depth that allows you to grasp conceptually how it works. 0 - https://mergiraf.org/architecture.html https://mergiraf.org/architecture.html
- __MatrixMan__ 2y agoThat is nicely done, often hard to find, and usually it's what I'm looking for when deciding whether to use a piece of software: Show me the complexity you've encapsulated so that I can be the judge of whether the juice is worth the squeeze. Armed with that, I can tolerate some rough edges. Without it, I'll get stuck in weird ways that your docs can't anticipate.
- _flux 2y agoPython support would certainly be seem useful for this, in particular as its intentation-based AST should play nicely with this.
- fuzzy2 2y agoI'm eager to try this. Seems like it could revive the genre after Semantic Merge died.
- ctenb 2y agoWould it be possible to make this work with a treesitter grammar?
- wetneb 2y agoIt's based on tree-sitter indeed: https://mergiraf.org/adding-a-language.html https://mergiraf.org/adding-a-language.html
- froh 2y agowhat do you mean? its treesitter based already: https://mergiraf.org/architecture.html https://mergiraf.org/architecture.html
- ctenb 2y agoHm, I failed to notice that, since it is not mentioned in their introduction, and the list of supported languages is fairly small :)
- IshKebab 2y agoThis sounds great. To be honest though none of the merge tools really give me enough information to resolve all conflicts easily. The best I've got to is zdiff3 in VSCode (not using their fancy merge view which I don't understand at all). But it's missing: 1. Blame for the merge base. 2. Detection of the commit that introduced the first conflict. 3. Most annoyingly, no way to show diffs between the "current" and "incoming". IIRC it has buttons to compare both of those to the merge base, but not to each other. That often leaves me visually scanning the text to manually find differences like a neanderthal. Sometimes it's annoying enough that I copy & paste current/incoming into files and then diff those but that's a right pain.
- secondcoming 2y agoHave you tried p4merge? It's usually one of the first tools I install.
- IshKebab 2y agoNo. Does it do any of the things I want?
- DrBenCarson 2y agoHow is this better than Difftastic? https://github.com/Wilfred/difftastic https://github.com/Wilfred/difftastic
- jFriedensreich 2y agodifftastic does only diffing not merging.
- DarkPlayer 2y agoLooking at the architecture, they will probably run into some issues. We are doing something similar with SemanticDiff [1] and also started out using tree-sitter grammars for parsing and GumTree for matching. Both choices turned out to be problematic. Tree sitter grammars are primarily written to support syntax highlighting and often use a best effort approach to parsing. This is perfectly fine for syntax highlighting, since the worst that can happen is that a few characters are highlighted incorrectly. However, when diffing or modifying code you really want the code to be parsed according to the upstream grammar, not something that mostly resembles it. We are currently in the process of moving away from tree-sitter and instead using the parsers provided by the languages themselves where possible. GumTree is good at returning a result quickly, but there are quite a few cases where it always returned bad matches for us, no matter how many follow-up papers with improvements we tried to implement. In the end we switched over to a dijkstra based approach that tries to minimize the cost of the mapping, which is more computationally expensive but gives much better results. Difftastic uses a similar approach as well. [1]: https://semanticdiff.com/ https://semanticdiff.com/
- wetneb 2y agoThanks for the insightful comments! You surely have a lot more experience than me there, but my impression was that producing visual diffs and merging files are tasks that put different requirements on the tree matching algorithms, and Dijkstra-style approaches felt more fitting for diffs than for merging, so that's why I went for GumTree as it seemed to be the state of the art for merging. Does SemanticDiff offer a merge driver? I could only find documentation about diffing on the website. As to mismatches: yes, they are bound to happen in some cases. Even for line-based diffing, Git uses rather convoluted heuristics to avoid them (with the "histogram" diff algorithm), but they can't be completely ruled out there either. I hope that with enough safeguards (helper to review merges, downstream consistency checks with local fall-back to line-based diffing) they can be lived with. I'm happy to try other matching algorithms if they are more promising though (there isn't much coupling with the rest of the pipeline). Concerning tree-sitter, I have noticed some small issues, but nothing that was a show-stopper so far. I actually like it that it's designed for syntax highlighting, because it's really helpful that the representations it gives stay faithful to the original source, to avoid introducing reformatting noise in the merging process. Parsers written for a specific language can sometimes be too zealous (stripping comments out, doing some normalizations behind your back). That's a problem in Spork (which uses Spoon, a pretty advanced Java parser). And the uniform API tree-sitter offers over all those parsers is just too good to give up, in my opinion.
- jay-anderson 2y agoNice to see lilypond in the example.
- cool-RR 2y agoI'm flummoxed at the lack of Python support.
- 77pt77 2y agoNo typescript either, but it does support JS.
- erik_seaberg 2y agoSyntax-aware tools always have issues when a team extends the base language to fit their problem. Rust has macros. People started using "go generate" for stuff like early generics. Does Mergiraf take EBNF or plugins or does a team fork it to explain their syntax?
- wetneb 2y agoYeah at the moment it just supports whatever the tree-sitter parser accepts, period. A bring-your-own-grammar version could be interesting, I don't see why it couldn't work. Do you have any Rust crates to recommend, to do parsing according to a grammar supplied by the user at run time? It's likely to be slower, but maybe not prohibitively so… Another approach would be for the tool to accept doing structured merging even if there are error nodes in the parsed tree. If those error span the parts of the file where the extended language is used, then the tool could still help with merging the other parts, treating the errors as atomic blocks. I'd be a bit reluctant to do that, because there could be errors for all sorts of other reasons.
- papashell 2y agoSince tree sitter parsers output a c library, you could dynamically load it. The rust bindings themselves are a thin ffi wrapper. If you wanted to make it a little smoother than needing to compile the tree sitter syntax you could compile/bundle grammars up with wasm so its sandboxed and cross platform Edit: found this vscode extension that dynamically loads syntaxes compiled to wasm. You should be able to do the same thing in rust: https://github.com/selfint/vscode-tree-sitter https://github.com/selfint/vscode-tree-sitter
- manx 2y agoGreat to see more work in automatic git conflict resolution! I'll definitely give this a try. My own attempt at such a tool involved character based diffing and patching. It is able to solve most trivial conflicts that git cannot: https://fdietze.github.io/blend/ https://fdietze.github.io/blend/ I never found the time to make a cli out of this.