-
Notifications
You must be signed in to change notification settings - Fork 343
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
merge_tools: create builtin diff editor #2117
Conversation
c4fceae
to
a73431b
Compare
803d4a4
to
0f96d3b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, what's the benefit of API-level integration?
It's really nice that we have an integrated diff/merge editor, but I think it could also be achieved by spawning a separate binary or hidden jj subcommand (like jj scm-diff-editor
.) Since we can't drop support for external tools, using the existing entry point might be simpler.
As an aside, I am wondering about using This is related to a suggestion I once made on Discord to allow OTOH, we could go into the opposite direction and use something much simpler than |
@yuja I want to integrate
Also, you could potentially avoid materializing large (binary?) files on disk, since they're only rendered as hashes in the UI. We don't have an interface to communicate that kind of thing with the difftool/mergetool, and, since no other tools support anything like that (as far as I know?), it would just slow down development to try to create a stable command-line interface to do all that. We can establish an interface later if someone else ever tries to make such a tool.
@ilyagr That sounds reasonable and in-scope to me, although scm-record might have to recompute a diff in that case in order to render it after delegating to the external difftool. Computing a new diff could be a little involved because the VCS can often compute more sensible diffs than scm-record (it may know to ignore whitespace, etc., on the request of the user). I suppose that's another scenario where tight coupling between the VCS and scm-record would help.
Skim will probably be better indefinitely for keyboard shortcuts and fuzzy-matching, since it's hard to get good UX for that kind of thing (e.g. supporting Vim, Emacs, OS keybindings in text areas), and it's hard to compose TUI tools/libraries in Rust-land (can't just embed Skim into the scm-record interface somehow). It would be ideal to be able to search through files (and file contents) in scm-record, but there are only so many hours in the day! |
a73431b
to
759e27b
Compare
144b08d
to
a06f030
Compare
759e27b
to
b549f67
Compare
715b590
to
a375859
Compare
b549f67
to
7226334
Compare
a375859
to
ee77c67
Compare
d434c1e
to
9533a19
Compare
ee77c67
to
3e26b49
Compare
This commit will probably need another look for the latest |
3e26b49
to
82f2e6b
Compare
9533a19
to
50f6e06
Compare
82f2e6b
to
5dd2cfa
Compare
Checklist
If applicable:
CHANGELOG.md