fix(core): harden repo and diff behavior

Author: hewigovens

Cargo.lock

@@ -27,6 +27,9 @@ serde_json = "1"
 sha2 = "0.11.0"
 hex = "0.4"
 similar = "3.1.1"
+unicode-width = "0.2"
+unicode-segmentation = "1"
+nucleo-matcher = "0.3"
 toml = "1.1"
 directories = "6"
 

Cargo.toml

@@ -14,6 +14,7 @@ gix-url = { workspace = true }
 percent-encoding = { workspace = true }
 jj-diff = { version = "0.1.0", path = "../jj-diff" }
 jayjay-network = { version = "0.1.0", path = "../jayjay-network" }
+nucleo-matcher = { workspace = true }
 thiserror = { workspace = true }
 pollster = { workspace = true }
 chrono = { workspace = true }

crates/jayjay-core/Cargo.toml

@@ -0,0 +1,67 @@
+use nucleo_matcher::pattern::{CaseMatching, Normalization, Pattern};
+use nucleo_matcher::{Config, Matcher, Utf32Str};
+
+/// Rank `candidates` against `query`, best match first; returns their indices.
+/// Whitespace splits the query into atoms that must all match. An empty query
+/// keeps every candidate in original order.
+pub fn rank(query: &str, candidates: &[String]) -> Vec<u32> {
+    let query = query.trim();
+    if query.is_empty() {
+        return (0..candidates.len() as u32).collect();
+    }
+    let mut matcher = Matcher::new(Config::DEFAULT);
+    let pattern = Pattern::parse(query, CaseMatching::Ignore, Normalization::Smart);
+    let mut buf = Vec::new();
+    let mut scored: Vec<(u32, u32)> = candidates
+        .iter()
+        .enumerate()
+        .filter_map(|(ix, candidate)| {
+            pattern
+                .score(Utf32Str::new(candidate, &mut buf), &mut matcher)
+                .map(|score| (score, ix as u32))
+        })
+        .collect();
+    // Ties keep declaration order.
+    scored.sort_by(|a, b| b.0.cmp(&a.0).then(a.1.cmp(&b.1)));
+    scored.into_iter().map(|(_, ix)| ix).collect()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::rank;
+
+    fn candidates(items: &[&str]) -> Vec<String> {
+        items.iter().map(|s| s.to_string()).collect()
+    }
+
+    #[test]
+    fn empty_query_keeps_original_order() {
+        let items = candidates(&["b", "a"]);
+        assert_eq!(rank("", &items), vec![0, 1]);
+        assert_eq!(rank("  ", &items), vec![0, 1]);
+    }
+
+    #[test]
+    fn matches_subsequences_case_insensitively() {
+        let items = candidates(&["Toggle Side-by-side Diff", "Open Settings"]);
+        assert_eq!(rank("tsd", &items), vec![0]);
+        assert_eq!(rank("SETT", &items), vec![1]);
+        assert!(rank("bookmark", &items).is_empty());
+    }
+
+    #[test]
+    fn all_query_words_must_match() {
+        let items = candidates(&["Toggle Tree File List tree file folder list", "Refresh"]);
+        assert_eq!(rank("tree list", &items), vec![0]);
+        assert!(rank("tree bookmark", &items).is_empty());
+    }
+
+    #[test]
+    fn closer_matches_rank_first() {
+        let items = candidates(&["The Memo Editor", "Theme: Dark", "Theme: Light"]);
+        let ranked = rank("theme", &items);
+        assert_eq!(ranked.len(), 3);
+        // Contiguous "Theme" beats the scattered match; ties keep order.
+        assert_eq!(&ranked[..2], &[1, 2]);
+    }
+}

crates/jayjay-core/src/fuzzy.rs

@@ -1,7 +1,11 @@
 use std::path::Path;
-use std::process::Command;
 
-use crate::{CoreError, CoreResult, jj_binary};
+use crate::{CoreError, CoreResult, jj_binary, repo::subprocess_command};
+
+/// Palette runs capture stdout/stderr, so an interactive editor would hang
+/// forever. `["false"]` makes editor-requiring commands (`describe`/`commit`/
+/// `split` without -m) fail fast with a clear "Failed to edit" error.
+const NON_INTERACTIVE_ARGS: &[&str] = &["--config", r#"ui.editor=["false"]"#];
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct JjCommand {
@@ -54,7 +58,8 @@ impl JjCommand {
             });
         }
 
-        let output = Command::new(jj_binary())
+        let output = subprocess_command(&jj_binary())
+            .args(NON_INTERACTIVE_ARGS)
             .args(&args)
             .current_dir(path)
             .output()
@@ -204,14 +209,18 @@ mod tests {
 
     #[test]
     fn extracts_prefixed_command_body() {
-        assert_eq!(
-            JjCommand::from_palette_query("jj log").map(JjCommand::into_raw),
-            Some("log".to_owned())
-        );
-        assert_eq!(
-            JjCommand::from_palette_query("!status").map(JjCommand::into_raw),
-            Some("status".to_owned())
-        );
-        assert_eq!(JjCommand::from_palette_query("status"), None);
+        let body = |q: &str| JjCommand::from_palette_query(q).map(JjCommand::into_raw);
+        assert_eq!(body("jj log"), Some("log".to_owned()));
+        assert_eq!(body("!status"), Some("status".to_owned()));
+        // Bare prefixes mean "jj mode, no body yet".
+        assert_eq!(body("jj"), Some(String::new()));
+        assert_eq!(body("jj "), Some(String::new()));
+        assert_eq!(body("!"), Some(String::new()));
+        // Leading whitespace after the prefix is trimmed.
+        assert_eq!(body("jj   log"), Some("log".to_owned()));
+        assert_eq!(body("!  status"), Some("status".to_owned()));
+        // Plain words are not jj commands.
+        assert_eq!(body("status"), None);
+        assert_eq!(body("jjlog"), None);
     }
 }

crates/jayjay-core/src/jj_command.rs

@@ -2,8 +2,11 @@ pub use jj_diff as diff;
 pub use jj_diff::syntax;
 pub mod dag;
 pub mod file_tree;
+pub mod fuzzy;
 pub mod hash;
 mod jj_command;
+pub mod palette;
+pub mod placeholder;
 mod repo;
 pub mod review;
 pub mod theme;

crates/jayjay-core/src/lib.rs

@@ -0,0 +1,107 @@
+//! Command palette history: dedupe newest-first with a cap, and cursor recall.
+
+const HISTORY_LIMIT: usize = 20;
+
+/// Recall outcome: the query to show, with `index` `None` for the live "jj " query past the newest entry.
+pub struct HistoryRecall {
+    pub query: String,
+    pub index: Option<usize>,
+}
+
+/// Push `command` onto `history` newest-first, deduped, capped at the limit.
+pub fn record(command: &str, history: &[String]) -> Vec<String> {
+    let command = command.trim();
+    if command.is_empty() {
+        return history.to_vec();
+    }
+    let mut values = vec![command.to_owned()];
+    values.extend(
+        history
+            .iter()
+            .filter(|value| value.as_str() != command)
+            .cloned(),
+    );
+    values.truncate(HISTORY_LIMIT);
+    values
+}
+
+/// Walk the cursor one step (`older` toward older entries, else newer); `None` only when history is empty.
+pub fn recall(
+    history: &[String],
+    history_index: Option<usize>,
+    older: bool,
+) -> Option<HistoryRecall> {
+    if history.is_empty() {
+        return None;
+    }
+    let index = if older {
+        Some(
+            history_index
+                .map(|index| index.saturating_add(1))
+                .unwrap_or(0)
+                .min(history.len() - 1),
+        )
+    } else if let Some(index) = history_index
+        && index > 0
+    {
+        Some(index - 1)
+    } else {
+        None
+    };
+    Some(HistoryRecall {
+        query: index
+            .map(|index| format!("jj {}", history[index]))
+            .unwrap_or_else(|| "jj ".to_owned()),
+        index,
+    })
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn record_dedupes_and_keeps_newest_first() {
+        let history = record("status", &[]);
+        let history = record("log -r @", &history);
+        let history = record("status", &history);
+
+        assert_eq!(history, vec!["status", "log -r @"]);
+    }
+
+    #[test]
+    fn record_caps_at_history_limit() {
+        let mut history = Vec::new();
+        for i in 0..(HISTORY_LIMIT + 5) {
+            history = record(&format!("cmd {i}"), &history);
+        }
+        assert_eq!(history.len(), HISTORY_LIMIT);
+        assert_eq!(history[0], format!("cmd {}", HISTORY_LIMIT + 4));
+    }
+
+    #[test]
+    fn recall_walks_older_and_newer_entries() {
+        let history = vec!["status".to_owned(), "log -r @".to_owned()];
+
+        let first = recall(&history, None, true).expect("first recall");
+        assert_eq!(first.query, "jj status");
+        assert_eq!(first.index, Some(0));
+
+        let second = recall(&history, first.index, true).expect("second recall");
+        assert_eq!(second.query, "jj log -r @");
+        assert_eq!(second.index, Some(1));
+
+        let newer = recall(&history, second.index, false).expect("newer recall");
+        assert_eq!(newer.query, "jj status");
+        assert_eq!(newer.index, Some(0));
+
+        let live_query = recall(&history, newer.index, false).expect("live query recall");
+        assert_eq!(live_query.query, "jj ");
+        assert_eq!(live_query.index, None);
+    }
+
+    #[test]
+    fn recall_on_empty_history_returns_none() {
+        assert!(recall(&[], None, true).is_none());
+    }
+}

