doc: explain how to use Mergiraf with Jujutsu #317

senekor · 2025-04-07T21:18:52+02:00

senekor commented

2025-04-07 21:18:52 +02:00

No description provided.

👍 1 ❤️ 1

ada4a commented

2025-04-07 21:36:27 +02:00

Hi, thanks for the contribution! As an aspiring Jujutsu user myself, I was actually wondering whether I'd be able to use Mergiraf with it, so I'd say it'd be great to have it mentioned in docs (if @wetneb agrees)!

But AFAICT it won't quite work out of the box. As part of its work, Mergiraf parses conflict markers, the diff3-style ones in particular -- but Jujutsu uses different conflict markers by default. But (just checked), there is an option to use the ones from Git.

Could you try setting the option and testing with a couple of conflicts just to be sure?

Hi, thanks for the contribution! As an aspiring Jujutsu user myself, I was actually wondering whether I'd be able to use Mergiraf with it, so I'd say it'd be great to have it mentioned in docs (if @wetneb agrees)! But AFAICT it won't _quite_ work out of the box. As part of its work, Mergiraf parses conflict markers, the diff3-style ones in particular -- but Jujutsu uses different conflict markers by default. But (just checked), there is an [option](https://jj-vcs.github.io/jj/latest/conflicts/#alternative-conflict-marker-styles) to use the ones from Git. Could you try setting the option and testing with a couple of conflicts just to be sure?

ada4a reviewed

2025-04-07 21:55:54 +02:00

doc/src/usage.md Outdated

					
				@ -56,0 +60,4 @@

				$ jj git clone https://codeberg.org/mergiraf/example-repo

				$ cd example-repo

				$ jj new main other-branch@origin

				$ jj resolve --tool mergiraf

ada4a commented

2025-04-07 21:55:25 +02:00

Are you sure this will work? mergiraf when run by itself doesn't even do anything, you need at least mergiraf merge. And you'll also need to actually provide it with arguments/flags passed from Git (well, Jujutsu).

I think you will need to register Mergiraf as a merge tool after all: https://jj-vcs.github.io/jj/latest/config/#3-way-merge-tools-for-conflict-resolution. Unless, of course, Jujutsu can actually read Git's config files and reuse the merge driver definition from there??

Are you sure this will work? `mergiraf` when run by itself doesn't even do anything, you need at least `mergiraf merge`. And you'll also need to actually provide it with arguments/flags passed from Git (well, Jujutsu). I think you will need to register Mergiraf as a merge tool after all: https://jj-vcs.github.io/jj/latest/config/#3-way-merge-tools-for-conflict-resolution. _Unless_, of course, Jujutsu can actually read Git's config files and reuse the merge driver definition from there??

ada4a commented

2025-04-07 22:02:58 +02:00

Anyway, in case that sort of magic doesn't work, here's how you'd adapt the merge driver definition from Git to Jujutsu:

[merge-tools.mergiraf]
merge-args = ["merge", "$base", "$left", "$right", "-p", "$output", "-l", "$marker_length"]

at least I hope passing the subcommand there will work?..

Anyway, in case that sort of magic doesn't work, here's how you'd adapt the merge driver definition from Git to Jujutsu: ```toml [merge-tools.mergiraf] merge-args = ["merge", "$base", "$left", "$right", "-p", "$output", "-l", "$marker_length"] ``` at least I hope passing the subcommand there will work?..

senekor commented

2025-04-07 22:05:49 +02:00

Yes, this does work without configuration! I tested it. Jujutsu has builtin support for Mergiraf, it knows about how to prepare files (with git style conflict markers) for it and with which arguments to call it.

If you run jj config list --include-defaults merge-tools, you will see a list of merge tools that jj comes preconfigured with, including Mergiraf.

That is not well documented on the Jujutsu side though, I might open a PR for that as well. It should be easy for people to find out what merge tools are supported out-of-the box.

Yes, this does work without configuration! I tested it. Jujutsu has **builtin** support for Mergiraf, it knows about how to prepare files (with git style conflict markers) for it and with which arguments to call it. If you run `jj config list --include-defaults merge-tools`, you will see a list of merge tools that jj comes preconfigured with, including Mergiraf. That is not well documented on the Jujutsu side though, I might open a PR for that as well. It should be easy for people to find out what merge tools are supported out-of-the box.

senekor commented

2025-04-07 22:07:06 +02:00

Here's the default configuration of Mergiraf in Jujutsu:

merge-tools.mergiraf.program = "mergiraf"
merge-tools.mergiraf.merge-args = ["merge", "$base", "$left", "$right", "-o", "$output", "-l", "$marker_length", "--fast"]
merge-tools.mergiraf.merge-conflict-exit-codes = [1]

Here's the default configuration of Mergiraf in Jujutsu: ```toml merge-tools.mergiraf.program = "mergiraf" merge-tools.mergiraf.merge-args = ["merge", "$base", "$left", "$right", "-o", "$output", "-l", "$marker_length", "--fast"] merge-tools.mergiraf.merge-conflict-exit-codes = [1] ```

ada4a commented

2025-04-07 22:11:46 +02:00

Oh that's very cool! The documentation is indeed somewhat behind it seems -- it only mentions the following default merge drivers: "vscode", "vscodium", "meld", "kdiff3", and "vimdiff" (from the article linked above)

ada4a commented

2025-04-07 22:23:47 +02:00

I wonder why --fast is there though.. It enables fast mode, which, while being, well, faster, gives up the ability to solve some of the more complex conflicts.

Also the fact that I used -p while the config uses -o makes me wonder... aren't those options pretty much identical @wetneb? That would really be a shame, since adding -o was literally my first ever contribution to Mergiraf (#46). Here's how they are defined:

         /// The path to the file to write the merge result to
         #[arg(short, long, conflicts_with = "git")]
         output: Option<PathBuf>,
         /// Final path in which the merged result will be stored.
         /// It is used to detect the language of the files using the file extension.
         #[arg(short, long)]
         path_name: Option<PathBuf>,

I wonder why `--fast` is there though.. It enables [fast mode](https://mergiraf.org/architecture.html#fast-mode), which, while being, well, faster, gives up the ability to solve some of the more complex conflicts. Also the fact that I used `-p` while the config uses `-o` makes me wonder... aren't those options pretty much identical @wetneb? That would really be a shame, since adding `-o` was literally my first ever contribution to Mergiraf (#46). Here's how they are defined: https://codeberg.org/mergiraf/mergiraf/src/commit/8996970b17ef64f2bfd29336f20b54a97ebae15b/src/bin/mergiraf.rs#L75-L81

senekor commented

2025-04-07 22:48:15 +02:00

@ada4a wrote in #317 (comment):

I wonder why --fast is there though.. It enables fast mode, which, while being, well, faster, gives up the ability to solve some of the more complex conflicts.

There is a comment about it in the source code of Jujutsu:

Currently, the --fast flag often gives the best results because the structural merge algorithm doesn't always maintain comments and spacing in the merge output (even parts without conflicts), but you can remove it if you want to use a full structural merge.

I think I tried out Mergiraf around 0.4 and also encountered this very quickly. Additional line breaks were inserted into my files left and right. I stopped that attempt quickly. Maybe it's better now?

@ada4a wrote in https://codeberg.org/mergiraf/mergiraf/pulls/317#issuecomment-3750353: > I wonder why `--fast` is there though.. It enables [fast mode](https://mergiraf.org/architecture.html#fast-mode), which, while being, well, faster, gives up the ability to solve some of the more complex conflicts. There is a comment about it in the source code of Jujutsu: > Currently, the --fast flag often gives the best results because the structural merge algorithm doesn't always maintain comments and spacing in the merge output (even parts without conflicts), but you can remove it if you want to use a full structural merge. I think I tried out Mergiraf around `0.4` and also encountered this very quickly. Additional line breaks were inserted into my files left and right. I stopped that attempt quickly. Maybe it's better now?

ada4a commented

2025-04-08 07:32:02 +02:00

Oh, right, I can see that. It has become much become since then though :) The additional line breaks and comments were indeed a huge problem, but were mostly fixed with #98 (should be present in 0.5.0 I think). There were also problems with indentation in Rust (which is the language I'm using most of the time, so it was quite painful), which the just merged #312 should hopefully have resolved.

All in all, I think we will be able to recommend full structured merge (i.e. without --fast) starting with the next release:)

Oh, right, I can see that. It has become much become since then though :) The additional line breaks and comments were indeed a huge problem, but were mostly fixed with #98 (should be present in 0.5.0 I think). There were also problems with indentation in Rust (which is the language I'm using most of the time, so it was quite painful), which the just merged #312 should hopefully have resolved. All in all, I think we will be able to recommend full structured merge (i.e. without `--fast`) starting with the next release:)

senekor commented

2025-04-07 22:01:25 +02:00

That's a good point! I tested it and resolving conflicts manually with mergiraf solve <file> indeed doesn't work unless jj is configured with ui.conflict-marker-style = "git".

However, since Jujutsu users will always resolve conflicts manually, it would be better for them to do so using Jujutsu's native Mergiraf support with jj resolve --tool mergiraf <file>. Jujutsu will then automatically prepare the file in a way that Mergiraf undestands -- with git style conflict markers.

As a side benefit, Jujutsu has pretty smart command line completions and can complete only the conflicted files which need to be resolved as the argument to jj resolve --tool mergiraf. Pretty convenient.

So if we expect that Mergiraf users might accidentally use mergiraf solve <fiel>, it might be better to tell people not to do that at all in the documentation.

A more intrusive option would be to make Mergiraf detect a .jj directory next to the .git one when mergiraf solve is invoked and print an error informing the user they should use jj resolve instead.

That's a good point! I tested it and resolving conflicts manually with `mergiraf solve <file>` indeed **doesn't work** unless jj is configured with `ui.conflict-marker-style = "git"`. However, since Jujutsu users will always resolve conflicts manually, it would be better for them to do so using Jujutsu's native Mergiraf support with `jj resolve --tool mergiraf <file>`. Jujutsu will then automatically prepare the file in a way that Mergiraf undestands -- with git style conflict markers. As a side benefit, Jujutsu has pretty smart command line completions and can complete only the conflicted files which need to be resolved as the argument to `jj resolve --tool mergiraf`. Pretty convenient. So if we expect that Mergiraf users might accidentally use `mergiraf solve <fiel>`, it might be better to tell people not to do that at all in the documentation. A more intrusive option would be to make Mergiraf detect a `.jj` directory next to the `.git` one when `mergiraf solve` is invoked and print an error informing the user they should use `jj resolve` instead.

ada4a commented

2025-04-07 22:29:46 +02:00

@senekor wrote in #317 (comment):

So if we expect that Mergiraf users might accidentally use mergiraf solve <fiel>, it might be better to tell people not to do that at all in the documentation.

A more intrusive option would be to make Mergiraf detect a .jj directory next to the .git one when mergiraf solve is invoked and print an error informing the user they should use jj resolve instead.

People are fallible... I would indeed like to have some programmatic way of detecting Jujutsu, but looking for a .jj directory feels a bit hacky to me

@senekor wrote in https://codeberg.org/mergiraf/mergiraf/pulls/317#issuecomment-3749126: > So if we expect that Mergiraf users might accidentally use `mergiraf solve <fiel>`, it might be better to tell people not to do that at all in the documentation. > > A more intrusive option would be to make Mergiraf detect a `.jj` directory next to the `.git` one when `mergiraf solve` is invoked and print an error informing the user they should use `jj resolve` instead. People are fallible... I would indeed like to have some programmatic way of detecting Jujutsu, but looking for a `.jj` directory feels a bit hacky to me

senekor commented

2025-04-07 22:44:34 +02:00

People are fallible... I would indeed like to have some programmatic way of detecting Jujutsu, but looking for a .jj directory feels a bit hacky to me

We could also spawn jj root. If anything goes wrong or the exit code is not 0, the user is probably not using jj. Ideally we'd only do that when something has already gone wrong so as not to pay the overhead of spawning a process on the happy path.

> People are fallible... I would indeed like to have some programmatic way of detecting Jujutsu, but looking for a `.jj` directory feels a bit hacky to me We could also spawn `jj root`. If anything goes wrong or the exit code is not `0`, the user is probably not using jj. Ideally we'd only do that when something has already gone wrong so as not to pay the overhead of spawning a process on the happy path.

ada4a reviewed

2025-04-07 22:49:05 +02:00

doc/src/usage.md Outdated

					
				@ -7,6 +7,8 @@ There are two ways to use Mergiraf.

				The first way is recommended as it avoids interrupting your workflow with spurious conflicts.

				The second way can be useful for more occasional uses or when changes to Git's configuration are not possible.

				[**Jujutsu**](https://jj-vcs.github.io/jj) users can invoke Mergiraf with `jj resolve --tool mergiraf` without any configuration.

ada4a commented

2025-04-07 22:49:05 +02:00

I think "without any configuration" was what tripped me up -- there is configuration after all, it's just present by default. I'd prefer something like this instead:

In Jujutsu, Mergiraf is configured as a merge driver by default (<link to docs, eventually>), so it can be invoked using jj resolve --tool mergiraf

Also I think the placement is kind of out-of-place here. I wouldn't want to just shove Jujutsu info back somewhere of course... but it will require a (short) section on configuring Mergiraf as a merge driver, as well as a section about interactive use (talking about jj resolve)

I think "without any configuration" was what tripped me up -- there _is_ configuration after all, it's just present by default. I'd prefer something like this instead: > In Jujutsu, Mergiraf is configured as a merge driver by default (<link to docs, eventually>), so it can be invoked using `jj resolve --tool mergiraf` Also I think the placement is kind of out-of-place here. I wouldn't want to just shove Jujutsu info back somewhere of course... but it will require a (short) section on configuring Mergiraf as a merge driver, _as well as_ a section about interactive use (talking about `jj resolve`)

ada4a commented

2025-04-07 23:04:14 +02:00

Well, okay, both of those sections will basically boil down to "just use jj resolve", so maybe we could combine them into a section after "interactive use", and link to it from the start of the article.

Well, okay, both of those sections will basically boil down to "just use `jj resolve`", so maybe we could combine them into a section after "interactive use", and link to it from the start of the article.

senekor commented

2025-04-08 01:25:48 +02:00

Alright, I have added more extensive documentation at the bottom and referred to it from the top. Let me know if you have more ideas for improvements!

ada4a commented

2025-04-07 22:57:22 +02:00

We could also spawn jj root. If anything goes wrong or the exit code is not 0, the user is probably not using jj. Ideally we'd only do that when something has already gone wrong so as not to pay the overhead of spawning a process on the happy path.

That could be an option, yes. And the "gone wrong" part is especially important -- currently, we parse conflicts using regex (I really tried not to, but alas), and jj's default conflict markers won't get recognized by it at all, possibly leading to clobbering. We already compile a separate regex to recognize diff2-style conflicts, and the costs are adding up (especially since the parsing function can be called multiple times during runtime) -- I'd like to avoid needing to add another one if possible.

I have indeed been thinking about extracting the "validation" regex into a separate method (since we realistically need it only once, when parsing the input file) -- we could add the jj regex there as well

> We could also spawn `jj root`. If anything goes wrong or the exit code is not `0`, the user is probably not using jj. Ideally we'd only do that when something has already gone wrong so as not to pay the overhead of spawning a process on the happy path. That could be an option, yes. And the "gone wrong" part is especially important -- currently, we parse conflicts using regex (I [_really_](https://codeberg.org/mergiraf/mergiraf/pulls/185#issuecomment-2678688) tried not to, but alas), and jj's default conflict markers won't get recognized by it at all, possibly leading to clobbering. We already compile a [separate regex](https://codeberg.org/mergiraf/mergiraf/src/commit/8996970b17ef64f2bfd29336f20b54a97ebae15b/src/parsed_merge.rs#L87-L96) to recognize diff2-style conflicts, and the costs are adding up (especially since the parsing function can be called multiple times during runtime) -- I'd like to avoid needing to add another one if possible. I have indeed been thinking about extracting the "validation" regex into a separate method (since we realistically need it only once, when parsing the input file) -- we could add the jj regex there as well

~~senekor referenced this pull request 2025-04-07 23:16:25 +02:00~~

feat(mergiraf solve): improve error output for jj users #318

senekor commented

2025-04-07 23:20:01 +02:00

What would the new regex be for? Recognizing Jujutsu style conflict markers? I don't think that should be necessary. jj resolve should always be preferred (unless I'm missing something), so I think failing with a good error message is the best we can do.

