helix icon indicating copy to clipboard operation
helix copied to clipboard

feat: vcs: support Jujutsu as a diff-provider

Open poliorcetics opened this issue 1 year ago • 18 comments

Built on top of https://github.com/helix-editor/helix/pull/9951, I'll be waiting for it to be merged to un-draft this


Jujutsu (jj) is a new change-based VCS (whereas git is branch-based).

In this PR, I add the ability for helix to get the diffs and current head, behind a feature called jj that is active by default. That would make it the first editor I know off that officially handles that VCS!

To handle all current, future and private backends (Google already has one I believe), I instead made it so Helix can behave as a diff tool for Jujutsu and then use that as a subcommand to get the diff base.

For the head, I simply used the templating system to extract all relevant informations.

Jujutsu has a library, called jj-lib but it's not ready for use in third party programs and wouldn't fix the issue of custom backends anyway.

Testing

Since testing needs jj installed, I haven't written any for Helix yet to discuss how to do it:

  1. Ignore the issue. I don't like it much, but it doesn't cost CI time and doesn't need any change. For a feature that probably won't be used much yet, that's probably ok for a time
  2. Install jj in CI and use it to test the new feature

poliorcetics avatar Nov 07 '24 14:11 poliorcetics

Hi @poliorcetics,

Thanks for this PR, hope it will be merged soon. Quick question. I just built from it but all I get the change hash in the status line but no diff indicators in the gutter. Did I miss something?

yerlaser avatar Dec 16 '24 23:12 yerlaser

Hi @poliorcetics,

Thank you for working on integrating jj into helix, I am using helix from your branch, and it has been great so far.

I am new to jj, and this PR is still a draft, but wanted to make a suggestion on the status-line entry,

Will description.first_line() be more useful thanchange_id in the status-line message?

Change-Id: image

Description: image

VasanthakumarV avatar Dec 24 '24 06:12 VasanthakumarV

I am new to jj, and this PR is still a draft, but wanted to make a suggestion on the status-line entry,

Will description.first_line() be more useful thanchange_id in the status-line message?

It could easily be done code-wise, but I'm trying to get the same behavior as the git backend (to make it easier for this PR to get in), so I don't think I'll do it

If maintainers confirm it would be ok to add, I'll do it :)

poliorcetics avatar Dec 26 '24 21:12 poliorcetics

This gets a bit weird with reloading newly-added or untracked files.

  1. Create a new file, add some content. Nothing shows up in the gutter (expected).
  2. Reload the file with :reload. Still nothing in gutter (expected).
  3. Edit the file. Edits show up as changes (unexpected!!)

Each time the file is reloaded, the diff base is set to the current file.

I have somewhat of a fix for this at d4a0e44dc09c2e3d. If the file is untracked, return early. If the file is new, return an empty base. Otherwise, continue as before.

I'm not really a fan of my implementation, since it relies on jj emitting a warning message if the file isn't found in the repo. Could do without that by doing a separate (jj file list).contains(file_relative_to_root), but I didn't want to add another jj invocation.

bryceberger avatar Jan 05 '25 07:01 bryceberger

Was curious to see what an implementation using jj-lib would look like. Have a proof of concept at https://github.com/bryceberger/helix/tree/jj-lib (permalink: a8a105d3445a92ff77b85515254e1ef661ee4959). Needs better integration with helix's async code, doesn't solve custom backends. Otherwise seems to work.

bryceberger avatar Jan 06 '25 06:01 bryceberger

Looks like this feature doesn't always work. Often it fails to show any diffs.

I experienced this on a non-collocated repo as well as (albeit less often) on pure Git repos.

yerlaser avatar Jul 01 '25 15:07 yerlaser

I'm not getting any gutter information in a pure JJ repo, and there aren't any logs to suggest something went wrong. Does this PR not provide gutter information? If not could we? I'd be happy to hack on this and see if I can get it going

icorbrey avatar Aug 07 '25 14:08 icorbrey

I'm not getting any gutter information in a pure JJ repo, and there aren't any logs to suggest something went wrong. Does this PR not provide gutter information? If not could we? I'd be happy to hack on this and see if I can get it going

It worked intermittently for me as well.

I suspect it hits some threshold timeout in case the repo is large enough. You can try to init a blank repo and try it there to check if it's the same issue as I faced.

yerlaser avatar Aug 07 '25 14:08 yerlaser

Just fixed lots of bug that were due to JJ doing breaking changes (which is fine, they did correct semver for them) and this MR being 5 versions behind.

It should now correctly:

  • Detect current change ID (and bookmark, if current change is on any)
  • Modified/Removed/Untracked files (NOTE: if you have unresolved conflicts, it will not: https://github.com/jj-vcs/jj/issues/7264)
  • Compute diffs (may fail for the same reason as previous)

All of these requires you to either have a daemon watching file changes or running jj command one way or another, Helix by itself always does --ignore-working-copy to avoid creating dozens of updates of the JJ state as you save files.


I also fixed many edge-case bugs around rename detection by using jj diff --template ... instead of jj diff --types: you can now have { or \n in you filename without breaking it for example

poliorcetics avatar Aug 15 '25 23:08 poliorcetics

I havent tried this yet, but, for example, would this handle finding the most recent bookmark in the decedents, and then use that for the "branch" name, like how git has?

Its nice for the command line to have the revisions and not care so much about the bookmarks, but in the editor it would be nice to know what the direct parent is for what I am working on. Sorry if this is out of scope.

RoloEdits avatar Aug 16 '25 04:08 RoloEdits

would this handle finding the most recent bookmark in the decedents, and then use that for the "branch" name, like how git has?

That not how Git works: Git only finds the "current branch" because your current state is not a commit, it's the staging area: if you use detached HEAD mode with a branch on the previous commit, Git will not find the branch.

Still, I added this feature because I can see it being requested often.

If exactly on a bookmark it will look like kmlpqmrv (exact-1) and if the bookmark(s) are on ancestors it will look like sxrrsnun [anc-1 anc-2]

poliorcetics avatar Aug 16 '25 12:08 poliorcetics

Fixed the issue around conflicts, they are now correctly listed in repo changes 🎉

poliorcetics avatar Aug 16 '25 13:08 poliorcetics

Maybe you fixed it already (or I am doing something wrong when merging this in), but, for example, doing a jj new master on helix, all I see is: image

I would expect to see [master].

RoloEdits avatar Aug 16 '25 23:08 RoloEdits

Ok, now I see: image

This makes sense in essence, but I wonder if there is only a single direct ancestor bookmark should it only display that bookmark? For example, here I have master -> build but it displays both master and build. As there is only a single bookmark found first, should it stop there and only display build?

If there was a "merge" with two ancestor bookmarks, then maybe it would make more sense to have them both displayed? feat/bookmark-render -> current rev and fix/ui-component-render -> current rev would display [feat/bookmark-render fix/ui-component-render]? And then if you make a new bookmark on that merge change, like dev/merge-x-y it would just display that bookmark name, [dev/merge-x-y] again? Not sure if anyone else has any thoughts on how this should be displayed.

It makes sense to me to only display a single bookmark if it has a linear history, but the rules around merge bookmarks are more complicated.

RoloEdits avatar Aug 17 '25 00:08 RoloEdits

This makes sense in essence, but I wonder if there is only a single direct ancestor bookmark should it only display that bookmark?

The issue is, in a single call to jj log I cannot find out if the bookmarks are parallel or chained because JJ templates don't have loops, so I erred on the side of caution of listing everything. The "closest" bookmarks will come first which is good enough IMO, and running multiple JJ commands would be expensive for very little gain here (measurably expensive, if you use the picker for VCS files you'll see a small delay because I have to run two commands to account for conflicts and it's fairly visible)

poliorcetics avatar Aug 17 '25 00:08 poliorcetics

Also, because of jj absorb existing (and being very cool), the latest bookmark is not necessarily the one your current modifications will go into, especially if you're like me and stack PRs a lot (at $work I've stack more than 10 at once a few times), in which case listing more of them is nice.

Ultimately this is a situation where I prefer to display more information than less, though it would be cool to find a single-command way to order bookmarks (e.g. [level-1/b1 level-1/b2 > base > main]), but I don't think it's possible

poliorcetics avatar Aug 17 '25 00:08 poliorcetics

Ahh, good point! I hadnt thought of the other interactions you can have like absorb. I would agree then that more is better. Its also more then necessary for an initial PR. Good work on this, it may not be much, but not having a raw commit hash is very welcome. The context is now fully back.

RoloEdits avatar Aug 17 '25 00:08 RoloEdits

Thank you for maintaining this! I've used this as inspiration for adding mercurial support for myself.

tingerrr avatar Nov 28 '25 15:11 tingerrr