crates/jayjay-core/src/palette.rs

@@ -0,0 +1,77 @@
+//! Classifies diff content as editable text vs. a synthetic placeholder the
+//! diff layer emits for content it cannot show inline. Single source of truth
+//! so the Rust diff-edit safety check and the Swift shell cannot drift apart;
+//! producers live in `repo/diff/materialize.rs` and `repo/diff/entry.rs`.
+
+/// Every placeholder prefix the diff layer can emit. Keep in sync with the
+/// producers in `repo/diff` — a new placeholder must be listed here.
+const PLACEHOLDER_PREFIXES: &[&str] = &[
+    "<binary file",
+    "<directory>",
+    "<git lfs ",
+    "<git submodule",
+    "<conflict",
+    "<access denied",
+    "<image ",
+];
+
+/// True when `text` is editable text rather than a placeholder.
+pub fn is_editable_text(text: &str) -> bool {
+    !PLACEHOLDER_PREFIXES
+        .iter()
+        .any(|prefix| text.starts_with(prefix))
+}
+
+/// True when `text` is a Git LFS pointer/object placeholder.
+pub fn is_git_lfs_placeholder(text: &str) -> bool {
+    text.starts_with("<git lfs ")
+}
+
+/// True when `text` is a Git submodule placeholder.
+pub fn is_git_submodule_placeholder(text: &str) -> bool {
+    text.starts_with("<git submodule")
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn real_text_is_editable() {
+        assert!(is_editable_text("let x = 1;"));
+        assert!(is_editable_text(""));
+        // A literal '<' that is not one of our placeholders stays editable.
+        assert!(is_editable_text("<html>"));
+    }
+
+    #[test]
+    fn every_placeholder_is_non_editable() {
+        for sample in [
+            "<binary file (742800 bytes)>",
+            "<directory>",
+            "<git lfs pointer sha256:abc (10 bytes)>",
+            "<git lfs object sha256:abc (10 bytes)>",
+            "<git submodule deadbeef>",
+            "<conflict>",
+            "<access denied: permission>",
+            "<image (100 bytes)>",
+        ] {
+            assert!(
+                !is_editable_text(sample),
+                "should be non-editable: {sample}"
+            );
+        }
+    }
+
+    #[test]
+    fn lfs_and_submodule_classifiers() {
+        assert!(is_git_lfs_placeholder(
+            "<git lfs pointer sha256:abc (10 bytes)>"
+        ));
+        assert!(!is_git_lfs_placeholder("<git submodule deadbeef>"));
+        assert!(is_git_submodule_placeholder("<git submodule deadbeef>"));
+        assert!(!is_git_submodule_placeholder(
+            "<git lfs object sha256:abc (1 bytes)>"
+        ));
+    }
+}