What would the new regex be for? Recognizing Jujutsu style conflict markers? I don't think that should be necessary. `jj resolve` should always be preferred (unless I'm missing something), so I think failing with a good error message is the best we can do.

ada4a commented

2025-04-07 23:46:33 +02:00

Yeah, just validating -- I'm of course not suggesting we try and solve those. As I said, we do the same with diff2 currently -- if we recognize it, we effectively return a specific kind of Err, which we then match on and give an appropriate error message. You can grep for PARSED_MERGE_DIFF2_DETECTED to find out more.

Yeah, just validating -- I'm of course not suggesting we try and solve those. As I said, we do the same with diff2 currently -- if we recognize it, we effectively return a specific kind of `Err`, which we then match on and give an appropriate error message. You can grep for `PARSED_MERGE_DIFF2_DETECTED` to find out more.

ada4a referenced this pull request

2025-04-08 00:10:18 +02:00

feat(mergiraf solve): improve error output for jj users #318

senekor force-pushed senekor/doc-usage-with-jujutsu from c9d3f0b777 to 49a5a9d2c9

2025-04-08 01:24:31 +02:00

Compare

ada4a commented

2025-04-08 07:41:34 +02:00

I'll look at the latest changes later today, thanks in advance!

In the meantime, here's another question about with Jujutsu:
Does it have something similar to .gitattributes, so that one can set-up Mergiraf as the default merge driver for certain file types (=languages)? We do eventually want to make it possible to set it as the default globally, so that it just delegates to the line-based merge when the language isn't recognized, but we're not quite there yet – but for now, it's quite important to set this, to avoid specifying it each time manually. I imagine it would avoid the need to add --tool mergiraf after jj resolve

I'll look at the latest changes later today, thanks in advance! In the meantime, here's another question about with Jujutsu: Does it have something similar to `.gitattributes`, so that one can set-up Mergiraf as the default merge driver for certain file types (=languages)? We do eventually want to make it possible to set it as the default globally, so that it just delegates to the line-based merge when the language isn't recognized, but we're not quite there yet – but for now, it's quite important to set this, to avoid specifying it each time manually. I imagine it would avoid the need to add `--tool mergiraf` after `jj resolve`

ada4a commented

2025-04-08 08:32:41 +02:00

I'll look at the latest changes later today, thanks in advance!

Ah, it's looking very nice! Thanks for the detailed descriptions:)

Given that, as you say in the section, Jujutsu doesn't resolve conflicts manually, the .gitattributes thing might be less relevant. But do let me know if you have more context around it!

> I'll look at the latest changes later today, thanks in advance! Ah, it's looking very nice! Thanks for the detailed descriptions:) Given that, as you say in the section, Jujutsu doesn't resolve conflicts manually, the `.gitattributes` thing might be less relevant. But do let me know if you have more context around it!

senekor commented

2025-04-08 10:01:38 +02:00

Yeah, it's not possible to run mergiraf automatically, so the .gitattributes file is not necessary / has no effect. Do you think that should be pointed out explicitly in the section about Jujutsu?

Yeah, it's not possible to run mergiraf automatically, so the `.gitattributes` file is not necessary / has no effect. Do you think that should be pointed out explicitly in the section about Jujutsu?

ada4a commented

2025-04-08 10:09:46 +02:00

I guess those who already use Jujutsu may already know this, but I think it's worth mentioning nevertheless.

senekor force-pushed senekor/doc-usage-with-jujutsu from 49a5a9d2c9 to eb4b411de1

2025-04-08 10:40:51 +02:00

Compare

senekor commented

2025-04-08 10:41:15 +02:00

@ada4a wrote in #317 (comment):

I guess those who already use Jujutsu may already know this, but I think it's worth mentioning nevertheless.

done 👍

@ada4a wrote in https://codeberg.org/mergiraf/mergiraf/pulls/317#issuecomment-3753350: > I guess those who already use Jujutsu may already know this, but I think it's worth mentioning nevertheless. done 👍

ada4a commented

2025-04-09 21:28:17 +02:00

Love it, thanks again! Happy to merge once we have the second approval:)

❤️ 1

wetneb approved these changes

2025-04-09 22:45:38 +02:00

wetneb left a comment

Looking great! The update will be published on mergiraf.org at the next release (let's do one soon, no?)

🎉 1 🚀 1

ada4a commented

2025-04-09 23:44:07 +02:00

A quick question @senekor:

On Codeberg, when a PR is squash-merged (which is what we usually do), the commit message is taken from the PR description. That surprised me when my first PR got merged, so I just wanted to make sure that that's okay by you, or maybe you want something different.

A quick question @senekor: On Codeberg, when a PR is squash-merged (which is what we usually do), the commit message is taken from the PR description. That surprised me when my first PR got merged, so I just wanted to make sure that that's okay by you, or maybe you want something different.

senekor commented

2025-04-10 00:52:51 +02:00

Oh, thanks for the heads up! I'm used to the commit message being retained as-is. I removed the PR description, I think it would've been unhelpful clutter in the history. The change seems simple enough for just a subject line, but do let me know if you'd like me to write a more detailed description for the history.

ada4a commented

2025-04-10 01:04:07 +02:00

I'm used to the commit message being retained as-is

Yeah I expected something like that as well. Though one can in fact do that – if you submit a PR consisting of a single commit, its subject/body will turn into PR name/description, respectively (you can of course further modify them as you discuss the PR), and then will happily migrate to the squash commit subject/body.

I removed the PR description

Oh there was no need to 😅 I could've just edited the commit message when merging! I guess I could be more clear about that.

But anyway, I agree the subject line is enough here. I think the discussion above would be plenty for anyone wishing to learn more context:)

> I'm used to the commit message being retained as-is Yeah I expected something like that as well. Though one can in fact do that – if you submit a PR consisting of a single commit, its subject/body will turn into PR name/description, respectively (you can of course further modify them as you discuss the PR), and then will happily migrate to the squash commit subject/body. > I removed the PR description Oh there was no need to 😅 I could've just edited the commit message when merging! I guess I could be more clear about that. But anyway, I agree the subject line is enough here. I think the discussion above would be plenty for anyone wishing to learn more context:)

ada4a approved these changes

2025-04-10 01:04:47 +02:00

ada4a left a comment

Just to have me in the trailer as well:)

ada4a merged commit aa787aa902 into main

2025-04-10 01:05:06 +02:00

ada4a referenced this pull request from a commit

2025-04-10 01:05:07 +02:00

doc: explain how to use Mergiraf with Jujutsu (#317)

senekor referenced this pull request from a commit

2025-04-12 20:05:12 +02:00

doc: explain how to use Mergiraf with Jujutsu (#317)