Skip to content

Conversation

@poliorcetics
Copy link
Contributor

@poliorcetics poliorcetics commented Nov 7, 2024

Built on top of #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

@the-mikedavis the-mikedavis added the A-vcs Area: Version control system interaction label Nov 7, 2024
@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 2 times, most recently from d07b9ff to 601e3a5 Compare November 14, 2024 20:46
@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 2 times, most recently from 422e761 to fc7c8d1 Compare November 27, 2024 19:09
@yerlaser
Copy link
Contributor

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?

@VasanthakumarV
Copy link

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

@poliorcetics
Copy link
Contributor Author

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 :)

@bryceberger
Copy link
Contributor

bryceberger commented Jan 5, 2025

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 d4a0e44. 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
Copy link
Contributor

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: a8a105d). Needs better integration with helix's async code, doesn't solve custom backends. Otherwise seems to work.

@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 2 times, most recently from da5d835 to 035ccd0 Compare February 2, 2025 13:23
@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 035ccd0 to b58879e Compare March 29, 2025 12:02
@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from b58879e to 8b10cc2 Compare April 12, 2025 17:44
@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 8b10cc2 to 5e0f205 Compare May 1, 2025 10:58
@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 5e0f205 to 30dd7e2 Compare May 14, 2025 16:59
@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from 30dd7e2 to ec98a74 Compare June 16, 2025 21:30
@yerlaser
Copy link
Contributor

yerlaser commented Jul 1, 2025

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.

@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from ec98a74 to ee0e561 Compare July 19, 2025 15:13
@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch from ee0e561 to f597775 Compare August 5, 2025 21:28
@icorbrey
Copy link
Contributor

icorbrey commented Aug 7, 2025

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

@yerlaser
Copy link
Contributor

yerlaser commented Aug 7, 2025

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.

@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 5 times, most recently from 5b21777 to dde2d25 Compare August 15, 2025 23:23
@RoloEdits
Copy link
Contributor

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.

@poliorcetics
Copy link
Contributor Author

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 poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 4 times, most recently from 720f86d to 833f25a Compare August 16, 2025 13:35
@poliorcetics
Copy link
Contributor Author

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

@poliorcetics poliorcetics force-pushed the ab/push-rlymwpyuzrrw branch 6 times, most recently from c2f51a9 to 18eb0df Compare August 16, 2025 23:49
@RoloEdits

This comment was marked as resolved.

@RoloEdits
Copy link
Contributor

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.

@poliorcetics
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@RoloEdits
Copy link
Contributor

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.

@tingerrr
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-vcs Area: Version control system interaction

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants