From a224ee50796a4a115969d7f58b393fd2c797ce15 Mon Sep 17 00:00:00 2001 From: Jonathan LEI Date: Mon, 1 Apr 2024 06:36:28 +0800 Subject: [PATCH] Changed file picker (#5645) Co-authored-by: WJH Co-authored-by: Michael Davis Co-authored-by: Pascal Kuthe --- Cargo.lock | 67 +++++++++++++++++++++ book/src/themes.md | 1 + helix-term/src/commands.rs | 100 ++++++++++++++++++++++++++++++- helix-term/src/keymap/default.rs | 3 +- helix-vcs/Cargo.toml | 2 +- helix-vcs/src/git.rs | 98 ++++++++++++++++++++++++++++-- helix-vcs/src/git/test.rs | 2 +- helix-vcs/src/lib.rs | 99 +++++++++++++++++++++++++----- helix-vcs/src/status.rs | 32 ++++++++++ 9 files changed, 380 insertions(+), 24 deletions(-) create mode 100644 helix-vcs/src/status.rs diff --git a/Cargo.lock b/Cargo.lock index d04a1c33..377e6c05 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -338,6 +338,19 @@ dependencies = [ "syn 2.0.48", ] +[[package]] +name = "dashmap" +version = "5.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "907076dfda823b0b36d2a1bb5f90c96660a5bbcd7729e10727f07858f22c4edc" +dependencies = [ + "cfg-if", + "hashbrown 0.12.3", + "lock_api", + "once_cell", + "parking_lot_core", +] + [[package]] name = "dunce" version = "1.0.4" @@ -536,6 +549,7 @@ dependencies = [ "gix-config", "gix-date", "gix-diff", + "gix-dir", "gix-discover", "gix-features", "gix-filter", @@ -557,6 +571,7 @@ dependencies = [ "gix-revision", "gix-revwalk", "gix-sec", + "gix-status", "gix-submodule", "gix-tempfile", "gix-trace", @@ -699,8 +714,36 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78e605593c2ef74980a534ade0909c7dc57cca72baa30cbb67d2dda621f99ac4" dependencies = [ "bstr", + "gix-command", + "gix-filter", + "gix-fs", "gix-hash", "gix-object", + "gix-path", + "gix-tempfile", + "gix-trace", + "gix-worktree", + "imara-diff", + "thiserror", +] + +[[package]] +name = "gix-dir" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3413ccd29130900c17574678aee640e4847909acae9febf6424dc77b782c6d32" +dependencies = [ + "bstr", + "gix-discover", + "gix-fs", + "gix-ignore", + "gix-index", + "gix-object", + "gix-path", + "gix-pathspec", + "gix-trace", + "gix-utils", + "gix-worktree", "thiserror", ] @@ -1054,6 +1097,28 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "gix-status" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca216db89947eca709f69ec5851aa76f9628e7c7aab7aa5a927d0c619d046bf2" +dependencies = [ + "bstr", + "filetime", + "gix-diff", + "gix-dir", + "gix-features", + "gix-filter", + "gix-fs", + "gix-hash", + "gix-index", + "gix-object", + "gix-path", + "gix-pathspec", + "gix-worktree", + "thiserror", +] + [[package]] name = "gix-submodule" version = "0.10.0" @@ -1075,6 +1140,7 @@ version = "13.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d337955b7af00fb87120d053d87cdfb422a80b9ff7a3aa4057a99c79422dc30" dependencies = [ + "dashmap", "gix-fs", "libc", "once_cell", @@ -1124,6 +1190,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0066432d4c277f9877f091279a597ea5331f68ca410efc874f0bdfb1cd348f92" dependencies = [ + "bstr", "fastrand", "unicode-normalization", ] diff --git a/book/src/themes.md b/book/src/themes.md index 29a8c4ba..0a49053f 100644 --- a/book/src/themes.md +++ b/book/src/themes.md @@ -251,6 +251,7 @@ We use a similar set of scopes as - `gutter` - gutter indicator - `delta` - modifications - `moved` - renamed or moved files/changes + - `conflict` - merge conflicts - `gutter` - gutter indicator #### Interface diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d927d3f4..d0b9047c 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3,10 +3,14 @@ pub(crate) mod lsp; pub(crate) mod typed; pub use dap::*; +use helix_event::status; use helix_stdx::rope::{self, RopeSliceExt}; -use helix_vcs::Hunk; +use helix_vcs::{FileChange, Hunk}; pub use lsp::*; -use tui::widgets::Row; +use tui::{ + text::Span, + widgets::{Cell, Row}, +}; pub use typed::*; use helix_core::{ @@ -39,6 +43,7 @@ use helix_view::{ info::Info, input::KeyEvent, keyboard::KeyCode, + theme::Style, tree, view::View, Document, DocumentId, Editor, ViewId, @@ -54,7 +59,7 @@ use crate::{ filter_picker_entry, job::Callback, keymap::ReverseKeymap, - ui::{self, overlay::overlaid, Picker, Popup, Prompt, PromptEvent}, + ui::{self, menu::Item, overlay::overlaid, Picker, Popup, Prompt, PromptEvent}, }; use crate::job::{self, Jobs}; @@ -324,6 +329,7 @@ impl MappableCommand { buffer_picker, "Open buffer picker", jumplist_picker, "Open jumplist picker", symbol_picker, "Open symbol picker", + changed_file_picker, "Open changed file picker", select_references_to_symbol_under_cursor, "Select symbol references", workspace_symbol_picker, "Open workspace symbol picker", diagnostics_picker, "Open diagnostic picker", @@ -2996,6 +3002,94 @@ fn jumplist_picker(cx: &mut Context) { cx.push_layer(Box::new(overlaid(picker))); } +fn changed_file_picker(cx: &mut Context) { + pub struct FileChangeData { + cwd: PathBuf, + style_untracked: Style, + style_modified: Style, + style_conflict: Style, + style_deleted: Style, + style_renamed: Style, + } + + impl Item for FileChange { + type Data = FileChangeData; + + fn format(&self, data: &Self::Data) -> Row { + let process_path = |path: &PathBuf| { + path.strip_prefix(&data.cwd) + .unwrap_or(path) + .display() + .to_string() + }; + + let (sign, style, content) = match self { + Self::Untracked { path } => ("[+]", data.style_untracked, process_path(path)), + Self::Modified { path } => ("[~]", data.style_modified, process_path(path)), + Self::Conflict { path } => ("[x]", data.style_conflict, process_path(path)), + Self::Deleted { path } => ("[-]", data.style_deleted, process_path(path)), + Self::Renamed { from_path, to_path } => ( + "[>]", + data.style_renamed, + format!("{} -> {}", process_path(from_path), process_path(to_path)), + ), + }; + + Row::new([Cell::from(Span::styled(sign, style)), Cell::from(content)]) + } + } + + let cwd = helix_stdx::env::current_working_dir(); + if !cwd.exists() { + cx.editor + .set_error("Current working directory does not exist"); + return; + } + + let added = cx.editor.theme.get("diff.plus"); + let modified = cx.editor.theme.get("diff.delta"); + let conflict = cx.editor.theme.get("diff.delta.conflict"); + let deleted = cx.editor.theme.get("diff.minus"); + let renamed = cx.editor.theme.get("diff.delta.moved"); + + let picker = Picker::new( + Vec::new(), + FileChangeData { + cwd: cwd.clone(), + style_untracked: added, + style_modified: modified, + style_conflict: conflict, + style_deleted: deleted, + style_renamed: renamed, + }, + |cx, meta: &FileChange, action| { + let path_to_open = meta.path(); + if let Err(e) = cx.editor.open(path_to_open, action) { + let err = if let Some(err) = e.source() { + format!("{}", err) + } else { + format!("unable to open \"{}\"", path_to_open.display()) + }; + cx.editor.set_error(err); + } + }, + ) + .with_preview(|_editor, meta| Some((meta.path().to_path_buf().into(), None))); + let injector = picker.injector(); + + cx.editor + .diff_providers + .clone() + .for_each_changed_file(cwd, move |change| match change { + Ok(change) => injector.push(change).is_ok(), + Err(err) => { + status::report_blocking(err); + true + } + }); + cx.push_layer(Box::new(overlaid(picker))); +} + impl ui::menu::Item for MappableCommand { type Data = ReverseKeymap; diff --git a/helix-term/src/keymap/default.rs b/helix-term/src/keymap/default.rs index ffd076ad..498a9a3e 100644 --- a/helix-term/src/keymap/default.rs +++ b/helix-term/src/keymap/default.rs @@ -225,9 +225,10 @@ pub fn default() -> HashMap { "S" => workspace_symbol_picker, "d" => diagnostics_picker, "D" => workspace_diagnostics_picker, + "g" => changed_file_picker, "a" => code_action, "'" => last_picker, - "g" => { "Debug (experimental)" sticky=true + "G" => { "Debug (experimental)" sticky=true "l" => dap_launch, "r" => dap_restart, "b" => dap_toggle_breakpoint, diff --git a/helix-vcs/Cargo.toml b/helix-vcs/Cargo.toml index d54f5312..872ec64b 100644 --- a/helix-vcs/Cargo.toml +++ b/helix-vcs/Cargo.toml @@ -19,7 +19,7 @@ tokio = { version = "1", features = ["rt", "rt-multi-thread", "time", "sync", "p parking_lot = "0.12" arc-swap = { version = "1.7.1" } -gix = { version = "0.61.0", features = ["attributes"], default-features = false, optional = true } +gix = { version = "0.61.0", features = ["attributes", "status"], default-features = false, optional = true } imara-diff = "0.1.5" anyhow = "1" diff --git a/helix-vcs/src/git.rs b/helix-vcs/src/git.rs index 995bade0..8d935b5f 100644 --- a/helix-vcs/src/git.rs +++ b/helix-vcs/src/git.rs @@ -5,15 +5,24 @@ use std::io::Read; use std::path::Path; use std::sync::Arc; +use gix::bstr::ByteSlice; +use gix::diff::Rewrites; +use gix::dir::entry::Status; use gix::objs::tree::EntryKind; use gix::sec::trust::DefaultForLevel; +use gix::status::{ + index_worktree::iter::Item, + plumbing::index_as_worktree::{Change, EntryStatus}, + UntrackedFiles, +}; use gix::{Commit, ObjectId, Repository, ThreadSafeRepository}; -use crate::DiffProvider; +use crate::{DiffProvider, FileChange}; #[cfg(test)] mod test; +#[derive(Clone, Copy)] pub struct Git; impl Git { @@ -61,10 +70,77 @@ impl Git { Ok(res) } + + /// Emulates the result of running `git status` from the command line. + fn status(repo: &Repository, f: impl Fn(Result) -> bool) -> Result<()> { + let work_dir = repo + .work_dir() + .ok_or_else(|| anyhow::anyhow!("working tree not found"))? + .to_path_buf(); + + let status_platform = repo + .status(gix::progress::Discard)? + // Here we discard the `status.showUntrackedFiles` config, as it makes little sense in + // our case to not list new (untracked) files. We could have respected this config + // if the default value weren't `Collapsed` though, as this default value would render + // the feature unusable to many. + .untracked_files(UntrackedFiles::Files) + // Turn on file rename detection, which is off by default. + .index_worktree_rewrites(Some(Rewrites { + copies: None, + percentage: Some(0.5), + limit: 1000, + })); + + // No filtering based on path + let empty_patterns = vec![]; + + let status_iter = status_platform.into_index_worktree_iter(empty_patterns)?; + + for item in status_iter { + let Ok(item) = item.map_err(|err| f(Err(err.into()))) else { + continue; + }; + let change = match item { + Item::Modification { + rela_path, status, .. + } => { + let path = work_dir.join(rela_path.to_path()?); + match status { + EntryStatus::Conflict(_) => FileChange::Conflict { path }, + EntryStatus::Change(Change::Removed) => FileChange::Deleted { path }, + EntryStatus::Change(Change::Modification { .. }) => { + FileChange::Modified { path } + } + _ => continue, + } + } + Item::DirectoryContents { entry, .. } if entry.status == Status::Untracked => { + FileChange::Untracked { + path: work_dir.join(entry.rela_path.to_path()?), + } + } + Item::Rewrite { + source, + dirwalk_entry, + .. + } => FileChange::Renamed { + from_path: work_dir.join(source.rela_path().to_path()?), + to_path: work_dir.join(dirwalk_entry.rela_path.to_path()?), + }, + _ => continue, + }; + if !f(Ok(change)) { + break; + } + } + + Ok(()) + } } -impl DiffProvider for Git { - fn get_diff_base(&self, file: &Path) -> Result> { +impl Git { + pub fn get_diff_base(&self, file: &Path) -> Result> { debug_assert!(!file.exists() || file.is_file()); debug_assert!(file.is_absolute()); @@ -95,7 +171,7 @@ impl DiffProvider for Git { } } - fn get_current_head_name(&self, file: &Path) -> Result>>> { + pub fn get_current_head_name(&self, file: &Path) -> Result>>> { debug_assert!(!file.exists() || file.is_file()); debug_assert!(file.is_absolute()); let repo_dir = file.parent().context("file has no parent directory")?; @@ -112,6 +188,20 @@ impl DiffProvider for Git { Ok(Arc::new(ArcSwap::from_pointee(name.into_boxed_str()))) } + + pub fn for_each_changed_file( + &self, + cwd: &Path, + f: impl Fn(Result) -> bool, + ) -> Result<()> { + Self::status(&Self::open_repo(cwd, None)?.to_thread_local(), f) + } +} + +impl From for DiffProvider { + fn from(value: Git) -> Self { + DiffProvider::Git(value) + } } /// Finds the object that contains the contents of a file at a specific commit. diff --git a/helix-vcs/src/git/test.rs b/helix-vcs/src/git/test.rs index 9c67d2c3..0f928204 100644 --- a/helix-vcs/src/git/test.rs +++ b/helix-vcs/src/git/test.rs @@ -2,7 +2,7 @@ use std::{fs::File, io::Write, path::Path, process::Command}; use tempfile::TempDir; -use crate::{DiffProvider, Git}; +use crate::Git; fn exec_git_cmd(args: &str, git_dir: &Path) { let res = Command::new("git") diff --git a/helix-vcs/src/lib.rs b/helix-vcs/src/lib.rs index 851fd6e9..7225c38e 100644 --- a/helix-vcs/src/lib.rs +++ b/helix-vcs/src/lib.rs @@ -1,6 +1,9 @@ -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use arc_swap::ArcSwap; -use std::{path::Path, sync::Arc}; +use std::{ + path::{Path, PathBuf}, + sync::Arc, +}; #[cfg(feature = "git")] pub use git::Git; @@ -14,18 +17,14 @@ mod diff; pub use diff::{DiffHandle, Hunk}; -pub trait DiffProvider { - /// Returns the data that a diff should be computed against - /// if this provider is used. - /// The data is returned as raw byte without any decoding or encoding performed - /// to ensure all file encodings are handled correctly. - fn get_diff_base(&self, file: &Path) -> Result>; - fn get_current_head_name(&self, file: &Path) -> Result>>>; -} +mod status; + +pub use status::FileChange; #[doc(hidden)] +#[derive(Clone, Copy)] pub struct Dummy; -impl DiffProvider for Dummy { +impl Dummy { fn get_diff_base(&self, _file: &Path) -> Result> { bail!("helix was compiled without git support") } @@ -33,10 +32,25 @@ impl DiffProvider for Dummy { fn get_current_head_name(&self, _file: &Path) -> Result>>> { bail!("helix was compiled without git support") } + + fn for_each_changed_file( + &self, + _cwd: &Path, + _f: impl Fn(Result) -> bool, + ) -> Result<()> { + bail!("helix was compiled without git support") + } } +impl From for DiffProvider { + fn from(value: Dummy) -> Self { + DiffProvider::Dummy(value) + } +} + +#[derive(Clone)] pub struct DiffProviderRegistry { - providers: Vec>, + providers: Vec, } impl DiffProviderRegistry { @@ -65,14 +79,71 @@ impl DiffProviderRegistry { } }) } + + /// Fire-and-forget changed file iteration. Runs everything in a background task. Keeps + /// iteration until `on_change` returns `false`. + pub fn for_each_changed_file( + self, + cwd: PathBuf, + f: impl Fn(Result) -> bool + Send + 'static, + ) { + tokio::task::spawn_blocking(move || { + if self + .providers + .iter() + .find_map(|provider| provider.for_each_changed_file(&cwd, &f).ok()) + .is_none() + { + f(Err(anyhow!("no diff provider returns success"))); + } + }); + } } impl Default for DiffProviderRegistry { fn default() -> Self { // currently only git is supported // TODO make this configurable when more providers are added - let git: Box = Box::new(Git); - let providers = vec![git]; + let providers = vec![Git.into()]; DiffProviderRegistry { providers } } } + +/// A union type that includes all types that implement [DiffProvider]. We need this type to allow +/// cloning [DiffProviderRegistry] as `Clone` cannot be used in trait objects. +#[derive(Clone)] +pub enum DiffProvider { + Dummy(Dummy), + #[cfg(feature = "git")] + Git(Git), +} + +impl DiffProvider { + fn get_diff_base(&self, file: &Path) -> Result> { + match self { + Self::Dummy(inner) => inner.get_diff_base(file), + #[cfg(feature = "git")] + Self::Git(inner) => inner.get_diff_base(file), + } + } + + fn get_current_head_name(&self, file: &Path) -> Result>>> { + match self { + Self::Dummy(inner) => inner.get_current_head_name(file), + #[cfg(feature = "git")] + Self::Git(inner) => inner.get_current_head_name(file), + } + } + + fn for_each_changed_file( + &self, + cwd: &Path, + f: impl Fn(Result) -> bool, + ) -> Result<()> { + match self { + Self::Dummy(inner) => inner.for_each_changed_file(cwd, f), + #[cfg(feature = "git")] + Self::Git(inner) => inner.for_each_changed_file(cwd, f), + } + } +} diff --git a/helix-vcs/src/status.rs b/helix-vcs/src/status.rs new file mode 100644 index 00000000..f3433490 --- /dev/null +++ b/helix-vcs/src/status.rs @@ -0,0 +1,32 @@ +use std::path::{Path, PathBuf}; + +pub enum FileChange { + Untracked { + path: PathBuf, + }, + Modified { + path: PathBuf, + }, + Conflict { + path: PathBuf, + }, + Deleted { + path: PathBuf, + }, + Renamed { + from_path: PathBuf, + to_path: PathBuf, + }, +} + +impl FileChange { + pub fn path(&self) -> &Path { + match self { + Self::Untracked { path } => path, + Self::Modified { path } => path, + Self::Conflict { path } => path, + Self::Deleted { path } => path, + Self::Renamed { to_path, .. } => to_path, + } + } +}