From c1839b10afeed3b5fe7125e4d2d6f678697144e6 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 | 424 ++++++++++++++++++++++++++++- 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 +- 6 files changed, 526 insertions(+), 31 deletions(-) create mode 100644 cli/src/merge_tools/builtin_web.rs diff --git a/Cargo.lock b/Cargo.lock index 3da0d8ca45..2c272fdb70 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" @@ -468,7 +474,7 @@ dependencies = [ "nom", "pathdiff", "serde", - "toml", + "toml 0.5.11", ] [[package]] @@ -655,6 +661,26 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" +[[package]] +name = "diffedit3" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5fa36ce7f595a2c70d5a31eda0f621d282b61daefb565b4f3851c84a438d7b9" +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 +1495,25 @@ dependencies = [ "regex-syntax 0.8.2", ] +[[package]] +name = "h2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51ee2dd2e4f378392eeff5d51618cd9a63166a2513846bbc55f21cfacd9199d4" +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 +1534,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 +1591,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 +1730,7 @@ checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26" dependencies = [ "equivalent", "hashbrown", + "serde", ] [[package]] @@ -1618,6 +1770,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 +1790,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 +1851,7 @@ dependencies = [ "config", "criterion", "crossterm", + "diffedit3", "dirs", "esl01-renderdag", "futures 0.3.30", @@ -1711,7 +1883,7 @@ dependencies = [ "textwrap", "thiserror", "timeago", - "toml_edit", + "toml_edit 0.19.15", "tracing", "tracing-chrome", "tracing-subscriber", @@ -1932,6 +2104,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 +2168,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.5.0", + "cfg-if", + "libc", +] + [[package]] name = "nom" version = "7.1.3" @@ -2058,6 +2257,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 +2450,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.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 = "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" @@ -2305,6 +2564,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" @@ -2534,6 +2802,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" @@ -2561,6 +2838,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" @@ -2686,6 +2997,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" @@ -2824,6 +3158,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" @@ -3050,6 +3393,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" @@ -3059,6 +3416,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.8", +] + [[package]] name = "toml_datetime" version = "0.6.5" @@ -3081,6 +3451,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.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c12219811e0c1ba077867254e5ad62ee2c9c190b0d957110750ac0cda1ae96cd" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "winnow 0.6.5", +] + [[package]] name = "tracing" version = "0.1.40" @@ -3174,6 +3568,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" @@ -3353,7 +3765,7 @@ dependencies = [ "serde_bser", "thiserror", "tokio", - "tokio-util", + "tokio-util 0.6.10", "winapi", ] @@ -3390,6 +3802,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/cli/Cargo.toml b/cli/Cargo.toml index 1444902a56..04bf5d2d15 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -34,6 +34,7 @@ name = "runner" cargo_metadata = { workspace = true } [dependencies] +diffedit3 = "0.1.2" 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 1338138106..5d2d59e8bc 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":