external diff editors: extract utility functions to a separate file in preparation for #3292 #3296
Conversation
…TRUCTIONS This will be reused for integration with the new `:builtin-web` diff editor in jj-vcs#3292. `instructions-path_to_cleanup` is moved into DiffWorkingCopies. DiffWorkingCopies: add instructions_path_to_cleanup
This is preparation for jj-vcs#3292, which will use these functions. The main goal is to merge the parts of jj-vcs#3292 that are likely to cause merge conflicts with other PRs while I polish it up.
ilyagr
changed the title
There was a problem hiding this comment.
Regarding #3292, I don't think we need to link the GUI frontend with the main jj executable so long as the GUI relies on the same machinery as the external diff frontends. Bundling the default GUI editor might be nice, but they don't have to be the same executable. IIRC, we have :builtin TUI mainly because it does something that can't be achieved within the external diff interface.
That said, this PR looks good as its own, thanks.
I'm not sure why you feel this way, how strong your feelings are, or if you see some approach here that I don't, so I just wrote down a bunch of my thoughts. My goal here is to have a 0-configuration GUI that can work anywhere Requiring people to find, install, and configure the tool before they could use it would defeat most of its purpose. Anybody who can use and is willing to install Meld can use that already. On a Mac, or over SSH (to another machine, a VM, etc), using Meld is difficult or impossible even for people who are willing to put some time into it, and they get a sub-optimal I don't know an easy way to bundle the executables together without requiring configuration; getting We can have the diff editor be a Cargo feature, so people who want to save 1.5MB of binary size could compile jj without it. (Also, perhaps this could be the default for incremental/debug builds)
I'm not sure that's true, what can it do that an external binary couldn't? I was and am considering making it possible for One way to think about it is as analogous to linking in a built-in pager on Windows. The difference is that there is no platform where a good enough diff editor is provided out of the box.
Thanks for the review! I'll merge it tomorrow, I think. |
|
BTW, I'd prefer having a TUI three-pane-diff tool to a local server webapp, but that's harder to make. The easiest way for me would be to use Vim (improve the DirDiff extension or vimtabdiff), but plenty of people would never use something with Vim keybindings. I was considering making something based on https://github.com/zyedidia/micro, but it's a lot of work, the project is not very active, and overall it's just a lot less work to use CodeMirror and all the work that's already put into that. |
#2117 (comment) So, yeah, I'm conservative regarding UI integration in general. I'm not sure why, but might be because: Anyway, it's not what I will decide. |
Right, thanks for the reminder.
Yeah, those are problems. I tried to keep this one as simple as possible, but there's only so much I could do.
I've been working in a few environments (remote Linux, Chrome OS Linux, Mac OS), and Meld had different challenges on each one (have to use remote desktop on remote, problems with Wayland on Chrome OS, having to find and use https://gist.github.com/syneart/4a8724cd479d31f0f742f499f807dcb2 on Mac). |
Your opinion certainly has a lot of weight in my mind, even (especially?) when I don't immediately agree with it. I'm not quite sure what to do right now, I'm hoping the Discord discussion might produce something. I realized I started it at the wrong time for you, sorry. |
Nice, I'm a bit surprised that the changes in Cargo.lock in your PR wasn't huge.
That doesn't apply to diffedit3, right? I expect it would be super easy to install as it can be integrated in jj with a small amount of changes.
As for packaging, I'm old man. I don't buy much about single binary, musl, etc. So feel free to disregard my comments around that. |
Yes, that's the idea. AFAIK, in all of these environments it's possible to start a server on a local port and start a browser to get a webpage from that port, like Jupyter does. In many of them, no setup would be needed; in some you'd need to SSH with port forwarding. Another example of an environment where Meld is hard (mentioned by @thoughtpolice) is WSL. |
The main goal is to merge the parts of #3292 that are likely to cause merge conflicts with other PRs, so that I can polish that PR up without worrying too much.
The intention is to have
external.rs,builtin-web.rs, andbuiltin.rsas separate files. The latter could be renamed tobuiltin-tui.rslater. Another option would be to keep external diff editors anddiffedit3-based diff editor in the same file, as they are very similar from jj's perspective for now.`