Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a TUI for editing diffs #48

Closed
martinvonz opened this issue Dec 18, 2021 · 16 comments
Closed

Add a TUI for editing diffs #48

martinvonz opened this issue Dec 18, 2021 · 16 comments
Labels
help wanted Extra attention is needed

Comments

@martinvonz
Copy link
Owner

martinvonz commented Dec 18, 2021

Surprisingly, there doesn't seem to be a good TUI tool for editing diffs. We need this for jj touchup, jj split, etc. We want a tool that can present the diff and let the user edit the right side of it. It should ideally support diffing directories. vimdiff might be the closest, but it doesn't support directories by default (it seems you need to install some plugin) and it's not intuitive.

@arxanas
Copy link
Collaborator

arxanas commented Dec 18, 2021

This would also be useful for git-branchless for the same reasons.

The TUI which I was used to with hg split met the majority of my needs, in which the diffs are displayed inline and you just select the lines you want to keep. For more complicated cases, it might be better to allow the user to shell out to their mergetool for a given file, to keep implementation complexity down.

@martinvonz
Copy link
Owner Author

This would also be useful for git-branchless for the same reasons.

Yes. If I start working on a TUI for editing diffs, I'll probably start making it part of jj, but then I hope to make it standalone. That probably involves moving out diffing to a separate crate so it can be depended on from both jujutsu-lib and this new diff editor.

The TUI which I was used to with hg split met the majority of my needs, in which the diffs are displayed inline and you just select the lines you want to keep.

That's also the UI I'm used to. The biggest problem I have with that is that it's really hard to edit the patch itself (after pressing e in that UI). I prefer how Meld instead lets you edit the end state itself. Inline diffs may still be best for most cases and it's just the "edit" action that should be different. I think hg's interface was too tailored to editing the literal patch that it wasn't possible to make that change there. I'd like the diff editor to get two full texts as input and figure out the diffing itself.

For more complicated cases, it might be better to allow the user to shell out to their mergetool for a given file, to keep implementation complexity down.

Except that there I don't know of any good terminal-based mergetool... :(

@arxanas
Copy link
Collaborator

arxanas commented Dec 19, 2021

What are the deficiencies of the terminal-based mergetools which you've used?

@martinvonz
Copy link
Owner Author

I don't think I've used any at all. I've started vimdiff but I couldn't figure out what to do after starting it. Last time I looked for tools, I remember reading about another tool called sdiff, but I'm not sure that even has a TUI.

Can you recommend any?

@arxanas
Copy link
Collaborator

arxanas commented Dec 19, 2021

I was using vimdiff as launched from git mergetool. It looks like this (not sure if it's what you're seeing):

Screen Shot 2021-12-19 at 5 34 24 PM

The bottom window is the actual buffer you want to edit. The top three show the "local", "base", and "remote" snapshots of the file for your reference (i.e. the components of the three-way diff). The diff coloration updates as you edit, and all the buffers should scroll in sync. You could also simply edit one of the above three windows and overwrite the target file, if you prefer not to use the conflict marker mechanism.

@martinvonz
Copy link
Owner Author

martinvonz commented Dec 20, 2021

Thanks. I just tried vimdiff again on a diff (i.e. two input files). I didn't see any instructions anywhere and the somewhat common '?' shortcut (shift+/ on my keyboard) didn't work either. That's why I said earlier that I don't find it intuitive. Mercurial's UI for hg split, on the other hand, has a line at the top with some brief instructions and tells me I can press '?' for more detailed help. I'd really like my diff/merge tool to have something like that.

@martinvonz martinvonz added the help wanted Extra attention is needed label Mar 24, 2022
martinvonz added a commit that referenced this issue Apr 9, 2022
I quite often want to move the changes to a particular file from one
commit to another. We already support that using `jj move -i`, but
that can be annoying to run because we don't have a TUI for it
(#48). Let's make it possible to do `jj move --from X --to Y <path>`.
martinvonz added a commit that referenced this issue Apr 9, 2022
I quite often want to move the changes to a particular file from one
commit to another. We already support that using `jj move -i`, but
that can be annoying to run because we don't have a TUI for it
(#48). Let's make it possible to do `jj move --from X --to Y <path>`.
martinvonz added a commit that referenced this issue Apr 9, 2022
I quite often want to move the changes to a particular file from one
commit to another. We already support that using `jj move -i`, but
that can be annoying to run because we don't have a TUI for it
(#48). Let's make it possible to do `jj move --from X --to Y <path>`.
@waldyrious
Copy link

waldyrious commented Apr 9, 2022

As an additional data point, git-crecord offers a TUI for selecting changes to stage/commit on an line-by-line basis. IIUC it doesn't allow actually editing the diff contents, but it has a dynamic expanding/collapsing interface that may be an interesting feature to consider (although a one-hunk-at-a-time approach like what git add -p does would likely make that unnecessary).

@martinvonz
Copy link
Owner Author

Thanks. I'd seen git-crecord, and I've used the orginal implementation for Mercurial a lot, but I had somehow not considered using it with jj :) Most of the time you don't need to edit the diff, so it seems fine if that's not supported to start with. I haven't spent much time looking through the code, but I would think most of it is reusable outside of git. I know Mercurial's crecord code actually works on the diff itself, while jj's configurable diff-editor works on two states (which tools like meld expect as input). We could probably create a wrapper that calculates a diff and calls some lower-level functions in git-crecord, and then applies the diff to the left input at the end.

Also, rumor has it that @arxanas is working on a TUI like this for (presumably for https://github.com/arxanas/git-branchless, but should be reusable).

martinvonz added a commit that referenced this issue Apr 9, 2022
I quite often want to move the changes to a particular file from one
commit to another. We already support that using `jj move -i`, but
that can be annoying to run because we don't have a TUI for it
(#48). Let's make it possible to do `jj move --from X --to Y <path>`.
martinvonz added a commit that referenced this issue Apr 9, 2022
I quite often want to move the changes to a particular file from one
commit to another. We already support that using `jj move -i`, but
that can be annoying to run because we don't have a TUI for it
(#48). Let's make it possible to do `jj move --from X --to Y <path>`.
@arxanas
Copy link
Collaborator

arxanas commented Apr 10, 2022

PR for the TUI which I'm working on: arxanas/git-branchless#343

Embedding the git-crecord Python library/binary might be a pragmatic option, too, although it's GPL-2-only.

@max-sixty
Copy link

Is there a config to use VSCode for this in the meantime? I tried ui.diff-editor = "code --wait --diff $LOCAL $REMOTE" but that wasn't successful.

@yuja
Copy link
Collaborator

yuja commented Feb 5, 2023

Try something like this. ui.diff-editor doesn't support list of arguments nor "$variable" expansion right now.

ui.diff-editor = "vscode"

[merge-tools.vscode]
program = "code"
edit-args = ["--wait", "--diff"]

https://github.com/martinvonz/jj/blob/b22a6db7e7ec4bae901fe4586ec771a8a3d9e8b3/src/config/merge_tools.toml

@max-sixty
Copy link

Thanks for suggesting something @yuja

FYI this does open VSCode with the files, but not a diff editor. But no great stress...

@yuja
Copy link
Collaborator

yuja commented Feb 7, 2023

Update: #1211 added variable expansion, and $left/$right is now required unless ui.diff-editor is a program name.

ui.diff-editor = "code"  # equivalent to ["code", "$left", "$right"]
ui.diff-editor = ["code", "--wait", "--diff", "$left", "$right"] 

(I have no idea about the option to forcibly open a diff editor.)

@ilyagr
Copy link
Collaborator

ilyagr commented Feb 8, 2023

VS Code doesn't support diffing directories out of the box, only one file at a time. I found https://marketplace.visualstudio.com/items?itemName=moshfeu.compare-folders that claims to be runnable from the command line, but I haven't tried it.

A script could also be written. I'm not sure what the best approach is, but it should be possible using diff -r or the Node.js library the extension I linked uses.

@arxanas
Copy link
Collaborator

arxanas commented Nov 10, 2023

Shall we close this? We now bundle scm-record into jj, and we can invoke external diff editors as desired. It's true that scm-record doesn't let you edit hunks directly, which is sometimes useful. It might be better to open a feature request for that specifically.

@martinvonz
Copy link
Owner Author

Shall we close this?

Yes, I agree. Thanks for creating scm-record and the integration!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants