From 897bc537349983d8def6c9ea305f2f5a33059a0e 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 7d78ff840e..950adf61a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,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`. + * The list of conflicted paths is printed whenever the working copy changes. This can be disabled with the `--quiet` option. diff --git a/Cargo.lock b/Cargo.lock index af1c44647c..1c94767c1f 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.37" @@ -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" @@ -1469,6 +1501,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" @@ -1489,6 +1540,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" @@ -1522,6 +1597,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" @@ -1579,6 +1736,7 @@ checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26" dependencies = [ "equivalent", "hashbrown", + "serde", ] [[package]] @@ -1618,6 +1776,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" @@ -1629,6 +1796,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" @@ -1680,6 +1857,7 @@ dependencies = [ "config", "criterion", "crossterm", + "diffedit3", "dirs", "esl01-renderdag", "futures 0.3.30", @@ -1711,7 +1889,7 @@ dependencies = [ "textwrap", "thiserror", "timeago", - "toml_edit", + "toml_edit 0.19.15", "tracing", "tracing-chrome", "tracing-subscriber", @@ -1931,6 +2109,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" @@ -1979,6 +2173,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" @@ -2057,6 +2263,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" @@ -2239,6 +2456,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" @@ -2304,6 +2569,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.79" @@ -2532,6 +2806,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" @@ -2559,6 +2842,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" @@ -2684,6 +3001,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" @@ -2822,6 +3162,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" @@ -3048,6 +3397,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" @@ -3057,6 +3420,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" @@ -3079,6 +3455,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" @@ -3172,6 +3572,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" @@ -3351,7 +3769,7 @@ dependencies = [ "serde_bser", "thiserror", "tokio", - "tokio-util", + "tokio-util 0.6.10", "winapi", ] @@ -3376,6 +3794,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 51040d8212..1d1f1792f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ chrono = { version = "0.4.37", 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.10.0" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 1444902a56..7625eb7e6c 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -34,6 +34,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 2c98abce16..cb5fcfc5b9 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 95140cd133..b7d0caffbe 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")] @@ -100,7 +104,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), } @@ -122,7 +127,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, @@ -138,8 +143,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)) } @@ -225,21 +232,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(), + ), } } } @@ -311,10 +323,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, ), @@ -343,7 +356,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###" @@ -402,7 +415,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###" @@ -549,7 +562,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###" @@ -598,7 +611,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 94b01d728d..8f56b71970 100644 --- a/cli/tests/test_resolve_command.rs +++ b/cli/tests/test_resolve_command.rs @@ -447,7 +447,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. @@ -525,7 +525,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": @@ -582,7 +582,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 7429a833e3..f2246dce3a 100644 --- a/docs/config.md +++ b/docs/config.md @@ -439,13 +439,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.