From 834a74fd19b65aa73a2ddfcb8528df17d5514a00 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. --- Cargo.lock | 449 +++++++++++++++++++++++++++++- cli/Cargo.toml | 1 + cli/src/merge_tools/builtin.rs | 2 + cli/src/merge_tools/external.rs | 41 +++ cli/src/merge_tools/mod.rs | 63 +++-- cli/tests/test_resolve_command.rs | 6 +- 6 files changed, 525 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4389bbe6c9..8d78867b2a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -177,6 +177,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" @@ -459,7 +465,7 @@ dependencies = [ "nom", "pathdiff", "serde", - "toml", + "toml 0.5.11", ] [[package]] @@ -646,6 +652,26 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" +[[package]] +name = "diffedit3" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b65eff79656508cf0eedb3bc07e2fa18c50f37262ae1180c5d480807f21b77dd" +dependencies = [ + "clap", + "indexmap", + "open", + "parking_lot", + "poem", + "rust-embed", + "serde", + "thiserror", + "tokio", + "toml 0.8.10", + "tracing-subscriber", + "walkdir", +] + [[package]] name = "difflib" version = "0.4.0" @@ -777,6 +803,12 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "form_urlencoded" version = "1.2.1" @@ -983,7 +1015,7 @@ dependencies = [ "gix-date", "itoa", "thiserror", - "winnow", + "winnow 0.5.40", ] [[package]] @@ -1036,7 +1068,7 @@ dependencies = [ "smallvec", "thiserror", "unicode-bom", - "winnow", + "winnow 0.5.40", ] [[package]] @@ -1220,7 +1252,7 @@ dependencies = [ "itoa", "smallvec", "thiserror", - "winnow", + "winnow 0.5.40", ] [[package]] @@ -1307,7 +1339,7 @@ dependencies = [ "gix-validate", "memmap2", "thiserror", - "winnow", + "winnow 0.5.40", ] [[package]] @@ -1455,6 +1487,25 @@ dependencies = [ "regex-syntax 0.8.2", ] +[[package]] +name = "h2" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "31d030e59af851932b72ceebadf4a2b5986dba4c3b99dd2493f8273a0f151943" +dependencies = [ + "bytes 1.5.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" @@ -1471,6 +1522,30 @@ version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "290f1a1d9242c78d09ce40a5e87e7554ee637af1351968159f4952f028f75604" +[[package]] +name = "headers" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "322106e6bd0cba2d5ead589ddb8150a13d7c4217cf80d7c4f682ca994ccc6aa9" +dependencies = [ + "base64", + "bytes 1.5.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" @@ -1498,6 +1573,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.5.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.5.0", + "http", +] + +[[package]] +name = "http-body-util" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41cb79eb393015dadd30fc252023adb0b2400a0caee0fa2a077e6e21a551e840" +dependencies = [ + "bytes 1.5.0", + "futures-util", + "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.5.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.5.0", + "futures-util", + "http", + "http-body", + "hyper", + "pin-project-lite", + "socket2", + "tokio", +] + [[package]] name = "iana-time-zone" version = "0.1.60" @@ -1555,6 +1712,7 @@ checksum = "7b0b929d511467233429c45a44ac1dcaa21ba0f5ba11e4879e6ed28ddb4f9df4" dependencies = [ "equivalent", "hashbrown", + "serde", ] [[package]] @@ -1595,6 +1753,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" @@ -1606,6 +1773,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" @@ -1657,6 +1834,7 @@ dependencies = [ "config", "criterion", "crossterm", + "diffedit3", "dirs", "esl01-renderdag", "futures 0.3.30", @@ -1687,7 +1865,7 @@ dependencies = [ "textwrap", "thiserror", "timeago", - "toml_edit", + "toml_edit 0.19.15", "tracing", "tracing-chrome", "tracing-subscriber", @@ -1907,6 +2085,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" @@ -1955,6 +2149,17 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5ce46fe64a9d73be07dcbe690a38ce1b293be448fd8ce1e6c1b8062c9f72c6a" +[[package]] +name = "nix" +version = "0.27.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2eb04e9c688eff1c89d72b407f168cf79bb9e867a9d3323ed6c01519eb9cc053" +dependencies = [ + "bitflags 2.4.2", + "cfg-if", + "libc", +] + [[package]] name = "nom" version = "7.1.3" @@ -2033,6 +2238,17 @@ version = "11.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ab1bc2a289d34bd04a330323ac98a1b4bc82c9d9fcb1e66b63caa84da26b575" +[[package]] +name = "open" +version = "5.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68b3fbb0d52bf0cbb5225ba3d2c303aa136031d43abff98284332a9981ecddec" +dependencies = [ + "is-wsl", + "libc", + "pathdiff", +] + [[package]] name = "openssl-probe" version = "0.1.5" @@ -2215,6 +2431,55 @@ dependencies = [ "plotters-backend", ] +[[package]] +name = "poem" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec6f0033707ce6eb76240e39a592e7eac2f702768636cb03969293227ca8e642" +dependencies = [ + "async-trait", + "bytes 1.5.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 = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4826d63b6760f8e5d24be9f738a5e38a43d726f32a3c2cc9388b1cc66e587f5c" +dependencies = [ + "proc-macro-crate", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "pollster" version = "0.3.0" @@ -2280,6 +2545,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" @@ -2509,6 +2783,15 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c08c74e62047bb2de4ff487b251e4a92e24f48745648451635cec7d591162d9f" +[[package]] +name = "rfc7239" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "087317b3cf7eb481f13bd9025d729324b7cd068d6f470e2d76d049e191f5ba47" +dependencies = [ + "uncased", +] + [[package]] name = "roff" version = "0.2.1" @@ -2536,6 +2819,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" @@ -2661,6 +2978,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" @@ -2799,6 +3139,15 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "sync_wrapper" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2047c6ded9c721764247e62cd3b03c09ffc529b2ba5b10ec482ae507a4a70160" +dependencies = [ + "futures-core", +] + [[package]] name = "tempfile" version = "3.10.1" @@ -3025,6 +3374,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.5.0", + "futures-core", + "futures-sink", + "pin-project-lite", + "tokio", + "tracing", +] + [[package]] name = "toml" version = "0.5.11" @@ -3034,6 +3397,19 @@ dependencies = [ "serde", ] +[[package]] +name = "toml" +version = "0.8.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a9aad4a3066010876e8dcf5a8a06e70a558751117a145c6ce2b82c2e2054290" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit 0.22.6", +] + [[package]] name = "toml_datetime" version = "0.6.5" @@ -3053,7 +3429,31 @@ dependencies = [ "serde", "serde_spanned", "toml_datetime", - "winnow", + "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.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c1b5fd4128cc8d3e0cb74d4ed9a9cc7c7284becd4df68f5f940e1ad123606f6" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "winnow 0.6.5", ] [[package]] @@ -3149,6 +3549,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" @@ -3328,7 +3746,7 @@ dependencies = [ "serde_bser", "thiserror", "tokio", - "tokio-util", + "tokio-util 0.6.10", "winapi", ] @@ -3365,6 +3783,12 @@ dependencies = [ "web-sys", ] +[[package]] +name = "wildmatch" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "495ec47bf3c1345005f40724f0269362c8556cbc43aed0526ed44cae1d35fceb" + [[package]] name = "winapi" version = "0.3.9" @@ -3546,6 +3970,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "winnow" +version = "0.6.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dffa400e67ed5a4dd237983829e66475f0a4a26938c4b04c21baede6262215b8" +dependencies = [ + "memchr", +] + [[package]] name = "winreg" version = "0.52.0" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 43b4ec237d..da994057e2 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -34,6 +34,7 @@ name = "runner" cargo_metadata = { workspace = true } [dependencies] +diffedit3 = "0.1.0" 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/external.rs b/cli/src/merge_tools/external.rs index d7527633bc..672c24629a 100644 --- a/cli/src/merge_tools/external.rs +++ b/cli/src/merge_tools/external.rs @@ -557,6 +557,47 @@ pub fn edit_diff_external( snapshot_diffedit_results(diff_wc, base_ignores, instructions_path_to_cleanup) } +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 (diff_wc, instructions_path_to_cleanup) = check_out_trees_for_diffedit( + 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: diff_wc.left_working_copy_path().to_path_buf(), + right: diff_wc.right_working_copy_path().to_path_buf(), + edit: diff_wc.output_working_copy_path().unwrap().to_path_buf(), + }; + tracing::info!(?diffedit_input, "Starting the diffedit3 local server"); + // TODO: allow changing the ports and whether to open the browser. 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. + match diffedit3::local_server::run_server_sync(Box::new(diffedit_input), 17376, 17380, true) { + Ok(()) => {} + Err(e) => { + return Err(DiffEditError::InternalWebTool(Box::new(e))); + } + }; + + snapshot_diffedit_results(diff_wc, base_ignores, instructions_path_to_cleanup) +} + /// Generates textual diff by the specified `tool`, and writes into `writer`. pub fn generate_diff( ui: &Ui, diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index a8e8299363..7491b0aea0 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. @@ -30,17 +30,17 @@ use pollster::FutureExt; use thiserror::Error; use self::builtin::{edit_diff_builtin, edit_merge_builtin, BuiltinToolError}; -use self::external::{edit_diff_external, DiffCheckoutError, ExternalToolError}; +use self::external::{edit_diff_external, edit_diff_web, DiffCheckoutError, 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)] @@ -63,6 +63,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")] @@ -98,7 +100,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), } @@ -120,7 +123,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"; writeln!( ui.hint(), "Using default editor '{default_editor}'; run `jj config set --user {key} :builtin` \ @@ -134,8 +137,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)) } @@ -221,21 +226,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(), + ), } } } @@ -307,10 +317,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, ), @@ -339,7 +350,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###" @@ -398,7 +409,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###" @@ -545,7 +556,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###" @@ -594,7 +605,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 a1ff2954cf..d8ce0d5e4e 100644 --- a/cli/tests/test_resolve_command.rs +++ b/cli/tests/test_resolve_command.rs @@ -443,7 +443,7 @@ fn test_too_many_parents() { let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]); insta::assert_snapshot!(error, @r###" - Using default editor ':builtin'; run `jj config set --user ui.merge-editor :builtin` to disable this message. + 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. @@ -521,7 +521,7 @@ fn test_file_vs_dir() { "###); let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]); insta::assert_snapshot!(error, @r###" - Using default editor ':builtin'; run `jj config set --user ui.merge-editor :builtin` to disable this message. + 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": @@ -578,7 +578,7 @@ fn test_description_with_dir_and_deletion() { "###); let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]); insta::assert_snapshot!(error, @r###" - Using default editor ':builtin'; run `jj config set --user ui.merge-editor :builtin` to disable this message. + 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":