From acdc670d986563095b76d1f771678c9fbf2da722 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Thu, 31 Aug 2023 01:41:51 -0400 Subject: [PATCH 1/4] remove diff provider fallback behavior --- helix-term/src/commands/typed.rs | 4 +-- helix-vcs/src/git.rs | 8 ++--- helix-vcs/src/git/test.rs | 2 +- helix-vcs/src/lib.rs | 57 ++------------------------------ helix-view/src/document.rs | 27 +++++++++------ helix-view/src/editor.rs | 24 ++++++++++---- 6 files changed, 43 insertions(+), 79 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 4148257fc494..7aaaf4a8d2b8 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1284,7 +1284,7 @@ fn reload( let scrolloff = cx.editor.config().scrolloff; let (view, doc) = current!(cx.editor); - doc.reload(view, &cx.editor.diff_providers).map(|_| { + doc.reload(view, &cx.editor.diff_provider).map(|_| { view.ensure_cursor_in_view(doc, scrolloff); })?; if let Some(path) = doc.path() { @@ -1332,7 +1332,7 @@ fn reload_all( // Ensure that the view is synced with the document's history. view.sync_changes(doc); - doc.reload(view, &cx.editor.diff_providers)?; + doc.reload(view, &cx.editor.diff_provider)?; if let Some(path) = doc.path() { cx.editor .language_servers diff --git a/helix-vcs/src/git.rs b/helix-vcs/src/git.rs index 88dba70c819e..6fc1e35f168f 100644 --- a/helix-vcs/src/git.rs +++ b/helix-vcs/src/git.rs @@ -7,8 +7,6 @@ use gix::objs::tree::EntryMode; use gix::sec::trust::DefaultForLevel; use gix::{Commit, ObjectId, Repository, ThreadSafeRepository}; -use crate::DiffProvider; - #[cfg(test)] mod test; @@ -59,10 +57,8 @@ impl Git { Ok(res) } -} -impl DiffProvider for Git { - fn get_diff_base(&self, file: &Path) -> Result> { + pub fn get_diff_base(&self, file: &Path) -> Result> { debug_assert!(!file.exists() || file.is_file()); debug_assert!(file.is_absolute()); @@ -101,7 +97,7 @@ impl DiffProvider for Git { Ok(data) } - 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")?; diff --git a/helix-vcs/src/git/test.rs b/helix-vcs/src/git/test.rs index 9c67d2c331de..0f928204a354 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 63487fbcde6c..2dbe6b4055c1 100644 --- a/helix-vcs/src/lib.rs +++ b/helix-vcs/src/lib.rs @@ -14,65 +14,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>>>; -} - #[doc(hidden)] pub struct Dummy; -impl DiffProvider for Dummy { - fn get_diff_base(&self, _file: &Path) -> Result> { +impl Dummy { + pub fn get_diff_base(&self, _file: &Path) -> Result> { bail!("helix was compiled without git support") } - fn get_current_head_name(&self, _file: &Path) -> Result>>> { + pub fn get_current_head_name(&self, _file: &Path) -> Result>>> { bail!("helix was compiled without git support") } } - -pub struct DiffProviderRegistry { - providers: Vec>, -} - -impl DiffProviderRegistry { - pub fn get_diff_base(&self, file: &Path) -> Option> { - self.providers - .iter() - .find_map(|provider| match provider.get_diff_base(file) { - Ok(res) => Some(res), - Err(err) => { - log::info!("{err:#?}"); - log::info!("failed to open diff base for {}", file.display()); - None - } - }) - } - - pub fn get_current_head_name(&self, file: &Path) -> Option>>> { - self.providers - .iter() - .find_map(|provider| match provider.get_current_head_name(file) { - Ok(res) => Some(res), - Err(err) => { - log::info!("{err:#?}"); - log::info!("failed to obtain current head name for {}", file.display()); - None - } - }) - } -} - -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]; - DiffProviderRegistry { providers } - } -} diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index bb61eaa6aae6..c058ca1fcf42 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -8,7 +8,7 @@ use helix_core::doc_formatter::TextFormat; use helix_core::encoding::Encoding; use helix_core::syntax::{Highlight, LanguageServerFeature}; use helix_core::text_annotations::{InlineAnnotation, TextAnnotations}; -use helix_vcs::{DiffHandle, DiffProviderRegistry}; +use helix_vcs::{DiffHandle, Git}; use ::parking_lot::Mutex; use serde::de::{self, Deserialize, Deserializer}; @@ -991,11 +991,7 @@ impl Document { } /// Reload the document from its path. - pub fn reload( - &mut self, - view: &mut View, - provider_registry: &DiffProviderRegistry, - ) -> Result<(), Error> { + pub fn reload(&mut self, view: &mut View, diff_provider: &Git) -> Result<(), Error> { let encoding = self.encoding; let path = self .path() @@ -1021,12 +1017,23 @@ impl Document { self.detect_indent_and_line_ending(); - match provider_registry.get_diff_base(&path) { - Some(diff_base) => self.set_diff_base(diff_base), - None => self.diff_handle = None, + match diff_provider.get_diff_base(&path) { + Ok(diff_base) => self.set_diff_base(diff_base), + Err(err) => { + log::info!("{err:#?}"); + log::info!("failed to open diff base for for {}", path.display()); + self.diff_handle = None; + } } - self.version_control_head = provider_registry.get_current_head_name(&path); + self.version_control_head = match diff_provider.get_current_head_name(&path) { + Ok(res) => Some(res), + Err(err) => { + log::info!("{err:#?}"); + log::info!("failed to obtain current head name for {}", path.display()); + None + } + }; Ok(()) } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 5a81cb5d6da8..f4bf5ab8fcc4 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -11,11 +11,11 @@ use crate::{ Align, Document, DocumentId, View, ViewId, }; use dap::StackFrame; -use helix_vcs::DiffProviderRegistry; use futures_util::stream::select_all::SelectAll; use futures_util::{future, StreamExt}; use helix_lsp::Call; +use helix_vcs::Git; use tokio_stream::wrappers::UnboundedReceiverStream; use std::{ @@ -889,7 +889,7 @@ pub struct Editor { pub macro_replaying: Vec, pub language_servers: helix_lsp::Registry, pub diagnostics: BTreeMap>, - pub diff_providers: DiffProviderRegistry, + pub diff_provider: Git, pub debugger: Option, pub debugger_events: SelectAll>, @@ -1035,7 +1035,7 @@ impl Editor { theme: theme_loader.default(), language_servers, diagnostics: BTreeMap::new(), - diff_providers: DiffProviderRegistry::default(), + diff_provider: Git, debugger: None, debugger_events: SelectAll::new(), breakpoints: HashMap::new(), @@ -1452,10 +1452,22 @@ impl Editor { self.config.clone(), )?; - if let Some(diff_base) = self.diff_providers.get_diff_base(&path) { - doc.set_diff_base(diff_base); + match self.diff_provider.get_diff_base(&path) { + Ok(diff_base) => doc.set_diff_base(diff_base), + Err(err) => { + log::info!("{err:#?}"); + log::info!("failed to open diff base for for {}", path.display()); + } } - doc.set_version_control_head(self.diff_providers.get_current_head_name(&path)); + + doc.set_version_control_head(match self.diff_provider.get_current_head_name(&path) { + Ok(res) => Some(res), + Err(err) => { + log::info!("{err:#?}"); + log::info!("failed to obtain current head name for {}", path.display()); + None + } + }); let id = self.new_document(doc); self.launch_language_servers(id); From 18dfca0fb03258811710bc170cd308a2e8d4d40d Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Thu, 31 Aug 2023 04:29:23 -0400 Subject: [PATCH 2/4] add a diff source for the diff gutter based on the on-disk file contents --- helix-term/src/application.rs | 14 +++++- helix-term/src/commands/typed.rs | 11 ++-- helix-view/src/document.rs | 86 ++++++++++++++++++++------------ helix-view/src/editor.rs | 38 ++++++++------ 4 files changed, 96 insertions(+), 53 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index ed085749b6d3..4095e8c5f1b1 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -13,7 +13,7 @@ use helix_lsp::{ use helix_view::{ align_view, document::DocumentSavedEventResult, - editor::{ConfigEvent, EditorEvent}, + editor::{ConfigEvent, DiffSource, EditorEvent}, graphics::Rect, theme, tree::Layout, @@ -362,11 +362,15 @@ impl Application { // the Application can apply it. ConfigEvent::Update(editor_config) => { let mut app_config = (*self.config.load().clone()).clone(); + let update_diff_base = app_config.editor.diff_source != editor_config.diff_source; app_config.editor = *editor_config; if let Err(err) = self.terminal.reconfigure(app_config.editor.clone().into()) { self.editor.set_error(err.to_string()); }; self.config.store(Arc::new(app_config)); + if update_diff_base { + self.editor.update_diff_base(); + } } } @@ -568,6 +572,14 @@ impl Application { self.editor.refresh_language_servers(id); } + let diff_source = self.editor.config().diff_source; + let doc = doc_mut!(self.editor, &doc_save_event.doc_id); + if diff_source == DiffSource::File { + if let Some(path) = doc.path().cloned() { + doc.update_diff_base(&path, &self.editor.diff_provider, diff_source); + } + } + // TODO: fix being overwritten by lsp self.editor.set_status(format!( "'{}' written, {}L {}B", diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 7aaaf4a8d2b8..93165b421bf3 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1283,10 +1283,12 @@ fn reload( } let scrolloff = cx.editor.config().scrolloff; + let diff_source = cx.editor.config().diff_source; let (view, doc) = current!(cx.editor); - doc.reload(view, &cx.editor.diff_provider).map(|_| { - view.ensure_cursor_in_view(doc, scrolloff); - })?; + doc.reload(view, &cx.editor.diff_provider, diff_source) + .map(|_| { + view.ensure_cursor_in_view(doc, scrolloff); + })?; if let Some(path) = doc.path() { cx.editor .language_servers @@ -1324,6 +1326,7 @@ fn reload_all( .collect(); for (doc_id, view_ids) in docs_view_ids { + let diff_source = cx.editor.config().diff_source; let doc = doc_mut!(cx.editor, &doc_id); // Every doc is guaranteed to have at least 1 view at this point. @@ -1332,7 +1335,7 @@ fn reload_all( // Ensure that the view is synced with the document's history. view.sync_changes(doc); - doc.reload(view, &cx.editor.diff_provider)?; + doc.reload(view, &cx.editor.diff_provider, diff_source)?; if let Some(path) = doc.path() { cx.editor .language_servers diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index c058ca1fcf42..0e61102c71c3 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -33,7 +33,7 @@ use helix_core::{ ChangeSet, Diagnostic, LineEnding, Range, Rope, RopeBuilder, Selection, Syntax, Transaction, }; -use crate::editor::Config; +use crate::editor::{Config, DiffSource}; use crate::{DocumentId, Editor, Theme, View, ViewId}; /// 8kB of buffer space for encoding and decoding `Rope`s. @@ -991,7 +991,12 @@ impl Document { } /// Reload the document from its path. - pub fn reload(&mut self, view: &mut View, diff_provider: &Git) -> Result<(), Error> { + pub fn reload( + &mut self, + view: &mut View, + diff_provider: &Git, + diff_source: DiffSource, + ) -> Result<(), Error> { let encoding = self.encoding; let path = self .path() @@ -1017,23 +1022,7 @@ impl Document { self.detect_indent_and_line_ending(); - match diff_provider.get_diff_base(&path) { - Ok(diff_base) => self.set_diff_base(diff_base), - Err(err) => { - log::info!("{err:#?}"); - log::info!("failed to open diff base for for {}", path.display()); - self.diff_handle = None; - } - } - - self.version_control_head = match diff_provider.get_current_head_name(&path) { - Ok(res) => Some(res), - Err(err) => { - log::info!("{err:#?}"); - log::info!("failed to obtain current head name for {}", path.display()); - None - } - }; + self.update_diff_base(&path, diff_provider, diff_source); Ok(()) } @@ -1589,27 +1578,60 @@ impl Document { } /// Intialize/updates the differ for this document with a new base. - pub fn set_diff_base(&mut self, diff_base: Vec) { + fn set_diff_base_bytes(&mut self, diff_base: Vec) { if let Ok((diff_base, ..)) = from_reader(&mut diff_base.as_slice(), Some(self.encoding)) { - if let Some(differ) = &self.diff_handle { - differ.update_diff_base(diff_base); - return; - } - self.diff_handle = Some(DiffHandle::new(diff_base, self.text.clone())) + self.set_diff_base(diff_base); } else { self.diff_handle = None; } } - pub fn version_control_head(&self) -> Option>> { - self.version_control_head.as_ref().map(|a| a.load_full()) + fn set_diff_base(&mut self, diff_base: Rope) { + if let Some(differ) = &self.diff_handle { + differ.update_diff_base(diff_base); + return; + } + self.diff_handle = Some(DiffHandle::new(diff_base, self.text.clone())) } - pub fn set_version_control_head( - &mut self, - version_control_head: Option>>>, - ) { - self.version_control_head = version_control_head; + pub fn update_diff_base(&mut self, path: &Path, diff_provider: &Git, diff_source: DiffSource) { + match diff_source { + DiffSource::Git => match diff_provider.get_diff_base(path) { + Ok(diff_base) => self.set_diff_base_bytes(diff_base), + Err(err) => { + log::info!("{err:#?}"); + log::info!("failed to open diff base for for {}", path.display()); + self.diff_handle = None; + } + }, + DiffSource::File => { + if self.is_modified() { + match std::fs::read(path) { + Ok(bytes) => self.set_diff_base_bytes(bytes), + Err(err) => { + log::info!("{err:#?}"); + log::info!("failed to open diff base for for {}", path.display()); + self.diff_handle = None; + } + } + } else { + self.set_diff_base(self.text.clone()); + } + } + } + + self.version_control_head = match diff_provider.get_current_head_name(path) { + Ok(res) => Some(res), + Err(err) => { + log::info!("{err:#?}"); + log::info!("failed to obtain current head name for {}", path.display()); + None + } + }; + } + + pub fn version_control_head(&self) -> Option>> { + self.version_control_head.as_ref().map(|a| a.load_full()) } #[inline] diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index f4bf5ab8fcc4..1068a4f1e22d 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -73,6 +73,14 @@ where ) } +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum DiffSource { + #[default] + Git, + File, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default, deny_unknown_fields)] pub struct GutterConfig { @@ -291,6 +299,9 @@ pub struct Config { pub insert_final_newline: bool, /// Enables smart tab pub smart_tab: Option, + #[serde(default)] + /// What the diff gutter should diff against + pub diff_source: DiffSource, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] @@ -841,6 +852,7 @@ impl Default for Config { default_line_ending: LineEndingConfig::default(), insert_final_newline: true, smart_tab: Some(SmartTabConfig::default()), + diff_source: DiffSource::default(), } } } @@ -1452,22 +1464,7 @@ impl Editor { self.config.clone(), )?; - match self.diff_provider.get_diff_base(&path) { - Ok(diff_base) => doc.set_diff_base(diff_base), - Err(err) => { - log::info!("{err:#?}"); - log::info!("failed to open diff base for for {}", path.display()); - } - } - - doc.set_version_control_head(match self.diff_provider.get_current_head_name(&path) { - Ok(res) => Some(res), - Err(err) => { - log::info!("{err:#?}"); - log::info!("failed to obtain current head name for {}", path.display()); - None - } - }); + doc.update_diff_base(&path, &self.diff_provider, self.config().diff_source); let id = self.new_document(doc); self.launch_language_servers(id); @@ -1839,6 +1836,15 @@ impl Editor { .as_ref() .and_then(|debugger| debugger.current_stack_frame()) } + + pub fn update_diff_base(&mut self) { + let diff_source = self.config().diff_source; + for doc in self.documents.values_mut() { + if let Some(path) = doc.path().cloned() { + doc.update_diff_base(&path, &self.diff_provider, diff_source); + } + } + } } fn try_restore_indent(doc: &mut Document, view: &mut View) { From d011db26d0c14b26262424198da8db5c13580ad2 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Thu, 31 Aug 2023 21:30:46 -0400 Subject: [PATCH 3/4] also allow disabling the diff provider entirely --- helix-view/src/document.rs | 3 +++ helix-view/src/editor.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0e61102c71c3..3b2d318c17f0 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1618,6 +1618,9 @@ impl Document { self.set_diff_base(self.text.clone()); } } + DiffSource::None => { + self.diff_handle = None; + } } self.version_control_head = match diff_provider.get_current_head_name(path) { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 1068a4f1e22d..6432af74d6f2 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -79,6 +79,7 @@ pub enum DiffSource { #[default] Git, File, + None, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] From adb8996fd2a58173bb04c3e22e1e7d60a5035dc4 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Thu, 31 Aug 2023 23:33:53 -0400 Subject: [PATCH 4/4] add documentation for the diff-source configuration option --- book/src/configuration.md | 1 + 1 file changed, 1 insertion(+) diff --git a/book/src/configuration.md b/book/src/configuration.md index f70ce5470dc8..34948b831bd6 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -65,6 +65,7 @@ Its settings will be merged with the configuration directory `config.toml` and t | `workspace-lsp-roots` | Directories relative to the workspace root that are treated as LSP roots. Should only be set in `.helix/config.toml` | `[]` | | `default-line-ending` | The line ending to use for new documents. Can be `native`, `lf`, `crlf`, `ff`, `cr` or `nel`. `native` uses the platform's native line ending (`crlf` on Windows, otherwise `lf`). | `native` | | `insert-final-newline` | Whether to automatically insert a trailing line-ending on write if missing | `true` | +| `diff-source` | The source to use for diff operations (gutter, `:diffg`, `]g`, etc). Can be `git`, `file` (uses the contents of the file on disk), or `none`. | `git` | ### `[editor.statusline]` Section