From a385651b13e89763e95a42158a1720a28c9bda97 Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Wed, 13 Mar 2024 19:58:34 -0700 Subject: [PATCH] diff editor: bundle a new `:builtin-web` GUI diff editor When called with `--tool :builtin-web`, `jj` will start a local web server and open a web browser with a GUI to edit the diff. See https://github.com/ilyagr/diffedit3 for more details (or to run it as a standalone executable). This is the diff editor previously advertised in https://github.com/martinvonz/jj/discussions/3094, based on CodeMirror5, with some improvements since. I would like to bundle it with `jj` so that everybody has access to a GUI diff editor that can be run locally or over SSH (with port forwarding). It is intended for situations where it is a fuss (or impossible) for a user to install Meld and use the `meld-3` configuration. Some TODOs and thoughts: - This diff editor is somewhat restricted; it will ignore symlinks and currently has no support for executable bits for example. There are some known bugs, see e.g. the end of [the `diffedit3 README](https://github.com/ilyagr/diffedit3/?tab=readme-ov-file#shorter-term-todos-and-known-bugs). I'm hoping it's already usable. - Bundling the diff editor seems to add ~1.5MB to the `jj` binary. This is more than I expected. Unless there's a way to optimize this, I'm thinking of making the diff editor an optional feature, but I'd like it enabled by default, at least in release binaries. Or we could not worry about it. - I'm considering folding the `diffedit3` repo (or the majority of it) into the jj source repo, so that it benefits from Dependabot, established code review procedures, and the reviewers we have for `jj`. The downside is that `jj` will then contain some Typescript code. However, there will be no need to install `node` or `npm` *unless* you are specifically working on the webapp; the "compiled" webapp is included in the repo. - Currently, `:builtin-web` works just like an external diff editor, creating a temporary directory on disk and then modifying it. At some point, we might want to switch to keeping the tree in-memory. --- CHANGELOG.md | 5 + Cargo.lock | 430 ++++++++++++++++++++++++++++- Cargo.toml | 1 + cli/Cargo.toml | 1 + cli/src/merge_tools/builtin.rs | 2 + cli/src/merge_tools/builtin_web.rs | 61 ++++ cli/src/merge_tools/mod.rs | 63 +++-- cli/tests/test_resolve_command.rs | 6 +- docs/config.md | 24 +- 9 files changed, 558 insertions(+), 35 deletions(-) create mode 100644 cli/src/merge_tools/builtin_web.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c653ef5d57..cde9c5d500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### New features +* New web-based `:builtin-web` diff editor. Try it with `jj split --tool + :builtin-web`. + +* The `:builtin` diff editor now has an alias `:builtin-tui`. + * `jj tag list` now prints commit summary along with the tag name. * Conflict markers now include an explanation of what each part of the conflict diff --git a/Cargo.lock b/Cargo.lock index a9249eae9d..befe1810eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -195,6 +195,12 @@ dependencies = [ "rustc-demangle", ] +[[package]] +name = "base64" +version = "0.21.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" + [[package]] name = "bitflags" version = "1.3.2" @@ -327,6 +333,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "cfg_aliases" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" + [[package]] name = "chrono" version = "0.4.38" @@ -468,7 +480,7 @@ dependencies = [ "nom", "pathdiff", "serde", - "toml", + "toml 0.5.11", ] [[package]] @@ -655,6 +667,26 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" +[[package]] +name = "diffedit3" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f26192bc4a31bef4c22e15e2b66d9278bd380f8e0a3f00dc48d18680d4f3c16" +dependencies = [ + "clap", + "indexmap", + "open", + "parking_lot", + "poem", + "rust-embed", + "serde", + "thiserror", + "tokio", + "toml 0.8.12", + "tracing-subscriber", + "walkdir", +] + [[package]] name = "difflib" version = "0.4.0" @@ -1470,6 +1502,25 @@ dependencies = [ "regex-syntax 0.8.3", ] +[[package]] +name = "h2" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "816ec7294445779408f36fe57bc5b7fc1cf59664059096c65f905c1c61f58069" +dependencies = [ + "bytes 1.6.0", + "fnv", + "futures-core", + "futures-sink", + "futures-util", + "http", + "indexmap", + "slab", + "tokio", + "tokio-util 0.7.10", + "tracing", +] + [[package]] name = "half" version = "2.4.0" @@ -1490,6 +1541,30 @@ dependencies = [ "allocator-api2", ] +[[package]] +name = "headers" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "322106e6bd0cba2d5ead589ddb8150a13d7c4217cf80d7c4f682ca994ccc6aa9" +dependencies = [ + "base64", + "bytes 1.6.0", + "headers-core", + "http", + "httpdate", + "mime", + "sha1", +] + +[[package]] +name = "headers-core" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "54b4a22553d4242c49fddb9ba998a99962b5cc6f22cb5a3482bec22522403ce4" +dependencies = [ + "http", +] + [[package]] name = "heck" version = "0.4.1" @@ -1523,6 +1598,88 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "http" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21b9ddb458710bc376481b842f5da65cdf31522de232c1ca8146abce2a358258" +dependencies = [ + "bytes 1.6.0", + "fnv", + "itoa", +] + +[[package]] +name = "http-body" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1cac85db508abc24a2e48553ba12a996e87244a0395ce011e62b37158745d643" +dependencies = [ + "bytes 1.6.0", + "http", +] + +[[package]] +name = "http-body-util" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0475f8b2ac86659c21b64320d5d653f9efe42acd2a4e560073ec61a155a34f1d" +dependencies = [ + "bytes 1.6.0", + "futures-core", + "http", + "http-body", + "pin-project-lite", +] + +[[package]] +name = "httparse" +version = "1.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d897f394bad6a705d5f4104762e116a75639e470d80901eed05a860a95cb1904" + +[[package]] +name = "httpdate" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" + +[[package]] +name = "hyper" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "186548d73ac615b32a73aafe38fb4f56c0d340e110e5a200bcadbaf2e199263a" +dependencies = [ + "bytes 1.6.0", + "futures-channel", + "futures-util", + "h2", + "http", + "http-body", + "httparse", + "httpdate", + "itoa", + "pin-project-lite", + "smallvec", + "tokio", +] + +[[package]] +name = "hyper-util" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca38ef113da30126bbff9cd1705f9273e15d45498615d138b0c20279ac7a76aa" +dependencies = [ + "bytes 1.6.0", + "futures-util", + "http", + "http-body", + "hyper", + "pin-project-lite", + "socket2", + "tokio", +] + [[package]] name = "iana-time-zone" version = "0.1.60" @@ -1580,6 +1737,7 @@ checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26" dependencies = [ "equivalent", "hashbrown", + "serde", ] [[package]] @@ -1619,6 +1777,15 @@ dependencies = [ "libc", ] +[[package]] +name = "is-docker" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "928bae27f42bc99b60d9ac7334e3a21d10ad8f1835a4e12ec3ec0464765ed1b3" +dependencies = [ + "once_cell", +] + [[package]] name = "is-terminal" version = "0.4.12" @@ -1630,6 +1797,16 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "is-wsl" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "173609498df190136aa7dea1a91db051746d339e18476eed5ca40521f02d7aa5" +dependencies = [ + "is-docker", + "once_cell", +] + [[package]] name = "itertools" version = "0.10.5" @@ -1681,6 +1858,7 @@ dependencies = [ "config", "criterion", "crossterm", + "diffedit3", "dirs", "esl01-renderdag", "futures 0.3.30", @@ -1712,7 +1890,7 @@ dependencies = [ "textwrap", "thiserror", "timeago", - "toml_edit", + "toml_edit 0.19.15", "tracing", "tracing-chrome", "tracing-subscriber", @@ -1932,6 +2110,22 @@ dependencies = [ "libc", ] +[[package]] +name = "mime" +version = "0.3.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" + +[[package]] +name = "mime_guess" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4192263c238a5f0d0c6bfd21f336a313a4ce1c450542449ca191bb657b4642ef" +dependencies = [ + "mime", + "unicase", +] + [[package]] name = "minimal-lexical" version = "0.2.1" @@ -1980,6 +2174,18 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "defc4c55412d89136f966bbb339008b474350e5e6e78d2714439c386b3137a03" +[[package]] +name = "nix" +version = "0.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab2156c4fce2f8df6c499cc1c763e4394b7482525bf2a9701c9d79d215f519e4" +dependencies = [ + "bitflags 2.5.0", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "nom" version = "7.1.3" @@ -2058,6 +2264,17 @@ version = "11.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ab1bc2a289d34bd04a330323ac98a1b4bc82c9d9fcb1e66b63caa84da26b575" +[[package]] +name = "open" +version = "5.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "449f0ff855d85ddbf1edd5b646d65249ead3f5e422aaa86b7d2d0b049b103e32" +dependencies = [ + "is-wsl", + "libc", + "pathdiff", +] + [[package]] name = "openssl-probe" version = "0.1.5" @@ -2240,6 +2457,54 @@ dependencies = [ "plotters-backend", ] +[[package]] +name = "poem" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b735eaaaa6bc7ed2dcbcab1d5373afe1f6d03a37d8695ba3c42101f733a8455" +dependencies = [ + "bytes 1.6.0", + "futures-util", + "headers", + "hex", + "http", + "http-body-util", + "hyper", + "hyper-util", + "mime", + "mime_guess", + "nix", + "parking_lot", + "percent-encoding", + "pin-project-lite", + "poem-derive", + "regex", + "rfc7239", + "rust-embed", + "serde", + "serde_json", + "serde_urlencoded", + "smallvec", + "sync_wrapper", + "thiserror", + "tokio", + "tokio-util 0.7.10", + "tracing", + "wildmatch", +] + +[[package]] +name = "poem-derive" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2b961d58a6c53380c20236394381d9292fda03577f902b158f1638932964dcf" +dependencies = [ + "proc-macro-crate", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "pollster" version = "0.3.0" @@ -2305,6 +2570,15 @@ dependencies = [ "syn", ] +[[package]] +name = "proc-macro-crate" +version = "3.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d37c51ca738a55da99dc0c4a34860fd675453b8b36209178c2249bb13651284" +dependencies = [ + "toml_edit 0.21.1", +] + [[package]] name = "proc-macro2" version = "1.0.81" @@ -2533,6 +2807,15 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "adad44e29e4c806119491a7f06f03de4d1af22c3a680dd47f1e6e179439d1f56" +[[package]] +name = "rfc7239" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b106a85eeb5b0336d16d6a20eab857f92861d4fbb1eb9a239866fb98fb6a1063" +dependencies = [ + "uncased", +] + [[package]] name = "roff" version = "0.2.1" @@ -2560,6 +2843,40 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "rust-embed" +version = "8.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb78f46d0066053d16d4ca7b898e9343bc3530f71c61d5ad84cd404ada068745" +dependencies = [ + "rust-embed-impl", + "rust-embed-utils", + "walkdir", +] + +[[package]] +name = "rust-embed-impl" +version = "8.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b91ac2a3c6c0520a3fb3dd89321177c3c692937c4eb21893378219da10c44fc8" +dependencies = [ + "proc-macro2", + "quote", + "rust-embed-utils", + "syn", + "walkdir", +] + +[[package]] +name = "rust-embed-utils" +version = "8.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86f69089032567ffff4eada41c573fc43ff466c7db7c5688b2e7969584345581" +dependencies = [ + "sha2", + "walkdir", +] + [[package]] name = "rustc-demangle" version = "0.1.23" @@ -2685,6 +3002,29 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_urlencoded" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3491c14715ca2294c4d6a88f15e84739788c1d030eed8c110436aafdaa2f3fd" +dependencies = [ + "form_urlencoded", + "itoa", + "ryu", + "serde", +] + +[[package]] +name = "sha1" +version = "0.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3bf829a2d51ab4a5ddf1352d8470c140cadc8301b2ae1789db023f01cedd6ba" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "sha1_smol" version = "1.0.0" @@ -2823,6 +3163,15 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "sync_wrapper" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "384595c11a4e2969895cad5a8c4029115f5ab956a9e5ef4de79d11a426e5f20c" +dependencies = [ + "futures-core", +] + [[package]] name = "tempfile" version = "3.10.1" @@ -3049,6 +3398,20 @@ dependencies = [ "tokio", ] +[[package]] +name = "tokio-util" +version = "0.7.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5419f34732d9eb6ee4c3578b7989078579b7f039cbbb9ca2c4da015749371e15" +dependencies = [ + "bytes 1.6.0", + "futures-core", + "futures-sink", + "pin-project-lite", + "tokio", + "tracing", +] + [[package]] name = "toml" version = "0.5.11" @@ -3058,6 +3421,19 @@ dependencies = [ "serde", ] +[[package]] +name = "toml" +version = "0.8.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9dd1545e8208b4a5af1aa9bbd0b4cf7e9ea08fabc5d0a5c67fcaafa17433aa3" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit 0.22.9", +] + [[package]] name = "toml_datetime" version = "0.6.5" @@ -3080,6 +3456,30 @@ dependencies = [ "winnow 0.5.40", ] +[[package]] +name = "toml_edit" +version = "0.21.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a8534fd7f78b5405e860340ad6575217ce99f38d4d5c8f2442cb5ecb50090e1" +dependencies = [ + "indexmap", + "toml_datetime", + "winnow 0.5.40", +] + +[[package]] +name = "toml_edit" +version = "0.22.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e40bb779c5187258fd7aad0eb68cb8706a0a81fa712fbea808ab43c4b8374c4" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "winnow 0.6.5", +] + [[package]] name = "tracing" version = "0.1.40" @@ -3173,6 +3573,24 @@ dependencies = [ "arrayvec", ] +[[package]] +name = "uncased" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1b88fcfe09e89d3866a5c11019378088af2d24c3fbd4f0543f96b479ec90697" +dependencies = [ + "version_check", +] + +[[package]] +name = "unicase" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7d2d4dafb69621809a81864c9c1b864479e1235c0dd4e199924b9742439ed89" +dependencies = [ + "version_check", +] + [[package]] name = "unicode-bidi" version = "0.3.15" @@ -3352,7 +3770,7 @@ dependencies = [ "serde_bser", "thiserror", "tokio", - "tokio-util", + "tokio-util 0.6.10", "winapi", ] @@ -3377,6 +3795,12 @@ dependencies = [ "web-sys", ] +[[package]] +name = "wildmatch" +version = "2.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "939e59c1bc731542357fdaad98b209ef78c8743d652bb61439d16b16a79eb025" + [[package]] name = "winapi" version = "0.3.9" diff --git a/Cargo.toml b/Cargo.toml index 6819fa72c7..841b5ac440 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ chrono = { version = "0.4.38", default-features = false, features = [ config = { version = "0.13.4", default-features = false, features = ["toml"] } criterion = "0.5.1" crossterm = { version = "0.27", default-features = false } +diffedit3 = "0.2.0" digest = "0.10.7" dirs = "5.0.1" either = "1.11.0" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 8d2b41c514..61febba541 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -46,6 +46,7 @@ name = "runner" cargo_metadata = { workspace = true } [dependencies] +diffedit3 = { workspace = true } chrono = { workspace = true } clap = { workspace = true } clap-markdown = { workspace = true } diff --git a/cli/src/merge_tools/builtin.rs b/cli/src/merge_tools/builtin.rs index 673fc7e0fe..9719147ea1 100644 --- a/cli/src/merge_tools/builtin.rs +++ b/cli/src/merge_tools/builtin.rs @@ -17,6 +17,8 @@ use jj_lib::store::Store; use pollster::FutureExt; use thiserror::Error; +// TODO(ilyagr): Rename this file and types to use BuiltinTUI instead of +// Builtin, for symmetry with BuiltinWeb #[derive(Debug, Error)] pub enum BuiltinToolError { #[error("Failed to record changes")] diff --git a/cli/src/merge_tools/builtin_web.rs b/cli/src/merge_tools/builtin_web.rs new file mode 100644 index 0000000000..67b5e4d339 --- /dev/null +++ b/cli/src/merge_tools/builtin_web.rs @@ -0,0 +1,61 @@ +use std::sync::Arc; + +use jj_lib::backend::MergedTreeId; +use jj_lib::gitignore::GitIgnoreFile; +use jj_lib::matchers::Matcher; +use jj_lib::merged_tree::MergedTree; + +use super::diff_working_copies::{DiffEditWorkingCopies, DiffSide}; +use super::DiffEditError; + +pub fn edit_diff_web( + left_tree: &MergedTree, + right_tree: &MergedTree, + matcher: &dyn Matcher, + instructions: Option<&str>, + base_ignores: Arc, +) -> Result { + let store = left_tree.store(); + let diffedit_wc = DiffEditWorkingCopies::check_out( + store, + left_tree, + right_tree, + matcher, + Some(DiffSide::Right), + instructions, + )?; + + // TODO(ilyagr): We may want to keep the files in-memory for the internal diff + // editor instead of treating the internal editor like an external tool. The + // main (minor) difficulty is to extract the functions to render and load + // conflicted files. + let diffedit_input = diffedit3::ThreeDirInput { + left: diffedit_wc + .working_copies + .left_working_copy_path() + .to_path_buf(), + right: diffedit_wc + .working_copies + .right_working_copy_path() + .to_path_buf(), + edit: diffedit_wc + .working_copies + .output_working_copy_path() + .unwrap() + .to_path_buf(), + }; + tracing::info!(?diffedit_input, "Starting the diffedit3 local server"); + // 17376 is a verified random number, as in https://xkcd.com/221/ :). I am + // trying to avoid 8000 or 8080 in case those, more commonly used, port + // numbers are used for something else. + // + // TODO: allow changing the ports and whether to open the browser. + match diffedit3::local_server::run_server_sync(Box::new(diffedit_input), 17376, 17380, true) { + Ok(()) => {} + Err(e) => { + return Err(DiffEditError::InternalWebTool(Box::new(e))); + } + }; + + diffedit_wc.snapshot_results(base_ignores) +} diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index 8a45fa7a33..319c2d65e2 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -1,4 +1,4 @@ -// Copyright 2020 The Jujutsu Authors +// Copyright 2024 The Jujutsu Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -13,6 +13,7 @@ // limitations under the License. mod builtin; +mod builtin_web; mod diff_working_copies; mod external; @@ -31,18 +32,19 @@ use pollster::FutureExt; use thiserror::Error; use self::builtin::{edit_diff_builtin, edit_merge_builtin, BuiltinToolError}; +use self::builtin_web::edit_diff_web; use self::diff_working_copies::DiffCheckoutError; use self::external::{edit_diff_external, ExternalToolError}; pub use self::external::{generate_diff, ExternalMergeTool}; use crate::config::CommandNameAndArgs; use crate::ui::Ui; -const BUILTIN_EDITOR_NAME: &str = ":builtin"; - #[derive(Debug, Error)] pub enum DiffEditError { #[error(transparent)] - InternalTool(#[from] Box), + InternalTUITool(#[from] Box), + #[error(transparent)] + InternalWebTool(#[from] Box), #[error(transparent)] ExternalTool(#[from] ExternalToolError), #[error(transparent)] @@ -65,6 +67,8 @@ pub enum DiffGenerateError { pub enum ConflictResolveError { #[error(transparent)] InternalTool(#[from] Box), + #[error("The ':builtin-web' tool cannot yet be used to resolve merge conflicts.")] + BuiltinWebNotImplemented, #[error(transparent)] ExternalTool(#[from] ExternalToolError), #[error("Couldn't find the path {0:?} in this revision")] @@ -97,7 +101,8 @@ pub enum MergeToolConfigError { #[derive(Clone, Debug, Eq, PartialEq)] pub enum MergeTool { - Builtin, + BuiltinTUI, + BuiltinWeb, // Boxed because ExternalMergeTool is big compared to the Builtin variant. External(Box), } @@ -119,7 +124,7 @@ fn editor_args_from_settings( if let Some(args) = settings.config().get(key).optional()? { Ok(args) } else { - let default_editor = BUILTIN_EDITOR_NAME; + let default_editor = ":builtin-tui"; if let Some(mut writer) = ui.hint_default() { writeln!( writer, @@ -135,8 +140,10 @@ fn editor_args_from_settings( /// Resolves builtin merge tool name or loads external tool options from /// `[merge-tools.]`. fn get_tool_config(settings: &UserSettings, name: &str) -> Result, ConfigError> { - if name == BUILTIN_EDITOR_NAME { - Ok(Some(MergeTool::Builtin)) + if [":builtin", ":builtin-tui"].contains(&name) { + Ok(Some(MergeTool::BuiltinTUI)) + } else if name == ":builtin-web" { + Ok(Some(MergeTool::BuiltinWeb)) } else { Ok(get_external_tool_config(settings, name)?.map(MergeTool::external)) } @@ -222,21 +229,26 @@ impl DiffEditor { matcher: &dyn Matcher, instructions: Option<&str>, ) -> Result { + let instructions = self.use_instructions.then_some(instructions).flatten(); match &self.tool { - MergeTool::Builtin => { + MergeTool::BuiltinTUI => { Ok(edit_diff_builtin(left_tree, right_tree, matcher).map_err(Box::new)?) } - MergeTool::External(editor) => { - let instructions = self.use_instructions.then_some(instructions).flatten(); - edit_diff_external( - editor, - left_tree, - right_tree, - matcher, - instructions, - self.base_ignores.clone(), - ) - } + MergeTool::BuiltinWeb => edit_diff_web( + left_tree, + right_tree, + matcher, + instructions, + self.base_ignores.clone(), + ), + MergeTool::External(editor) => edit_diff_external( + editor, + left_tree, + right_tree, + matcher, + instructions, + self.base_ignores.clone(), + ), } } } @@ -308,10 +320,11 @@ impl MergeEditor { let content = extract_as_single_hunk(&file_merge, tree.store(), repo_path).block_on(); match &self.tool { - MergeTool::Builtin => { + MergeTool::BuiltinTUI => { let tree_id = edit_merge_builtin(tree, repo_path, content).map_err(Box::new)?; Ok(tree_id) } + MergeTool::BuiltinWeb => Err(ConflictResolveError::BuiltinWebNotImplemented), MergeTool::External(editor) => external::run_mergetool_external( editor, file_merge, content, repo_path, conflict, tree, ), @@ -340,7 +353,7 @@ mod tests { DiffEditor::with_name(name, &settings, GitIgnoreFile::empty()).map(|editor| editor.tool) }; - insta::assert_debug_snapshot!(get(":builtin", "").unwrap(), @"Builtin"); + insta::assert_debug_snapshot!(get(":builtin", "").unwrap(), @"BuiltinTUI"); // Just program name, edit_args are filled by default insta::assert_debug_snapshot!(get("my diff", "").unwrap(), @r###" @@ -399,7 +412,7 @@ mod tests { }; // Default - insta::assert_debug_snapshot!(get("").unwrap(), @"Builtin"); + insta::assert_debug_snapshot!(get("").unwrap(), @"BuiltinTUI"); // Just program name, edit_args are filled by default insta::assert_debug_snapshot!(get(r#"ui.diff-editor = "my-diff""#).unwrap(), @r###" @@ -546,7 +559,7 @@ mod tests { MergeEditor::with_name(name, &settings).map(|editor| editor.tool) }; - insta::assert_debug_snapshot!(get(":builtin", "").unwrap(), @"Builtin"); + insta::assert_debug_snapshot!(get(":builtin", "").unwrap(), @"BuiltinTUI"); // Just program name insta::assert_debug_snapshot!(get("my diff", "").unwrap_err(), @r###" @@ -595,7 +608,7 @@ mod tests { }; // Default - insta::assert_debug_snapshot!(get("").unwrap(), @"Builtin"); + insta::assert_debug_snapshot!(get("").unwrap(), @"BuiltinTUI"); // Just program name insta::assert_debug_snapshot!(get(r#"ui.merge-editor = "my-merge""#).unwrap_err(), @r###" diff --git a/cli/tests/test_resolve_command.rs b/cli/tests/test_resolve_command.rs index d5f79f693c..7960308ad0 100644 --- a/cli/tests/test_resolve_command.rs +++ b/cli/tests/test_resolve_command.rs @@ -478,7 +478,7 @@ fn test_too_many_parents() { let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]); insta::assert_snapshot!(error, @r###" - Hint: Using default editor ':builtin'; run `jj config set --user ui.merge-editor :builtin` to disable this message. + Hint: Using default editor ':builtin-tui'; run `jj config set --user ui.merge-editor :builtin` to disable this message. Resolving conflicts in: file Error: Failed to resolve conflicts Caused by: The conflict at "file" has 3 sides. At most 2 sides are supported. @@ -556,7 +556,7 @@ fn test_file_vs_dir() { "###); let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]); insta::assert_snapshot!(error, @r###" - Hint: Using default editor ':builtin'; run `jj config set --user ui.merge-editor :builtin` to disable this message. + Hint: Using default editor ':builtin-tui'; run `jj config set --user ui.merge-editor :builtin` to disable this message. Resolving conflicts in: file Error: Failed to resolve conflicts Caused by: Only conflicts that involve normal files (not symlinks, not executable, etc.) are supported. Conflict summary for "file": @@ -613,7 +613,7 @@ fn test_description_with_dir_and_deletion() { "###); let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]); insta::assert_snapshot!(error, @r###" - Hint: Using default editor ':builtin'; run `jj config set --user ui.merge-editor :builtin` to disable this message. + Hint: Using default editor ':builtin-tui'; run `jj config set --user ui.merge-editor :builtin` to disable this message. Resolving conflicts in: file Error: Failed to resolve conflicts Caused by: Only conflicts that involve normal files (not symlinks, not executable, etc.) are supported. Conflict summary for "file": diff --git a/docs/config.md b/docs/config.md index ec27c07e3b..f85800ec2a 100644 --- a/docs/config.md +++ b/docs/config.md @@ -448,13 +448,29 @@ Obviously, you would only set one line, don't copy them all in! ## Editing diffs The `ui.diff-editor` setting affects the tool used for editing diffs (e.g. `jj -split`, `jj squash -i`). The default is the special value `:builtin`, which -launches a built-in TUI tool (known as [scm-diff-editor]) to edit the diff in -your terminal. +split`, `jj squash -i`). + +### Built-in diff editors + +The default is the special value `:builtin`, which is equivlaent to +`:builtin-tui`. Either of these launches a built-in TUI tool +(known as [scm-diff-editor]) to edit the diff in your terminal. + +There is also a built-in `:builtin-web` GUI editor (also knows as [diffedit3]) +that runs as a local server that is accessed with a web browser. The interface +is similar to, and less poished than, that of the [`meld-3` Meld +config](#experimental-3-pane-diff-editing), but requires neither installing Meld +nor running Meld. `:builtin-web` can also be run over SSH with port forwarding. + + [scm-diff-editor]: https://github.com/arxanas/scm-record?tab=readme-ov-file#scm-diff-editor +[diffedit3]: https://github.com/ilyagr/diffedit3 -`jj` makes the following substitutions: +### External diff editors + +`ui.diff-editor` can also be set to a command, or to a list of a command +followed by its arguments. `jj` makes the following substitutions: - `$left` and `$right` are replaced with the paths to the left and right directories to diff respectively.