From 29f442887a62c49524ac8b2b34d3ec154d4a4f6e Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 18 Mar 2025 01:11:57 +0000 Subject: [PATCH 01/22] feat: Inline Git Blame fix: use relative path when finding file style: cargo fmt _ chore: better error message refactor: rename to `blame_line` fix: use line of primary cursor for git blame feat: basic implementation of blocking Blame handler feat: implement basic virtual text (end of line blame) feat: figure out how to draw stuff at the end of lines feat: implement end of line virtual text for the current line feat: implement inline git blame chore: clean up chore: remove unused import _ chore: set `blame` to `false` by default docs: document `[editor.vcs.blame]` chore: add progress perf: use background task for worker _ chore: remove unnecessary panic!s chore: remove commented code refactor: remove some layers of abstraction refactor: remove nesting feat: [editor.vcs] -> [editor.version-control] fix: account for inserted and deleted lines _ refactor: extract into a `blame` module feat: allow using custom commit format feat: allow more customizability for inline blame test: add tests for custom inline commit parsser refactor: rename `blame` -> `blame_line` _ _ test: create helper macros for tests test: make test syntax more expressive. Allow specifying line numbers that just got added test: with interspersed lines feat: add `line_blame` static command _ test: add an extra test case test: add ability to have `delete`d lines test: fix on windows (?) test: `delete` test case test: add extra step to test case test: add documentation for macro refactor: use `hashmap!` macro refactor: collapse match arm fix: remove panic perf: update inline git blame every 150 milliseconds instead of on each command test: add attributes on blocks style: move function earlier in the file perf: cache blame results in a hashma chore: remove log statements chore: clean up. ALSO: removes checking for inline blame every N seconds. _ perf: use mspc instead of busy-wait docs: add information why we don't optimize the repo _ test: add back the commented out tests chore: comment out cfg(not(windows)) test: add extra history to blame test docs: remove incorrect static command _ test: disable test on windows feat: send inline blame event update when reloading or saving the document feat: rename `version-control` -> `inline-blame` feat: update theme key used for inline-blame chore: remove unused #![allow] chore: style: remove accidental formatting docs: remove incorrect key perf: Use a single `ThreadSafeRepository` instead of re-constructing it each time feat: add `inline_blame` static command bound to `space + B` style: revert formatting in keymap.md chore: do not compute blame for document when changing config option This isn't needed anymore because the inline-blame will be computed regardless if `inline_blame.enable` is set or not style: remove newline refactor: use `fold` instead of loop chore: clean up feat: log error forl line blame when it happens feat: improve message when we don't have the blame We know that we don't have it because we're still calculating it. feat: do not render inline blame for empty lines _ feat: do not show blame output when we are on a hunk that was added refactor: remove additional wrapper methods fix _ feat: more readable time for git blame chr feat: feat: improved error handling fix: path separator on Windows test: disable on windows refactor: move pretty date function formatter into `helix-stdx` perf: do not use a syscall on each render chore: add TODO comment to update gix version chore: use `gix::path` conversion from Path -> BString _ _ chore: do not update file blame on document save This is not needed because when we write the file, we don't make a new commit so the blame will not change. refactor: use statics to get time elapsed instead of editor state refactor: do not use custom event, use handler instead fix: do not spawn a new handler docs: correct examples for `editor.inline-blame.format` docs: correct static command name refactor: add comments, and improve variable names I didn't really understand this function when I made it. Was just copy-pasted from end of line diagnostics I wanted to know what this is actually doing, so I investigated and while doing this also added comments and improved names of variables so others can understand too fix: time in future is accounted for perf: inline some functions that are called in only 1 place, during a render loop perf: add option to disable requesting inline blame in the background fix: request blame again when document is reloaded chore: inline blame is disabled with request on demand feat: when requesting line blame with "blame on demand", show blame in status perf: use less allocations perf: less allocations in `format_relative_time` _ _ _ _ docs: correct name of command _ feat: improve error message _ feat: rename enum variants for inline blame behaviour docs: improve description of behaviour field --- Cargo.lock | 52 +- book/src/editor.md | 36 ++ book/src/generated/static-cmd.md | 1 + book/src/keymap.md | 1 + book/src/themes.md | 1 + helix-stdx/src/lib.rs | 1 + helix-stdx/src/time.rs | 81 +++ helix-term/src/commands.rs | 50 ++ helix-term/src/commands/typed.rs | 34 ++ helix-term/src/handlers.rs | 5 + helix-term/src/handlers/blame.rs | 94 +++ helix-term/src/keymap/default.rs | 1 + helix-term/src/ui/editor.rs | 22 +- helix-term/src/ui/text_decorations.rs | 1 + helix-term/src/ui/text_decorations/blame.rs | 67 +++ helix-vcs/Cargo.toml | 3 +- helix-vcs/src/git.rs | 4 +- helix-vcs/src/git/blame.rs | 632 ++++++++++++++++++++ helix-vcs/src/git/test.rs | 26 +- helix-vcs/src/lib.rs | 5 +- helix-view/src/document.rs | 50 ++ helix-view/src/editor.rs | 43 ++ helix-view/src/events.rs | 3 +- helix-view/src/handlers.rs | 12 + 24 files changed, 1196 insertions(+), 29 deletions(-) create mode 100644 helix-stdx/src/time.rs create mode 100644 helix-term/src/handlers/blame.rs create mode 100644 helix-term/src/ui/text_decorations/blame.rs create mode 100644 helix-vcs/src/git/blame.rs diff --git a/Cargo.lock b/Cargo.lock index b0e34b3f5..6964b4bac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -211,6 +211,15 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "crossbeam-channel" +version = "0.5.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06ba6d68e24814cb8de6bb986db8222d3a027d15872cabc0d18817bc3c0e4471" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "crossbeam-deque" version = "0.8.5" @@ -509,6 +518,7 @@ checksum = "736f14636705f3a56ea52b553e67282519418d9a35bb1e90b3a9637a00296b68" dependencies = [ "gix-actor", "gix-attributes", + "gix-blame", "gix-command", "gix-commitgraph", "gix-config", @@ -591,6 +601,21 @@ dependencies = [ "thiserror 2.0.12", ] +[[package]] +name = "gix-blame" +version = "0.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "adc795e239a2347eb50ed18b8c529382dd8b62439c57277f79af3d8f8928a986" +dependencies = [ + "gix-diff", + "gix-hash", + "gix-object", + "gix-trace", + "gix-traverse", + "gix-worktree", + "thiserror 2.0.12", +] + [[package]] name = "gix-chunk" version = "0.4.11" @@ -739,12 +764,14 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8bfdd4838a8d42bd482c9f0cb526411d003ee94cc7c7b08afe5007329c71d554" dependencies = [ "crc32fast", + "crossbeam-channel", "flate2", "gix-hash", "gix-trace", "gix-utils", "libc", "once_cell", + "parking_lot", "prodash", "sha1_smol", "thiserror 2.0.12", @@ -1526,6 +1553,7 @@ dependencies = [ "gix", "helix-core", "helix-event", + "helix-stdx", "imara-diff", "log", "parking_lot", @@ -1823,15 +1851,15 @@ dependencies = [ [[package]] name = "jiff-tzdb" -version = "0.1.1" +version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91335e575850c5c4c673b9bd467b0e025f164ca59d0564f69d0c2ee0ffad4653" +checksum = "962e1dfe9b2d75a84536cf5bf5eaaa4319aa7906c7160134a22883ac316d5f31" [[package]] name = "jiff-tzdb-platform" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9835f0060a626fe59f160437bc725491a6af23133ea906500027d1bd2f8f4329" +checksum = "a63c62e404e7b92979d2792352d885a7f8f83fd1d0d31eea582d77b2ceca697e" dependencies = [ "jiff-tzdb", ] @@ -2087,15 +2115,15 @@ checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" [[package]] name = "portable-atomic" -version = "1.7.0" +version = "1.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da544ee218f0d287a911e9c99a39a8c9bc8fcad3cb8db5959940044ecfc67265" +checksum = "350e9b48cbc6b0e028b0473b114454c6316e57336ee184ceab6e53f72c178b3e" [[package]] name = "proc-macro2" -version = "1.0.86" +version = "1.0.94" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e719e8df665df0d1c8fbfd238015744736151d4445ec0836b8e628aae103b77" +checksum = "a31971752e70b8b2686d7e46ec17fb38dad4051d94024c88df49b667caea9c84" dependencies = [ "unicode-ident", ] @@ -2132,9 +2160,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.37" +version = "1.0.40" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5b9d34b8991d19d98081b46eacdd8eb58c6f2b201139f7c5f643cc155a633af" +checksum = "1885c039570dc00dcb4ff087a89e185fd56bae234ddc7f056a945bf36467248d" dependencies = [ "proc-macro2", ] @@ -2463,9 +2491,9 @@ checksum = "e9557cb6521e8d009c51a8666f09356f4b817ba9ba0981a305bd86aee47bd35c" [[package]] name = "syn" -version = "2.0.87" +version = "2.0.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25aa4ce346d03a6dcd68dd8b4010bcb74e54e62c90c573f394c46eae99aba32d" +checksum = "b09a44accad81e1ba1cd74a32461ba89dee89095ba17b32f5d03683b1b1fc2a0" dependencies = [ "proc-macro2", "quote", diff --git a/book/src/editor.md b/book/src/editor.md index 1e5c2a507..9958798eb 100644 --- a/book/src/editor.md +++ b/book/src/editor.md @@ -4,6 +4,7 @@ - [`[editor.clipboard-provider]` Section](#editorclipboard-provider-section) - [`[editor.statusline]` Section](#editorstatusline-section) - [`[editor.lsp]` Section](#editorlsp-section) +- [`[editor.inline-blame]` Section](#editorinlineblame-section) - [`[editor.cursor-shape]` Section](#editorcursor-shape-section) - [`[editor.file-picker]` Section](#editorfile-picker-section) - [`[editor.auto-pairs]` Section](#editorauto-pairs-section) @@ -161,6 +162,41 @@ The following statusline elements can be configured: [^2]: You may also have to activate them in the language server config for them to appear, not just in Helix. Inlay hints in Helix are still being improved on and may be a little bit laggy/janky under some circumstances. Please report any bugs you see so we can fix them! +### `[editor.inline-blame]` Section + +| Key | Description | Default | +| ------- | ------------------------------------------ | ------- | +| `behaviour` | Choose the behaviour of inline blame | `"disabled"` | +| `format` | The format in which to show the inline blame | `"{author}, {time-ago} • {message} • {commit}"` | + +The `behaviour` can be one of the following: +- `"visible"`: Inline blame is turned on. Virtual text will appear at the end of each non-empty line, showing information about the latest commit for that line. +- `"background"`: Inline blame is turned off, but the blame is still requested in the background when opening and reloading files. This will have zero impact on performance, but will use slightly more resources in the background. This allows blame for line (`space + B`) to be retrieved instantaneously with zero delay. +- `"disabled"`: Inline blame is turned off, with no requests happening in the background. When you run `space + B` for the first time in a file, it will load the blame for this file. You may have to wait a little bit for the blame to become available, depending on the size of your repository. After it becomes available, for this file `space + B` will retrieve the blame for any line in this file with zero delay. If the file is reloaded, the process will repeat as the blame is potentially out of date and needs to be refreshed. + +`inline-blame-format` allows customization of the blame message, and can be set to any string. Variables can be used like so: `{variable}`. These are the available variables: + +- `author`: The author of the commit +- `date`: When the commit was made +- `time-ago`: How long ago the commit was made +- `message`: The message of the commit, excluding the body +- `body`: The body of the commit +- `commit`: The short hex SHA1 hash of the commit +- `email`: The email of the author of the commit + +Any of the variables can potentially be empty. +In this case, the content before the variable will not be included in the string. +If the variable is at the beginning of the string, the content after the variable will not be included. + +Some examples, using the default value `format` value: + +- If `author` is empty: `"{time-ago} • {message} • {commit}"` +- If `time-ago` is empty: `"{author} • {message} • {commit}"` +- If `message` is empty: `"{author}, {time-ago} • {commit}"` +- If `commit` is empty: `"{author}, {time-ago} • {message}"` +- If `time-ago` and `message` is empty: `"{author} • {commit}"` +- If `author` and `message` is empty: `"{time-ago} • {commit}"` + ### `[editor.cursor-shape]` Section Defines the shape of cursor in each mode. diff --git a/book/src/generated/static-cmd.md b/book/src/generated/static-cmd.md index af7515b8e..3efba2a66 100644 --- a/book/src/generated/static-cmd.md +++ b/book/src/generated/static-cmd.md @@ -298,3 +298,4 @@ | `extend_to_word` | Extend to a two-character label | select: `` gw `` | | `goto_next_tabstop` | goto next snippet placeholder | | | `goto_prev_tabstop` | goto next snippet placeholder | | +| `blame_line` | Show blame for the current line | normal: `` B ``, select: `` B `` | diff --git a/book/src/keymap.md b/book/src/keymap.md index 2797eaee2..86ad51363 100644 --- a/book/src/keymap.md +++ b/book/src/keymap.md @@ -309,6 +309,7 @@ This layer is a kludge of mappings, mostly pickers. | `R` | Replace selections by clipboard contents | `replace_selections_with_clipboard` | | `/` | Global search in workspace folder | `global_search` | | `?` | Open command palette | `command_palette` | +| `B` | Show blame for the current line | `blame_line` | > 💡 Global search displays results in a fuzzy picker, use `Space + '` to bring it back up after opening a file. diff --git a/book/src/themes.md b/book/src/themes.md index 412d17efc..4201720e6 100644 --- a/book/src/themes.md +++ b/book/src/themes.md @@ -314,6 +314,7 @@ These scopes are used for theming the editor interface: | `ui.virtual.inlay-hint.type` | Style for inlay hints of kind `type` (language servers are not required to set a kind) | | `ui.virtual.wrap` | Soft-wrap indicator (see the [`editor.soft-wrap` config][editor-section]) | | `ui.virtual.jump-label` | Style for virtual jump labels | +| `ui.virtual.inline-blame` | Inline blame indicator (see the [`editor.inline-blame` config][editor-section]) | | `ui.menu` | Code and command completion menus | | `ui.menu.selected` | Selected autocomplete item | | `ui.menu.scroll` | `fg` sets thumb color, `bg` sets track color of scrollbar | diff --git a/helix-stdx/src/lib.rs b/helix-stdx/src/lib.rs index d09df587a..eeaba0d81 100644 --- a/helix-stdx/src/lib.rs +++ b/helix-stdx/src/lib.rs @@ -3,5 +3,6 @@ pub mod faccess; pub mod path; pub mod range; pub mod rope; +pub mod time; pub use range::Range; diff --git a/helix-stdx/src/time.rs b/helix-stdx/src/time.rs new file mode 100644 index 000000000..a7bb1a040 --- /dev/null +++ b/helix-stdx/src/time.rs @@ -0,0 +1,81 @@ +use std::time::{Instant, SystemTime}; + +use once_cell::sync::Lazy; + +const SECOND: i64 = 1; +const MINUTE: i64 = 60 * SECOND; +const HOUR: i64 = 60 * MINUTE; +const DAY: i64 = 24 * HOUR; +const MONTH: i64 = 30 * DAY; +const YEAR: i64 = 365 * DAY; + +/// Like `std::time::SystemTime::now()` but does not cause a syscall on every invocation. +/// +/// There is just one syscall at the start of the program, subsequent invocations are +/// much cheaper and use the monotonic clock instead of trigerring a syscall. +#[inline] +fn now() -> SystemTime { + static START_INSTANT: Lazy = Lazy::new(Instant::now); + static START_SYSTEM_TIME: Lazy = Lazy::new(SystemTime::now); + + *START_SYSTEM_TIME + START_INSTANT.elapsed() +} + +/// Formats a timestamp into a human-readable relative time string. +/// +/// # Arguments +/// +/// * `timestamp` - A point in history. Seconds since UNIX epoch (UTC) +/// * `timezone_offset` - Timezone offset in seconds +/// +/// # Returns +/// +/// A String representing the relative time (e.g., "4 years ago", "11 months from now") +#[inline] +pub fn format_relative_time(timestamp: i64, timezone_offset: i32) -> String { + let timestamp = timestamp + timezone_offset as i64; + let now = now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs() as i64 + + timezone_offset as i64; + + let time_passed = now - timestamp; + + let time_difference = time_passed.abs(); + + let (value, unit) = if time_difference >= YEAR { + let years = time_difference / YEAR; + (years, if years == 1 { "year" } else { "years" }) + } else if time_difference >= MONTH { + let months = time_difference / MONTH; + (months, if months == 1 { "month" } else { "months" }) + } else if time_difference >= DAY { + let days = time_difference / DAY; + (days, if days == 1 { "day" } else { "days" }) + } else if time_difference >= HOUR { + let hours = time_difference / HOUR; + (hours, if hours == 1 { "hour" } else { "hours" }) + } else if time_difference >= MINUTE { + let minutes = time_difference / MINUTE; + (minutes, if minutes == 1 { "minute" } else { "minutes" }) + } else { + let seconds = time_difference / SECOND; + (seconds, if seconds == 1 { "second" } else { "seconds" }) + }; + let value = value.to_string(); + + let label = if time_passed.is_positive() { + "ago" + } else { + "from now" + }; + + let mut relative_time = String::with_capacity(value.len() + 1 + unit.len() + 1 + label.len()); + relative_time.push_str(&value); + relative_time.push(' '); + relative_time.push_str(unit); + relative_time.push(' '); + relative_time.push_str(label); + relative_time +} diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 2e15dcdcc..c1d1a46af 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -10,6 +10,7 @@ use helix_stdx::{ rope::{self, RopeSliceExt}, }; use helix_vcs::{FileChange, Hunk}; +use helix_view::document::LineBlameError; pub use lsp::*; use tui::{ text::{Span, Spans}, @@ -596,6 +597,7 @@ impl MappableCommand { extend_to_word, "Extend to a two-character label", goto_next_tabstop, "goto next snippet placeholder", goto_prev_tabstop, "goto next snippet placeholder", + blame_line, "Show blame for the current line", ); } @@ -3470,6 +3472,54 @@ fn insert_at_line_start(cx: &mut Context) { insert_with_indent(cx, IndentFallbackPos::LineStart); } +pub(crate) fn blame_line_impl(editor: &mut Editor, doc: DocumentId, cursor_line: u32) { + let inline_blame_config = &editor.config().inline_blame; + let Some(doc) = editor.document(doc) else { + return; + }; + let line_blame = match doc.line_blame(cursor_line, &inline_blame_config.format) { + result + if (result.is_ok() && doc.is_blame_potentially_out_of_date) + || matches!(result, Err(LineBlameError::NotReadyYet) if inline_blame_config.behaviour + == helix_view::editor::InlineBlameBehaviour::Disabled) => + { + if let Some(path) = doc.path() { + let tx = editor.handlers.blame.clone(); + helix_event::send_blocking( + &tx, + helix_view::handlers::BlameEvent { + path: path.to_path_buf(), + doc_id: doc.id(), + line: Some(cursor_line), + }, + ); + editor.set_status(format!("Requested blame for {}...", path.display())); + let doc = doc_mut!(editor); + doc.is_blame_potentially_out_of_date = false; + } else { + editor.set_error("Could not get path of document"); + }; + return; + } + Ok(line_blame) => line_blame, + Err(err @ (LineBlameError::NotCommittedYet | LineBlameError::NotReadyYet)) => { + editor.set_status(err.to_string()); + return; + } + Err(err @ LineBlameError::NoFileBlame(_, _)) => { + editor.set_error(err.to_string()); + return; + } + }; + + editor.set_status(line_blame); +} + +fn blame_line(cx: &mut Context) { + let (view, doc) = current_ref!(cx.editor); + blame_line_impl(cx.editor, doc.id(), doc.cursor_line(view.id) as u32); +} + // `A` inserts at the end of each line with a selection. // If the line is empty, automatically indent. fn insert_at_line_end(cx: &mut Context) { diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index e1c09a04d..15792bc66 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -14,6 +14,7 @@ use helix_stdx::path::home_dir; use helix_view::document::{read_to_string, DEFAULT_LANGUAGE_NAME}; use helix_view::editor::{CloseError, ConfigEvent}; use helix_view::expansion; +use helix_view::handlers::BlameEvent; use serde_json::Value; use ui::completers::{self, Completer}; @@ -1326,16 +1327,33 @@ fn reload(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> anyh } let scrolloff = cx.editor.config().scrolloff; + let inline_blame_behaviour = cx.editor.config().inline_blame.behaviour; let (view, doc) = current!(cx.editor); doc.reload(view, &cx.editor.diff_providers).map(|_| { view.ensure_cursor_in_view(doc, scrolloff); })?; + let doc_id = doc.id(); if let Some(path) = doc.path() { cx.editor .language_servers .file_event_handler .file_changed(path.clone()); } + + if doc.should_request_full_file_blame(inline_blame_behaviour) { + if let Some(path) = doc.path() { + helix_event::send_blocking( + &cx.editor.handlers.blame, + BlameEvent { + path: path.to_path_buf(), + doc_id, + line: None, + }, + ); + } + } + doc.is_blame_potentially_out_of_date = true; + Ok(()) } @@ -1362,6 +1380,8 @@ fn reload_all(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> }) .collect(); + let blame_behaviour = cx.editor.config().inline_blame.behaviour; + for (doc_id, view_ids) in docs_view_ids { let doc = doc_mut!(cx.editor, &doc_id); @@ -1389,6 +1409,20 @@ fn reload_all(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> view.ensure_cursor_in_view(doc, scrolloff); } } + + if doc.should_request_full_file_blame(blame_behaviour) { + if let Some(path) = doc.path() { + helix_event::send_blocking( + &cx.editor.handlers.blame, + BlameEvent { + path: path.to_path_buf(), + doc_id, + line: None, + }, + ); + } + } + doc.is_blame_potentially_out_of_date = true; } Ok(()) diff --git a/helix-term/src/handlers.rs b/helix-term/src/handlers.rs index c7d71526c..4686e6e71 100644 --- a/helix-term/src/handlers.rs +++ b/helix-term/src/handlers.rs @@ -10,9 +10,11 @@ use crate::handlers::signature_help::SignatureHelpHandler; pub use helix_view::handlers::Handlers; +use self::blame::BlameHandler; use self::document_colors::DocumentColorsHandler; mod auto_save; +pub mod blame; pub mod completion; mod diagnostics; mod document_colors; @@ -26,12 +28,14 @@ pub fn setup(config: Arc>) -> Handlers { let signature_hints = SignatureHelpHandler::new().spawn(); let auto_save = AutoSaveHandler::new().spawn(); let document_colors = DocumentColorsHandler::default().spawn(); + let blame = BlameHandler::default().spawn(); let handlers = Handlers { completions: helix_view::handlers::completion::CompletionHandler::new(event_tx), signature_hints, auto_save, document_colors, + blame, }; helix_view::handlers::register_hooks(&handlers); @@ -41,5 +45,6 @@ pub fn setup(config: Arc>) -> Handlers { diagnostics::register_hooks(&handlers); snippet::register_hooks(&handlers); document_colors::register_hooks(&handlers); + blame::register_hooks(&handlers); handlers } diff --git a/helix-term/src/handlers/blame.rs b/helix-term/src/handlers/blame.rs new file mode 100644 index 000000000..6a35859f8 --- /dev/null +++ b/helix-term/src/handlers/blame.rs @@ -0,0 +1,94 @@ +use std::time::Duration; + +use helix_event::register_hook; +use helix_vcs::FileBlame; +use helix_view::{ + editor::InlineBlameBehaviour, + events::DocumentDidOpen, + handlers::{BlameEvent, Handlers}, + DocumentId, +}; +use tokio::{sync::oneshot, time::Instant}; + +use crate::job; + +#[derive(Default)] +pub struct BlameHandler { + worker: Option>>, + doc_id: DocumentId, + show_blame_for_line_in_statusline: Option, +} + +impl helix_event::AsyncHook for BlameHandler { + type Event = BlameEvent; + + fn handle_event( + &mut self, + event: Self::Event, + _timeout: Option, + ) -> Option { + if let Some(worker) = &mut self.worker { + if worker.try_recv().is_ok() { + self.finish_debounce(); + return None; + } + } + + self.doc_id = event.doc_id; + self.show_blame_for_line_in_statusline = event.line; + let (tx, rx) = oneshot::channel(); + + tokio::spawn(async move { + let result = FileBlame::try_new(event.path); + let _ = tx.send(result); + }); + + self.worker = Some(rx); + + Some(Instant::now() + Duration::from_millis(50)) + } + + fn finish_debounce(&mut self) { + let doc_id = self.doc_id; + let line_blame = self.show_blame_for_line_in_statusline; + if let Some(worker) = self.worker.take() { + tokio::spawn(async move { + let Ok(result) = worker.await else { + return; + }; + + job::dispatch(move |editor, _| { + let Some(doc) = editor.document_mut(doc_id) else { + return; + }; + doc.file_blame = Some(result); + if editor.config().inline_blame.behaviour == InlineBlameBehaviour::Disabled { + if let Some(line) = line_blame { + crate::commands::blame_line_impl(editor, doc_id, line); + } else { + editor.set_status("Blame for this file is now available") + } + } + }) + .await; + }); + } + } +} + +pub(super) fn register_hooks(handlers: &Handlers) { + let tx = handlers.blame.clone(); + register_hook!(move |event: &mut DocumentDidOpen<'_>| { + if event.editor.config().inline_blame.behaviour != InlineBlameBehaviour::Disabled { + helix_event::send_blocking( + &tx, + BlameEvent { + path: event.path.to_path_buf(), + doc_id: event.doc, + line: None, + }, + ); + } + Ok(()) + }); +} diff --git a/helix-term/src/keymap/default.rs b/helix-term/src/keymap/default.rs index e160b2246..63d09ffbd 100644 --- a/helix-term/src/keymap/default.rs +++ b/helix-term/src/keymap/default.rs @@ -289,6 +289,7 @@ pub fn default() -> HashMap { "C" => toggle_block_comments, "A-c" => toggle_line_comments, "?" => command_palette, + "B" => blame_line, }, "z" => { "View" "z" | "c" => align_view_center, diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 6be565747..739f109c8 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -25,7 +25,7 @@ use helix_core::{ use helix_view::{ annotations::diagnostics::DiagnosticFilter, document::{Mode, SCRATCH_BUFFER_NAME}, - editor::{CompleteAction, CursorShapeConfig}, + editor::{CompleteAction, CursorShapeConfig, InlineBlameBehaviour}, graphics::{Color, CursorKind, Modifier, Rect, Style}, input::{KeyEvent, MouseButton, MouseEvent, MouseEventKind}, keyboard::{KeyCode, KeyModifiers}, @@ -35,6 +35,8 @@ use std::{mem::take, num::NonZeroUsize, path::PathBuf, rc::Rc}; use tui::{buffer::Buffer as Surface, text::Span}; +use super::text_decorations::blame::InlineBlame; + pub struct EditorView { pub keymaps: Keymaps, on_next_key: Option<(OnKeyCallback, OnKeyCallbackKind)>, @@ -201,6 +203,24 @@ impl EditorView { inline_diagnostic_config, config.end_of_line_diagnostics, )); + + if config.inline_blame.behaviour == InlineBlameBehaviour::Visible { + let cursor_line_idx = doc.cursor_line(view.id); + + // do not render inline blame for empty lines to reduce visual noise + if doc.text().line(cursor_line_idx) != doc.line_ending.as_str() { + if let Ok(line_blame) = + doc.line_blame(cursor_line_idx as u32, &config.inline_blame.format) + { + decorations.add_decoration(InlineBlame::new( + theme, + cursor_line_idx, + line_blame, + )); + }; + } + } + render_document( surface, inner, diff --git a/helix-term/src/ui/text_decorations.rs b/helix-term/src/ui/text_decorations.rs index 931ea4311..f9d757ad8 100644 --- a/helix-term/src/ui/text_decorations.rs +++ b/helix-term/src/ui/text_decorations.rs @@ -8,6 +8,7 @@ use crate::ui::document::{LinePos, TextRenderer}; pub use diagnostics::InlineDiagnostics; +pub mod blame; mod diagnostics; /// Decorations are the primary mechanism for extending the text rendering. diff --git a/helix-term/src/ui/text_decorations/blame.rs b/helix-term/src/ui/text_decorations/blame.rs new file mode 100644 index 000000000..49b0a31d7 --- /dev/null +++ b/helix-term/src/ui/text_decorations/blame.rs @@ -0,0 +1,67 @@ +use helix_core::Position; + +use helix_view::theme::Style; +use helix_view::Theme; + +use crate::ui::document::{LinePos, TextRenderer}; +use crate::ui::text_decorations::Decoration; + +pub struct InlineBlame { + message: String, + cursor: usize, + style: Style, +} + +impl InlineBlame { + pub fn new(theme: &Theme, cursor: usize, message: String) -> Self { + InlineBlame { + style: theme.get("ui.virtual.inline-blame"), + message, + cursor, + } + } +} + +impl Decoration for InlineBlame { + fn render_virt_lines( + &mut self, + renderer: &mut TextRenderer, + pos: LinePos, + virt_off: Position, + ) -> Position { + // do not draw inline blame for lines other than the cursor line + if self.cursor != pos.doc_line { + return Position::new(0, 0); + } + + // where the line in the document ends + let end_of_line = virt_off.col as u16; + // length of line in the document + // draw the git blame 6 spaces after the end of the line + let start_drawing_at = end_of_line + 6; + + let amount_of_characters_drawn = renderer + .column_in_bounds(start_drawing_at as usize, 1) + .then(|| { + // the column where we stop drawing the blame + let stopped_drawing_at = renderer + .set_string_truncated( + renderer.viewport.x + start_drawing_at, + pos.visual_line, + &self.message, + renderer.viewport.width.saturating_sub(start_drawing_at) as usize, + |_| self.style, + true, + false, + ) + .0; + + let line_length = end_of_line - renderer.offset.col as u16; + + stopped_drawing_at - line_length + }) + .unwrap_or_default(); + + Position::new(0, amount_of_characters_drawn as usize) + } +} diff --git a/helix-vcs/Cargo.toml b/helix-vcs/Cargo.toml index c08122421..6624cb94c 100644 --- a/helix-vcs/Cargo.toml +++ b/helix-vcs/Cargo.toml @@ -14,12 +14,13 @@ homepage.workspace = true [dependencies] helix-core = { path = "../helix-core" } helix-event = { path = "../helix-event" } +helix-stdx = { path = "../helix-stdx" } tokio = { version = "1", features = ["rt", "rt-multi-thread", "time", "sync", "parking_lot", "macros"] } parking_lot.workspace = true arc-swap = { version = "1.7.1" } -gix = { version = "0.70.0", features = ["attributes", "status"], default-features = false, optional = true } +gix = { version = "0.70.0", features = ["attributes", "status", "blame", "parallel"], default-features = false, optional = true } imara-diff = "0.1.8" anyhow = "1" diff --git a/helix-vcs/src/git.rs b/helix-vcs/src/git.rs index b8ddd79fa..7e76ab7cf 100644 --- a/helix-vcs/src/git.rs +++ b/helix-vcs/src/git.rs @@ -1,11 +1,11 @@ use anyhow::{bail, Context, Result}; use arc_swap::ArcSwap; +use gix::bstr::ByteSlice as _; use gix::filter::plumbing::driver::apply::Delay; 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; @@ -22,6 +22,8 @@ use crate::FileChange; #[cfg(test)] mod test; +pub mod blame; + #[inline] fn get_repo_dir(file: &Path) -> Result<&Path> { file.parent().context("file has no parent directory") diff --git a/helix-vcs/src/git/blame.rs b/helix-vcs/src/git/blame.rs new file mode 100644 index 000000000..70e630bcc --- /dev/null +++ b/helix-vcs/src/git/blame.rs @@ -0,0 +1,632 @@ +use anyhow::Context as _; +use anyhow::Result; +use std::collections::HashMap; +use std::path::PathBuf; + +use crate::DiffHandle; + +use super::{get_repo_dir, open_repo}; + +/// Stores information about the blame for a file +#[derive(Debug)] +pub struct FileBlame { + /// A map from line numbers to commit IDs + blame: HashMap, + /// The owning repository for this file's `ObjectId`s + repo: gix::ThreadSafeRepository, +} + +impl FileBlame { + /// Get the blame information corresponing to a line in file and diff for that line + /// + /// returns `None` if the line is part of an insertion, as the blame for that line would not + /// be meaningful + #[inline] + pub fn blame_for_line(&self, line: u32, diff_handle: Option<&DiffHandle>) -> Option { + let (inserted_lines, removed_lines) = diff_handle.map_or( + // in theory there can be situations where we don't have the diff for a file + // but we have the blame. In this case, we can just act like there is no diff + Some((0, 0)), + |diff_handle| { + // Compute the amount of lines inserted and deleted before the `line` + // This information is needed to accurately transform the state of the + // file in the file system into what gix::blame knows about (gix::blame only + // knows about commit history, it does not know about uncommitted changes) + diff_handle + .load() + .hunks_intersecting_line_ranges(std::iter::once((0, line as usize))) + .try_fold( + (0, 0), + |(total_inserted_lines, total_deleted_lines), hunk| { + // check if the line intersects the hunk's `after` (which represents + // inserted lines) + (hunk.after.start > line || hunk.after.end <= line).then_some(( + total_inserted_lines + (hunk.after.end - hunk.after.start), + total_deleted_lines + (hunk.before.end - hunk.before.start), + )) + }, + ) + }, + )?; + + Some(self.blame_for_line_inserted_removed(line, inserted_lines, removed_lines)) + } + + // this is a separate function for use in Tests + #[inline] + fn blame_for_line_inserted_removed( + &self, + line: u32, + inserted_lines: u32, + removed_lines: u32, + ) -> LineBlame { + // Because gix_blame doesn't care about stuff that is not commited, we have to "normalize" the + // line number to account for uncommited code. + // + // You'll notice that blame_line can be 0 when, for instance we have: + // - removed 0 lines + // - added 10 lines + // - cursor_line is 8 + // + // So when our cursor is on the 10th added line or earlier, blame_line will be 0. This means + // the blame will be incorrect. But that's fine, because when the cursor_line is on some hunk, + // we can show to the user nothing at all. This is detected in the editor + let blame_line = line.saturating_sub(inserted_lines) + removed_lines; + let repo = self.repo.to_thread_local(); + + let commit = self + .blame + .get(&blame_line) + .and_then(|obj| repo.find_commit(*obj).ok()); + let message = commit.as_ref().and_then(|c| c.message().ok()); + let author = commit.as_ref().and_then(|c| c.author().ok()); + + LineBlame { + commit_hash: commit + .as_ref() + .and_then(|c| c.short_id().map(|id| id.to_string()).ok()), + author_name: author.map(|a| a.name.to_string()), + author_email: author.map(|a| a.email.to_string()), + commit_date: author.map(|a| a.time.format(gix::date::time::format::SHORT)), + commit_message: message.as_ref().map(|msg| msg.title.to_string()), + commit_body: message + .as_ref() + .and_then(|msg| msg.body.map(|body| body.to_string())), + time_ago: author + .map(|a| helix_stdx::time::format_relative_time(a.time.seconds, a.time.offset)), + } + } + + /// Compute blame for this file + pub fn try_new(file: PathBuf) -> Result { + let thread_safe_repo = + open_repo(get_repo_dir(&file)?).context("Failed to open git repo")?; + let repo = thread_safe_repo.to_thread_local(); + let head = repo.head()?.peel_to_commit_in_place()?.id; + + // TODO: this iterator has a performane issue for large repos + // It was replaced in a new (yet unreleased) version of `gix`. + // + // Update to the new version once it releases. + // + // More info: https://github.com/helix-editor/helix/pull/13133#discussion_r2008611830 + let traverse = gix::traverse::commit::topo::Builder::from_iters( + &repo.objects, + [head], + None::>, + ) + .build()?; + + let mut resource_cache = repo.diff_resource_cache_for_tree_diff()?; + let file_blame = gix::blame::file( + &repo.objects, + traverse.into_iter(), + &mut resource_cache, + // bstr always uses unix separators + &gix::path::to_unix_separators_on_windows(gix::path::try_into_bstr( + file.strip_prefix( + repo.path() + .parent() + .context("Could not get the parent path of the repo")?, + )?, + )?), + None, + )? + .entries; + + Ok(Self { + blame: file_blame + .into_iter() + .flat_map(|blame| { + (blame.start_in_blamed_file..blame.start_in_blamed_file + blame.len.get()) + .map(move |i| (i, blame.commit_id)) + }) + .collect(), + repo: thread_safe_repo, + }) + } +} + +#[derive(Clone, PartialEq, PartialOrd, Ord, Eq, Debug)] +pub struct LineBlame { + commit_hash: Option, + author_name: Option, + author_email: Option, + commit_date: Option, + commit_message: Option, + commit_body: Option, + time_ago: Option, +} + +impl LineBlame { + /// Longest variable is: `time-ago` (and `message`) + const LONGEST_VARIABLE_LENGTH: usize = 7; + /// # Returns + /// + /// None => Invalid variable + /// Some(None) => Valid variable, but is empty + #[inline] + fn get_variable(&self, var: &str) -> Option> { + Some( + // if adding new variables, update `Self::LONGEST_VARIABLE_LENGTH` + match var { + "commit" => &self.commit_hash, + "author" => &self.author_name, + "date" => &self.commit_date, + "message" => &self.commit_message, + "email" => &self.author_email, + "body" => &self.commit_body, + "time-ago" => &self.time_ago, + _ => return None, + } + .as_deref(), + ) + } + + /// Parse the user's blame format + #[inline] + pub fn parse_format(&self, format: &str) -> String { + let mut line_blame = String::new(); + let mut content_before_variable = String::with_capacity(format.len()); + + let mut chars = format.char_indices().peekable(); + // in all cases, when any of the variables is empty we exclude the content before the variable + // However, if the variable is the first and it is empty - then exclude the content after the variable + let mut exclude_content_after_variable = false; + while let Some((ch_idx, ch)) = chars.next() { + if ch == '{' { + let mut variable = String::with_capacity(Self::LONGEST_VARIABLE_LENGTH); + // eat all characters until the end + while let Some((_, ch)) = chars.next_if(|(_, ch)| *ch != '}') { + variable.push(ch); + } + // eat the '}' if it was found + let has_closing = chars.next().is_some(); + + #[derive(PartialEq, Eq, PartialOrd, Ord)] + enum Variable<'a> { + Valid(&'a str), + Invalid(&'a str), + Empty, + } + + let variable_value = self.get_variable(&variable).map_or_else( + || { + // Invalid variable. So just add whatever we parsed before. + // The length of the variable, including opening and optionally + // closing curly braces + let variable_len = 1 + variable.len() + has_closing as usize; + + Variable::Invalid(&format[ch_idx..ch_idx + variable_len]) + }, + |s| s.map(Variable::Valid).unwrap_or(Variable::Empty), + ); + + match variable_value { + Variable::Invalid(value) | Variable::Valid(value) => { + if exclude_content_after_variable { + // don't push anything. + exclude_content_after_variable = false; + } else { + line_blame.push_str(&content_before_variable); + } + line_blame.push_str(value); + } + Variable::Empty => { + if line_blame.is_empty() { + // exclude content AFTER this variable (at next iteration of the loop, + // we'll exclude the content before a valid variable) + exclude_content_after_variable = true; + } else { + // exclude content BEFORE this variable + // also just don't add anything. + } + } + } + + // we've consumed the content before the variable so just get rid of it and + // make space for new + content_before_variable.drain(..); + } else { + content_before_variable.push(ch); + } + } + + line_blame + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::git::test::create_commit_with_message; + use crate::git::test::empty_git_repo; + use std::fs::File; + + /// describes how a line was modified + #[derive(PartialEq, PartialOrd, Ord, Eq)] + enum LineDiff { + /// this line is added + Insert, + /// this line is deleted + Delete, + /// no changes for this line + None, + } + + /// checks if the first argument is `no_commit` or not + macro_rules! no_commit_flag { + (no_commit, $commit_msg:literal) => { + false + }; + (, $commit_msg:literal) => { + true + }; + ($any:tt, $commit_msg:literal) => { + compile_error!(concat!( + "expected `no_commit` or nothing for commit ", + $commit_msg + )) + }; + } + + /// checks if the first argument is `insert` or `delete` + macro_rules! line_diff_flag { + (insert, $commit_msg:literal, $line:expr) => { + LineDiff::Insert + }; + (delete, $commit_msg:literal, $line:expr) => { + LineDiff::Delete + }; + (, $commit_msg:literal, $line:expr) => { + LineDiff::None + }; + ($any:tt, $commit_msg:literal, $line:expr) => { + compile_error!(concat!( + "expected `insert`, `delete` or nothing for commit ", + $commit_msg, + " line ", + $line + )) + }; + } + + /// This macro exists because we can't pass a `match` statement into `concat!` + /// we would like to exclude any lines that are `delete` + macro_rules! line_diff_flag_str { + (insert, $commit_msg:literal, $line:expr) => { + concat!($line, newline_literal!()) + }; + (delete, $commit_msg:literal, $line:expr) => { + "" + }; + (, $commit_msg:literal, $line:expr) => { + concat!($line, newline_literal!()) + }; + ($any:tt, $commit_msg:literal, $line:expr) => { + compile_error!(concat!( + "expected `insert`, `delete` or nothing for commit ", + $commit_msg, + " line ", + $line + )) + }; + } + + #[cfg(windows)] + macro_rules! newline_literal { + () => { + "\r\n" + }; + } + #[cfg(not(windows))] + macro_rules! newline_literal { + () => { + "\n" + }; + } + + /// Helper macro to create a history of the same file being modified. + macro_rules! assert_line_blame_progress { + ( + $( + // a unique identifier for the commit, other commits must not use this + // If `no_commit` option is used, use the identifier of the previous commit + $commit_msg:literal + // must be `no_commit` if exists. + // If exists, this block won't be committed + $($no_commit:ident)? => + $( + // contents of a line in the file + $line:literal + // what commit identifier we are expecting for this line + $($expected:literal)? + // must be `insert` or `delete` if exists + // if exists, must be used with `no_commit` + // - `insert`: this line is added + // - `delete`: this line is deleted + $($line_diff:ident)? + ),+ + );+ + $(;)? + ) => {{ + use std::fs::OpenOptions; + use std::io::Write; + + let repo = empty_git_repo(); + let file = repo.path().join("file.txt"); + File::create(&file).expect("could not create file"); + + $( + let file_content = concat!( + $( + line_diff_flag_str!($($line_diff)?, $commit_msg, $line), + )* + ); + eprintln!("at commit {}:\n\n{file_content}", stringify!($commit_msg)); + + let mut f = OpenOptions::new() + .write(true) + .truncate(true) + .open(&file) + .unwrap(); + + f.write_all(file_content.as_bytes()).unwrap(); + + let should_commit = no_commit_flag!($($no_commit)?, $commit_msg); + if should_commit { + create_commit_with_message(repo.path(), true, stringify!($commit_msg)); + } + + let mut line_number = 0; + let mut added_lines = 0; + let mut removed_lines = 0; + + $( + let line_diff_flag = line_diff_flag!($($line_diff)?, $commit_msg, $line); + #[allow(unused_assignments)] + match line_diff_flag { + LineDiff::Insert => added_lines += 1, + LineDiff::Delete => removed_lines += 1, + LineDiff::None => () + } + // completely skip lines that are marked as `delete` + if line_diff_flag != LineDiff::Delete { + // if there is no $expected, then we don't care what blame_line returns + // because we won't show it to the user. + $( + let blame_result = + FileBlame::try_new(file.clone()) + .unwrap() + .blame_for_line_inserted_removed(line_number, added_lines, removed_lines) + .commit_message; + + assert_eq!( + blame_result, + Some(concat!(stringify!($expected), newline_literal!()).to_owned()), + "Blame mismatch\nat commit: {}\nat line: {}\nline contents: {}\nexpected commit: {}\nbut got commit: {}", + $commit_msg, + line_number, + file_content + .lines() + .nth(line_number.try_into().unwrap()) + .unwrap(), + stringify!($expected), + blame_result + .as_ref() + .map(|blame| blame.trim_end()) + .unwrap_or("") + ); + )? + #[allow(unused_assignments)] + { + line_number += 1; + } + } + )* + )* + }}; + } + + // For some reasons the CI is failing on windows with the message "Commits not found". + // The created temporary repository has no commits... But this is not an issue on unix. + // There is nothing platform-specific in this implementation. This is a problem only + // for tests on Windows. + // As such it should be fine to disable this test in Windows. + // As long as these tests pass on other platforms, on Windows it will work too. + #[cfg(not(windows))] + #[test] + pub fn blamed_lines() { + assert_line_blame_progress! { + // initialize + 1 => + "fn main() {" 1, + "" 1, + "}" 1; + // modifying a line works + 2 => + "fn main() {" 1, + " one" 2, + "}" 1; + // inserting a line works + 3 => + "fn main() {" 1, + " one" 2, + " two" 3, + "}" 1; + // deleting a line works + 4 => + "fn main() {" 1, + " two" 3, + "}" 1; + // when a line is inserted in-between the blame order is preserved + 4 no_commit => + "fn main() {" 1, + " hello world" insert, + " two" 3, + "}" 1; + // Having a bunch of random lines interspersed should not change which lines + // have blame for which commits + 4 no_commit => + " six" insert, + " three" insert, + "fn main() {" 1, + " five" insert, + " four" insert, + " two" 3, + " five" insert, + " four" insert, + "}" 1, + " five" insert, + " four" insert; + // committing all of those insertions should recognize that they are + // from the current commit, while still keeping the information about + // previous commits + 5 => + " six" 5, + " three" 5, + "fn main() {" 1, + " five" 5, + " four" 5, + " two" 3, + " five" 5, + " four" 5, + "}" 1, + " five" 5, + " four" 5; + // several lines deleted + 5 no_commit => + " six" 5, + " three" 5, + "fn main() {" delete, + " five" delete, + " four" delete, + " two" delete, + " five" delete, + " four" 5, + "}" 1, + " five" 5, + " four" 5; + // committing the deleted changes + 6 => + " six" 5, + " three" 5, + " four" 5, + "}" 1, + " five" 5, + " four" 5; + // mixing inserts with deletes + 6 no_commit => + " six" delete, + " 2" insert, + " three" delete, + " four" 5, + " 1" insert, + "}" 1, + "]" insert, + " five" delete, + " four" 5; + // committing inserts and deletes + 7 => + " 2" 7, + " four" 5, + " 1" 7, + "}" 1, + "]" 7, + " four" 5; + }; + } + + fn bob() -> LineBlame { + LineBlame { + commit_hash: Some("f14ab1cf".to_owned()), + author_name: Some("Bob TheBuilder".to_owned()), + author_email: Some("bob@bob.com".to_owned()), + commit_date: Some("2028-01-10".to_owned()), + commit_message: Some("feat!: extend house".to_owned()), + commit_body: Some("BREAKING CHANGE: Removed door".to_owned()), + time_ago: None, + } + } + + #[test] + pub fn inline_blame_format_parser() { + let format = "{author}, {date} • {message} • {commit}"; + + assert_eq!( + bob().parse_format(format), + "Bob TheBuilder, 2028-01-10 • feat!: extend house • f14ab1cf".to_owned() + ); + assert_eq!( + LineBlame { + author_name: None, + ..bob() + } + .parse_format(format), + "2028-01-10 • feat!: extend house • f14ab1cf".to_owned() + ); + assert_eq!( + LineBlame { + commit_date: None, + ..bob() + } + .parse_format(format), + "Bob TheBuilder • feat!: extend house • f14ab1cf".to_owned() + ); + assert_eq!( + LineBlame { + commit_message: None, + author_email: None, + ..bob() + } + .parse_format(format), + "Bob TheBuilder, 2028-01-10 • f14ab1cf".to_owned() + ); + assert_eq!( + LineBlame { + commit_hash: None, + ..bob() + } + .parse_format(format), + "Bob TheBuilder, 2028-01-10 • feat!: extend house".to_owned() + ); + assert_eq!( + LineBlame { + commit_date: None, + author_name: None, + ..bob() + } + .parse_format(format), + "feat!: extend house • f14ab1cf".to_owned() + ); + assert_eq!( + LineBlame { + author_name: None, + commit_message: None, + ..bob() + } + .parse_format(format), + "2028-01-10 • f14ab1cf".to_owned() + ); + } +} diff --git a/helix-vcs/src/git/test.rs b/helix-vcs/src/git/test.rs index 164040f50..c758c80b0 100644 --- a/helix-vcs/src/git/test.rs +++ b/helix-vcs/src/git/test.rs @@ -4,11 +4,11 @@ use tempfile::TempDir; use crate::git; -fn exec_git_cmd(args: &str, git_dir: &Path) { +pub fn exec_git_cmd(args: &[&str], git_dir: &Path) { let res = Command::new("git") .arg("-C") .arg(git_dir) // execute the git command in this directory - .args(args.split_whitespace()) + .args(args) .env_remove("GIT_DIR") .env_remove("GIT_ASKPASS") .env_remove("SSH_ASKPASS") @@ -25,26 +25,30 @@ fn exec_git_cmd(args: &str, git_dir: &Path) { .env("GIT_CONFIG_KEY_1", "init.defaultBranch") .env("GIT_CONFIG_VALUE_1", "main") .output() - .unwrap_or_else(|_| panic!("`git {args}` failed")); + .unwrap_or_else(|_| panic!("`git {args:?}` failed")); if !res.status.success() { println!("{}", String::from_utf8_lossy(&res.stdout)); eprintln!("{}", String::from_utf8_lossy(&res.stderr)); - panic!("`git {args}` failed (see output above)") + panic!("`git {args:?}` failed (see output above)") } } -fn create_commit(repo: &Path, add_modified: bool) { +pub fn create_commit(repo: &Path, add_modified: bool) { + create_commit_with_message(repo, add_modified, "commit") +} + +pub fn create_commit_with_message(repo: &Path, add_modified: bool, message: &str) { if add_modified { - exec_git_cmd("add -A", repo); + exec_git_cmd(&["add", "-A"], repo); } - exec_git_cmd("commit -m message", repo); + exec_git_cmd(&["commit", "-m", message], repo); } -fn empty_git_repo() -> TempDir { +pub fn empty_git_repo() -> TempDir { let tmp = tempfile::tempdir().expect("create temp dir for git testing"); - exec_git_cmd("init", tmp.path()); - exec_git_cmd("config user.email test@helix.org", tmp.path()); - exec_git_cmd("config user.name helix-test", tmp.path()); + exec_git_cmd(&["init"], tmp.path()); + exec_git_cmd(&["config", "user.email", "test@helix.org"], tmp.path()); + exec_git_cmd(&["config", "user.name", "helix-test"], tmp.path()); tmp } diff --git a/helix-vcs/src/lib.rs b/helix-vcs/src/lib.rs index 539be779a..33078147e 100644 --- a/helix-vcs/src/lib.rs +++ b/helix-vcs/src/lib.rs @@ -7,6 +7,7 @@ use std::{ #[cfg(feature = "git")] mod git; +pub use git::blame::FileBlame; mod diff; @@ -16,7 +17,7 @@ mod status; pub use status::FileChange; -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct DiffProviderRegistry { providers: Vec, } @@ -84,7 +85,7 @@ impl Default for DiffProviderRegistry { /// cloning [DiffProviderRegistry] as `Clone` cannot be used in trait objects. /// /// `Copy` is simply to ensure the `clone()` call is the simplest it can be. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] pub enum DiffProvider { #[cfg(feature = "git")] Git, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 41c9ee1ef..2c2e57fda 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -42,6 +42,7 @@ use helix_core::{ ChangeSet, Diagnostic, LineEnding, Range, Rope, RopeBuilder, Selection, Syntax, Transaction, }; +use crate::editor::InlineBlameBehaviour; use crate::{ editor::Config, events::{DocumentDidChange, SelectionDidChange}, @@ -196,6 +197,10 @@ pub struct Document { diff_handle: Option, version_control_head: Option>>>, + /// Contains blame information for each line in the file + /// We store the Result because when we access the blame directly we want to log the error + /// But if it is in the background we are just going to ignore the error + pub file_blame: Option>, // when document was used for most-recent-used buffer picker pub focused_at: std::time::Instant, @@ -207,6 +212,8 @@ pub struct Document { // NOTE: ideally this would live on the handler for color swatches. This is blocked on a // large refactor that would make `&mut Editor` available on the `DocumentDidChange` event. pub color_swatch_controller: TaskController, + // when fetching blame on-demand, if this field is `true` we request the blame for this document again + pub is_blame_potentially_out_of_date: bool, } #[derive(Debug, Clone, Default)] @@ -283,6 +290,16 @@ pub struct DocumentInlayHintsId { pub last_line: usize, } +#[derive(Debug, thiserror::Error)] +pub enum LineBlameError<'a> { + #[error("Not committed yet")] + NotCommittedYet, + #[error("Unable to get blame for line {0}: {1}")] + NoFileBlame(u32, &'a anyhow::Error), + #[error("The blame for this file is not ready yet. Try again in a few seconds")] + NotReadyYet, +} + use std::{fmt, mem}; impl fmt::Debug for Document { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -719,6 +736,19 @@ impl Document { jump_labels: HashMap::new(), color_swatches: None, color_swatch_controller: TaskController::new(), + file_blame: None, + is_blame_potentially_out_of_date: false, + } + } + + pub fn should_request_full_file_blame( + &mut self, + blame_behaviour: InlineBlameBehaviour, + ) -> bool { + if blame_behaviour == InlineBlameBehaviour::Disabled { + self.is_blame_potentially_out_of_date + } else { + true } } @@ -1310,6 +1340,13 @@ impl Document { Range::new(0, 1).grapheme_aligned(self.text().slice(..)) } + /// Get the line of cursor for the primary selection + pub fn cursor_line(&self, view_id: ViewId) -> usize { + let text = self.text(); + let selection = self.selection(view_id); + text.char_to_line(selection.primary().cursor(text.slice(..))) + } + /// Reset the view's selection on this document to the /// [origin](Document::origin) cursor. pub fn reset_selection(&mut self, view_id: ViewId) { @@ -1541,6 +1578,19 @@ impl Document { self.apply_inner(transaction, view_id, true) } + /// Get the line blame for this view + pub fn line_blame(&self, cursor_line: u32, format: &str) -> Result { + Ok(self + .file_blame + .as_ref() + .ok_or(LineBlameError::NotReadyYet)? + .as_ref() + .map_err(|err| LineBlameError::NoFileBlame(cursor_line.saturating_add(1), err))? + .blame_for_line(cursor_line, self.diff_handle()) + .ok_or(LineBlameError::NotCommittedYet)? + .parse_format(format)) + } + /// Apply a [`Transaction`] to the [`Document`] to change its text /// without notifying the language servers. This is useful for temporary transactions /// that must not influence the server. diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 27a985ac3..c82eba536 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -172,6 +172,44 @@ impl Default for GutterLineNumbersConfig { } } +#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum InlineBlameBehaviour { + /// Do not show inline blame + /// + /// Loads blame for the file in the background when the document is + /// opened and request it again when it is `:reload`ed. + /// + /// This allows instantaneous access to line blame with `space + B` and when + /// `:toggle inline-blame.enable` but for the cost of consuming more + /// resources in the background + Background, + /// Do not show inline blame, and do not request it in the background + /// + /// When manually requesting the inline blame, it may take several seconds to appear. + Disabled, + /// Show the inline blame + Visible, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case", default, deny_unknown_fields)] +pub struct InlineBlameConfig { + /// Show inline blame for a line when cursor is on that line + pub behaviour: InlineBlameBehaviour, + /// How the inline blame should look like and the information it includes + pub format: String, +} + +impl Default for InlineBlameConfig { + fn default() -> Self { + Self { + behaviour: InlineBlameBehaviour::Disabled, + format: "{author}, {time-ago} • {message} • {commit}".to_owned(), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default, deny_unknown_fields)] pub struct FilePickerConfig { @@ -370,6 +408,8 @@ pub struct Config { /// Whether to read settings from [EditorConfig](https://editorconfig.org) files. Defaults to /// `true`. pub editor_config: bool, + /// Inline blame allows showing the latest commit that affected the line the cursor is on as virtual text + pub inline_blame: InlineBlameConfig, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] @@ -1016,6 +1056,7 @@ impl Default for Config { inline_diagnostics: InlineDiagnosticsConfig::default(), end_of_line_diagnostics: DiagnosticFilter::Disable, clipboard_provider: ClipboardProvider::default(), + inline_blame: InlineBlameConfig::default(), editor_config: true, } } @@ -1784,11 +1825,13 @@ impl Editor { doc.set_version_control_head(self.diff_providers.get_current_head_name(&path)); let id = self.new_document(doc); + self.launch_language_servers(id); helix_event::dispatch(DocumentDidOpen { editor: self, doc: id, + path: &path, }); id diff --git a/helix-view/src/events.rs b/helix-view/src/events.rs index 4a44beb35..89969642b 100644 --- a/helix-view/src/events.rs +++ b/helix-view/src/events.rs @@ -7,7 +7,8 @@ use crate::{Document, DocumentId, Editor, ViewId}; events! { DocumentDidOpen<'a> { editor: &'a mut Editor, - doc: DocumentId + doc: DocumentId, + path: &'a std::path::PathBuf } DocumentDidChange<'a> { doc: &'a mut Document, diff --git a/helix-view/src/handlers.rs b/helix-view/src/handlers.rs index 258ed89e5..e6a303c68 100644 --- a/helix-view/src/handlers.rs +++ b/helix-view/src/handlers.rs @@ -16,12 +16,24 @@ pub enum AutoSaveEvent { LeftInsertMode, } +#[derive(Debug)] +pub struct BlameEvent { + /// The path for which we request blame + pub path: std::path::PathBuf, + /// Document for which the blame is requested + pub doc_id: DocumentId, + /// If this field is set, when we obtain the blame for the file we will + /// show blame for this line in the status line + pub line: Option, +} + pub struct Handlers { // only public because most of the actual implementation is in helix-term right now :/ pub completions: CompletionHandler, pub signature_hints: Sender, pub auto_save: Sender, pub document_colors: Sender, + pub blame: Sender, } impl Handlers { From 647615ddecccad396be9136c3bb0913c4d0890d3 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 24 Mar 2025 01:31:01 +0000 Subject: [PATCH 02/22] perf: optimize obtaining blame for the same line _ fix: blame_line_impl _ _ _ _ _ --- helix-term/src/commands.rs | 8 ++- helix-vcs/src/git/blame.rs | 134 +++++++++++++++++++------------------ helix-view/src/document.rs | 61 ++++++++++++++--- 3 files changed, 125 insertions(+), 78 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index c1d1a46af..f8a29e1c3 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3472,9 +3472,9 @@ fn insert_at_line_start(cx: &mut Context) { insert_with_indent(cx, IndentFallbackPos::LineStart); } -pub(crate) fn blame_line_impl(editor: &mut Editor, doc: DocumentId, cursor_line: u32) { +pub(crate) fn blame_line_impl(editor: &mut Editor, doc_id: DocumentId, cursor_line: u32) { let inline_blame_config = &editor.config().inline_blame; - let Some(doc) = editor.document(doc) else { + let Some(doc) = editor.document(doc_id) else { return; }; let line_blame = match doc.line_blame(cursor_line, &inline_blame_config.format) { @@ -3494,7 +3494,9 @@ pub(crate) fn blame_line_impl(editor: &mut Editor, doc: DocumentId, cursor_line: }, ); editor.set_status(format!("Requested blame for {}...", path.display())); - let doc = doc_mut!(editor); + let doc = editor + .document_mut(doc_id) + .expect("exists since we return from the function earlier if it does not"); doc.is_blame_potentially_out_of_date = false; } else { editor.set_error("Could not get path of document"); diff --git a/helix-vcs/src/git/blame.rs b/helix-vcs/src/git/blame.rs index 70e630bcc..27887716d 100644 --- a/helix-vcs/src/git/blame.rs +++ b/helix-vcs/src/git/blame.rs @@ -1,65 +1,38 @@ use anyhow::Context as _; use anyhow::Result; +use std::cell::RefCell; use std::collections::HashMap; use std::path::PathBuf; -use crate::DiffHandle; - use super::{get_repo_dir, open_repo}; +/// Allows us to save compute resources when requesting blame for the same line +/// To go from an `ObjectId` (which represents a commit) to `LineBLame`, we have to perform some work. +/// +/// With this struct, we only do this work once. Getting `LineBlame` for the same line for the 2nd and subsequent +/// times is going to be free. This is important because we do this step on every render, in the main thread. +#[derive(Debug)] +enum LineBlameUnit { + /// The raw object id of the commit for a line. + /// It will take a bit of compute in order to obtain the `LineBlame` for it. + Unprocessed(gix::ObjectId), + /// Fully processed line blame information. + Processed(LineBlame), +} + /// Stores information about the blame for a file #[derive(Debug)] pub struct FileBlame { - /// A map from line numbers to commit IDs - blame: HashMap, + /// A map from line numbers to blame for that line + blame: RefCell>, /// The owning repository for this file's `ObjectId`s repo: gix::ThreadSafeRepository, } impl FileBlame { /// Get the blame information corresponing to a line in file and diff for that line - /// - /// returns `None` if the line is part of an insertion, as the blame for that line would not - /// be meaningful #[inline] - pub fn blame_for_line(&self, line: u32, diff_handle: Option<&DiffHandle>) -> Option { - let (inserted_lines, removed_lines) = diff_handle.map_or( - // in theory there can be situations where we don't have the diff for a file - // but we have the blame. In this case, we can just act like there is no diff - Some((0, 0)), - |diff_handle| { - // Compute the amount of lines inserted and deleted before the `line` - // This information is needed to accurately transform the state of the - // file in the file system into what gix::blame knows about (gix::blame only - // knows about commit history, it does not know about uncommitted changes) - diff_handle - .load() - .hunks_intersecting_line_ranges(std::iter::once((0, line as usize))) - .try_fold( - (0, 0), - |(total_inserted_lines, total_deleted_lines), hunk| { - // check if the line intersects the hunk's `after` (which represents - // inserted lines) - (hunk.after.start > line || hunk.after.end <= line).then_some(( - total_inserted_lines + (hunk.after.end - hunk.after.start), - total_deleted_lines + (hunk.before.end - hunk.before.start), - )) - }, - ) - }, - )?; - - Some(self.blame_for_line_inserted_removed(line, inserted_lines, removed_lines)) - } - - // this is a separate function for use in Tests - #[inline] - fn blame_for_line_inserted_removed( - &self, - line: u32, - inserted_lines: u32, - removed_lines: u32, - ) -> LineBlame { + pub fn blame_for_line(&self, line: u32, inserted_lines: u32, removed_lines: u32) -> LineBlame { // Because gix_blame doesn't care about stuff that is not commited, we have to "normalize" the // line number to account for uncommited code. // @@ -74,14 +47,19 @@ impl FileBlame { let blame_line = line.saturating_sub(inserted_lines) + removed_lines; let repo = self.repo.to_thread_local(); - let commit = self - .blame - .get(&blame_line) - .and_then(|obj| repo.find_commit(*obj).ok()); + let mut blame = self.blame.borrow_mut(); + let line_blame_unit = blame.get_mut(&blame_line); + + let commit = match line_blame_unit { + Some(LineBlameUnit::Unprocessed(object_id)) => repo.find_commit(*object_id).ok(), + Some(LineBlameUnit::Processed(line_blame)) => return line_blame.clone(), + None => None, + }; + let message = commit.as_ref().and_then(|c| c.message().ok()); let author = commit.as_ref().and_then(|c| c.author().ok()); - LineBlame { + let line_blame = LineBlame { commit_hash: commit .as_ref() .and_then(|c| c.short_id().map(|id| id.to_string()).ok()), @@ -92,12 +70,19 @@ impl FileBlame { commit_body: message .as_ref() .and_then(|msg| msg.body.map(|body| body.to_string())), - time_ago: author - .map(|a| helix_stdx::time::format_relative_time(a.time.seconds, a.time.offset)), - } + time_stamp: author.map(|a| (a.time.seconds, a.time.offset)), + time_ago: None, + }; + + // we know that `line_blame_unit` here is not processed + if let Some(line_blame_unit) = line_blame_unit { + *line_blame_unit = LineBlameUnit::Processed(line_blame.clone()); + }; + + line_blame } - /// Compute blame for this file + /// Compute blame for this file (expensive) pub fn try_new(file: PathBuf) -> Result { let thread_safe_repo = open_repo(get_repo_dir(&file)?).context("Failed to open git repo")?; @@ -135,13 +120,15 @@ impl FileBlame { .entries; Ok(Self { - blame: file_blame - .into_iter() - .flat_map(|blame| { - (blame.start_in_blamed_file..blame.start_in_blamed_file + blame.len.get()) - .map(move |i| (i, blame.commit_id)) - }) - .collect(), + blame: RefCell::new( + file_blame + .into_iter() + .flat_map(|blame| { + (blame.start_in_blamed_file..blame.start_in_blamed_file + blame.len.get()) + .map(move |i| (i, LineBlameUnit::Unprocessed(blame.commit_id))) + }) + .collect(), + ), repo: thread_safe_repo, }) } @@ -155,18 +142,28 @@ pub struct LineBlame { commit_date: Option, commit_message: Option, commit_body: Option, + /// Used to compute `time-ago` + time_stamp: Option<(i64, i32)>, + /// This field is the only one that needs to be re-computed every time + /// we request the `LineBlame`. It exists here for lifetime purposes, so we can return + /// `&str` from `Self::get_variable`. + /// + /// This should only be set from within and never initialized. time_ago: Option, } impl LineBlame { /// Longest variable is: `time-ago` (and `message`) + // this is just to reduce allocation by a little bit by specifying the max size we would expect a + // variable to be up-front. This function is called every render. const LONGEST_VARIABLE_LENGTH: usize = 7; + /// # Returns /// /// None => Invalid variable /// Some(None) => Valid variable, but is empty #[inline] - fn get_variable(&self, var: &str) -> Option> { + fn get_variable(&mut self, var: &str) -> Option> { Some( // if adding new variables, update `Self::LONGEST_VARIABLE_LENGTH` match var { @@ -176,7 +173,13 @@ impl LineBlame { "message" => &self.commit_message, "email" => &self.author_email, "body" => &self.commit_body, - "time-ago" => &self.time_ago, + "time-ago" => { + let time_ago = self.time_stamp.map(|(utc_seconds, timezone_offset)| { + helix_stdx::time::format_relative_time(utc_seconds, timezone_offset) + }); + self.time_ago = time_ago; + &self.time_ago + } _ => return None, } .as_deref(), @@ -185,7 +188,7 @@ impl LineBlame { /// Parse the user's blame format #[inline] - pub fn parse_format(&self, format: &str) -> String { + pub fn parse_format(&mut self, format: &str) -> String { let mut line_blame = String::new(); let mut content_before_variable = String::with_capacity(format.len()); @@ -418,7 +421,7 @@ mod test { let blame_result = FileBlame::try_new(file.clone()) .unwrap() - .blame_for_line_inserted_removed(line_number, added_lines, removed_lines) + .blame_for_line(line_number, added_lines, removed_lines) .commit_message; assert_eq!( @@ -565,6 +568,7 @@ mod test { commit_date: Some("2028-01-10".to_owned()), commit_message: Some("feat!: extend house".to_owned()), commit_body: Some("BREAKING CHANGE: Removed door".to_owned()), + time_stamp: None, time_ago: None, } } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 2c2e57fda..9c5437bca 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -198,7 +198,7 @@ pub struct Document { diff_handle: Option, version_control_head: Option>>>, /// Contains blame information for each line in the file - /// We store the Result because when we access the blame directly we want to log the error + /// We store the Result because when we access the blame manually we want to log the error /// But if it is in the background we are just going to ignore the error pub file_blame: Option>, @@ -1580,15 +1580,56 @@ impl Document { /// Get the line blame for this view pub fn line_blame(&self, cursor_line: u32, format: &str) -> Result { - Ok(self - .file_blame - .as_ref() - .ok_or(LineBlameError::NotReadyYet)? - .as_ref() - .map_err(|err| LineBlameError::NoFileBlame(cursor_line.saturating_add(1), err))? - .blame_for_line(cursor_line, self.diff_handle()) - .ok_or(LineBlameError::NotCommittedYet)? - .parse_format(format)) + // how many lines were inserted and deleted before the cursor line + let (inserted_lines, deleted_lines) = self + .diff_handle() + .map_or( + // in theory there can be situations where we don't have the diff for a file + // but we have the blame. In this case, we can just act like there is no diff + Some((0, 0)), + |diff_handle| { + // Compute the amount of lines inserted and deleted before the `line` + // This information is needed to accurately transform the state of the + // file in the file system into what gix::blame knows about (gix::blame only + // knows about commit history, it does not know about uncommitted changes) + diff_handle + .load() + .hunks_intersecting_line_ranges(std::iter::once((0, cursor_line as usize))) + .try_fold( + (0, 0), + |(total_inserted_lines, total_deleted_lines), hunk| { + // check if the line intersects the hunk's `after` (which represents + // inserted lines) + (hunk.after.start > cursor_line || hunk.after.end <= cursor_line) + .then_some(( + total_inserted_lines + (hunk.after.end - hunk.after.start), + total_deleted_lines + (hunk.before.end - hunk.before.start), + )) + }, + ) + }, + ) + .ok_or(LineBlameError::NotCommittedYet)?; + + let file_blame = match &self.file_blame { + None => return Err(LineBlameError::NotReadyYet), + Some(result) => match result { + Err(err) => { + return Err(LineBlameError::NoFileBlame( + // convert 0-based line into 1-based line + cursor_line.saturating_add(1), + err, + )); + } + Ok(file_blame) => file_blame, + }, + }; + + let line_blame = file_blame + .blame_for_line(cursor_line, inserted_lines, deleted_lines) + .parse_format(format); + + Ok(line_blame) } /// Apply a [`Transaction`] to the [`Document`] to change its text From 07c69c1e742caa8139ee115f9027ab3ef13b15df Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 24 Mar 2025 04:00:02 +0000 Subject: [PATCH 03/22] fix: update blame when editing config --- helix-term/src/application.rs | 6 ++++++ helix-term/src/events.rs | 3 ++- helix-term/src/handlers/blame.rs | 24 +++++++++++++++++++++++- helix-view/src/events.rs | 7 ++++++- 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 3bc324395..b97cb29e4 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -11,6 +11,7 @@ use helix_view::{ align_view, document::{DocumentOpenError, DocumentSavedEventResult}, editor::{ConfigEvent, EditorEvent}, + events::EditorConfigDidChange, graphics::Rect, theme, tree::Layout, @@ -364,6 +365,11 @@ impl Application { // the Application can apply it. ConfigEvent::Update(editor_config) => { let mut app_config = (*self.config.load().clone()).clone(); + helix_event::dispatch(EditorConfigDidChange { + old_config: &app_config.editor, + new_config: &editor_config, + editor: &mut self.editor, + }); app_config.editor = *editor_config; if let Err(err) = self.terminal.reconfigure(app_config.editor.clone().into()) { self.editor.set_error(err.to_string()); diff --git a/helix-term/src/events.rs b/helix-term/src/events.rs index 80f045cd7..4e1161d14 100644 --- a/helix-term/src/events.rs +++ b/helix-term/src/events.rs @@ -2,7 +2,7 @@ use helix_event::{events, register_event}; use helix_view::document::Mode; use helix_view::events::{ DiagnosticsDidChange, DocumentDidChange, DocumentDidClose, DocumentDidOpen, DocumentFocusLost, - LanguageServerExited, LanguageServerInitialized, SelectionDidChange, + EditorConfigDidChange, LanguageServerExited, LanguageServerInitialized, SelectionDidChange, }; use crate::commands; @@ -20,6 +20,7 @@ pub fn register() { register_event::(); register_event::(); register_event::(); + register_event::(); register_event::(); register_event::(); register_event::(); diff --git a/helix-term/src/handlers/blame.rs b/helix-term/src/handlers/blame.rs index 6a35859f8..93fdf4e3d 100644 --- a/helix-term/src/handlers/blame.rs +++ b/helix-term/src/handlers/blame.rs @@ -4,7 +4,7 @@ use helix_event::register_hook; use helix_vcs::FileBlame; use helix_view::{ editor::InlineBlameBehaviour, - events::DocumentDidOpen, + events::{DocumentDidOpen, EditorConfigDidChange}, handlers::{BlameEvent, Handlers}, DocumentId, }; @@ -91,4 +91,26 @@ pub(super) fn register_hooks(handlers: &Handlers) { } Ok(()) }); + let tx = handlers.blame.clone(); + register_hook!(move |event: &mut EditorConfigDidChange<'_>| { + if event.old_config.inline_blame.behaviour == InlineBlameBehaviour::Disabled + && event.new_config.inline_blame.behaviour != InlineBlameBehaviour::Disabled + { + // request blame for all documents, since any of them could have + // outdated blame + for doc in event.editor.documents() { + if let Some(path) = doc.path() { + helix_event::send_blocking( + &tx, + BlameEvent { + path: path.to_path_buf(), + doc_id: doc.id(), + line: None, + }, + ); + } + } + } + Ok(()) + }); } diff --git a/helix-view/src/events.rs b/helix-view/src/events.rs index 89969642b..58881d9ef 100644 --- a/helix-view/src/events.rs +++ b/helix-view/src/events.rs @@ -2,7 +2,7 @@ use helix_core::{ChangeSet, Rope}; use helix_event::events; use helix_lsp::LanguageServerId; -use crate::{Document, DocumentId, Editor, ViewId}; +use crate::{editor::Config, Document, DocumentId, Editor, ViewId}; events! { DocumentDidOpen<'a> { @@ -17,6 +17,11 @@ events! { changes: &'a ChangeSet, ghost_transaction: bool } + EditorConfigDidChange<'a> { + old_config: &'a Config, + new_config: &'a Config, + editor: &'a mut Editor + } DocumentDidClose<'a> { editor: &'a mut Editor, doc: Document From 8f0721f00aca257809a52af8c4bf8876053b6bbe Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 24 Mar 2025 16:07:19 +0000 Subject: [PATCH 04/22] use format! instead of preallocating this is more efficient apparently --- helix-stdx/src/time.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/helix-stdx/src/time.rs b/helix-stdx/src/time.rs index a7bb1a040..cb72a9386 100644 --- a/helix-stdx/src/time.rs +++ b/helix-stdx/src/time.rs @@ -71,11 +71,5 @@ pub fn format_relative_time(timestamp: i64, timezone_offset: i32) -> String { "from now" }; - let mut relative_time = String::with_capacity(value.len() + 1 + unit.len() + 1 + label.len()); - relative_time.push_str(&value); - relative_time.push(' '); - relative_time.push_str(unit); - relative_time.push(' '); - relative_time.push_str(label); - relative_time + format!("{value} {unit} {label}") } From f54fdef099c5e7ae8094579dfc70489d20620835 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 11:46:36 +0000 Subject: [PATCH 05/22] refactor: remove extra layer of sync --- helix-term/src/handlers/blame.rs | 30 ++++++------------------------ helix-vcs/src/git/blame.rs | 8 ++++---- 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/helix-term/src/handlers/blame.rs b/helix-term/src/handlers/blame.rs index 93fdf4e3d..218db731e 100644 --- a/helix-term/src/handlers/blame.rs +++ b/helix-term/src/handlers/blame.rs @@ -1,4 +1,4 @@ -use std::time::Duration; +use std::{mem, time::Duration}; use helix_event::register_hook; use helix_vcs::FileBlame; @@ -8,13 +8,13 @@ use helix_view::{ handlers::{BlameEvent, Handlers}, DocumentId, }; -use tokio::{sync::oneshot, time::Instant}; +use tokio::time::Instant; use crate::job; #[derive(Default)] pub struct BlameHandler { - worker: Option>>, + file_blame: Option>, doc_id: DocumentId, show_blame_for_line_in_statusline: Option, } @@ -27,36 +27,18 @@ impl helix_event::AsyncHook for BlameHandler { event: Self::Event, _timeout: Option, ) -> Option { - if let Some(worker) = &mut self.worker { - if worker.try_recv().is_ok() { - self.finish_debounce(); - return None; - } - } - self.doc_id = event.doc_id; self.show_blame_for_line_in_statusline = event.line; - let (tx, rx) = oneshot::channel(); - - tokio::spawn(async move { - let result = FileBlame::try_new(event.path); - let _ = tx.send(result); - }); - - self.worker = Some(rx); - + self.file_blame = Some(FileBlame::try_new(event.path)); Some(Instant::now() + Duration::from_millis(50)) } fn finish_debounce(&mut self) { let doc_id = self.doc_id; let line_blame = self.show_blame_for_line_in_statusline; - if let Some(worker) = self.worker.take() { + let result = mem::take(&mut self.file_blame); + if let Some(result) = result { tokio::spawn(async move { - let Ok(result) = worker.await else { - return; - }; - job::dispatch(move |editor, _| { let Some(doc) = editor.document_mut(doc_id) else { return; diff --git a/helix-vcs/src/git/blame.rs b/helix-vcs/src/git/blame.rs index 27887716d..0b83bd1d8 100644 --- a/helix-vcs/src/git/blame.rs +++ b/helix-vcs/src/git/blame.rs @@ -1,6 +1,6 @@ use anyhow::Context as _; use anyhow::Result; -use std::cell::RefCell; +use parking_lot::Mutex; use std::collections::HashMap; use std::path::PathBuf; @@ -24,7 +24,7 @@ enum LineBlameUnit { #[derive(Debug)] pub struct FileBlame { /// A map from line numbers to blame for that line - blame: RefCell>, + blame: Mutex>, /// The owning repository for this file's `ObjectId`s repo: gix::ThreadSafeRepository, } @@ -47,7 +47,7 @@ impl FileBlame { let blame_line = line.saturating_sub(inserted_lines) + removed_lines; let repo = self.repo.to_thread_local(); - let mut blame = self.blame.borrow_mut(); + let mut blame = self.blame.lock(); let line_blame_unit = blame.get_mut(&blame_line); let commit = match line_blame_unit { @@ -120,7 +120,7 @@ impl FileBlame { .entries; Ok(Self { - blame: RefCell::new( + blame: Mutex::new( file_blame .into_iter() .flat_map(|blame| { From 7478d9e688e32cb24b38c558307ab88c78cc0472 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 11:55:05 +0000 Subject: [PATCH 06/22] refactor: extract as variable --- helix-term/src/handlers/blame.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/helix-term/src/handlers/blame.rs b/helix-term/src/handlers/blame.rs index 218db731e..fdf53b8cc 100644 --- a/helix-term/src/handlers/blame.rs +++ b/helix-term/src/handlers/blame.rs @@ -75,9 +75,11 @@ pub(super) fn register_hooks(handlers: &Handlers) { }); let tx = handlers.blame.clone(); register_hook!(move |event: &mut EditorConfigDidChange<'_>| { - if event.old_config.inline_blame.behaviour == InlineBlameBehaviour::Disabled - && event.new_config.inline_blame.behaviour != InlineBlameBehaviour::Disabled - { + let has_enabled_inline_blame = event.old_config.inline_blame.behaviour + == InlineBlameBehaviour::Disabled + && event.new_config.inline_blame.behaviour != InlineBlameBehaviour::Disabled; + + if has_enabled_inline_blame { // request blame for all documents, since any of them could have // outdated blame for doc in event.editor.documents() { From 76a92aff2ff1873c0db0446c1568e3409e86be23 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 12:25:12 +0000 Subject: [PATCH 07/22] feat: `all-lines` option for inline blame --- book/src/editor.md | 3 ++- helix-term/src/ui/editor.rs | 20 ++++++++++++++++---- helix-term/src/ui/text_decorations/blame.rs | 18 +++++++++--------- helix-view/src/editor.rs | 6 ++++-- 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/book/src/editor.md b/book/src/editor.md index 9958798eb..29e1ac563 100644 --- a/book/src/editor.md +++ b/book/src/editor.md @@ -170,7 +170,8 @@ The following statusline elements can be configured: | `format` | The format in which to show the inline blame | `"{author}, {time-ago} • {message} • {commit}"` | The `behaviour` can be one of the following: -- `"visible"`: Inline blame is turned on. Virtual text will appear at the end of each non-empty line, showing information about the latest commit for that line. +- `"all-lines"`: Inline blame is turned on. Virtual text will appear at the end of each non-empty line, showing information about the latest commit for that line. +- `"cursor-line"`: Inline blame is turned on. Virtual text will appear at the end of the current cursor line, showing information about the latest commit for that line. - `"background"`: Inline blame is turned off, but the blame is still requested in the background when opening and reloading files. This will have zero impact on performance, but will use slightly more resources in the background. This allows blame for line (`space + B`) to be retrieved instantaneously with zero delay. - `"disabled"`: Inline blame is turned off, with no requests happening in the background. When you run `space + B` for the first time in a file, it will load the blame for this file. You may have to wait a little bit for the blame to become available, depending on the size of your repository. After it becomes available, for this file `space + B` will retrieve the blame for any line in this file with zero delay. If the file is reloaded, the process will repeat as the blame is potentially out of date and needs to be refreshed. diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 739f109c8..faf85344c 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -31,7 +31,7 @@ use helix_view::{ keyboard::{KeyCode, KeyModifiers}, Document, Editor, Theme, View, }; -use std::{mem::take, num::NonZeroUsize, path::PathBuf, rc::Rc}; +use std::{collections::HashMap, mem::take, num::NonZeroUsize, path::PathBuf, rc::Rc}; use tui::{buffer::Buffer as Surface, text::Span}; @@ -204,7 +204,7 @@ impl EditorView { config.end_of_line_diagnostics, )); - if config.inline_blame.behaviour == InlineBlameBehaviour::Visible { + if config.inline_blame.behaviour == InlineBlameBehaviour::CursorLine { let cursor_line_idx = doc.cursor_line(view.id); // do not render inline blame for empty lines to reduce visual noise @@ -214,11 +214,23 @@ impl EditorView { { decorations.add_decoration(InlineBlame::new( theme, - cursor_line_idx, - line_blame, + HashMap::from([(cursor_line_idx, line_blame)]), )); }; } + } else if config.inline_blame.behaviour == InlineBlameBehaviour::AllLines { + let blame_for_all_lines = (0..doc.text().len_lines()) + .filter_map(|line_idx| { + (doc.text().line(line_idx) != doc.line_ending.as_str()) + .then(|| { + doc.line_blame(line_idx as u32, &config.inline_blame.format) + .ok() + .map(|blame| (line_idx, blame)) + }) + .flatten() + }) + .collect(); + decorations.add_decoration(InlineBlame::new(theme, blame_for_all_lines)); } render_document( diff --git a/helix-term/src/ui/text_decorations/blame.rs b/helix-term/src/ui/text_decorations/blame.rs index 49b0a31d7..df2669c36 100644 --- a/helix-term/src/ui/text_decorations/blame.rs +++ b/helix-term/src/ui/text_decorations/blame.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use helix_core::Position; use helix_view::theme::Style; @@ -7,17 +9,15 @@ use crate::ui::document::{LinePos, TextRenderer}; use crate::ui::text_decorations::Decoration; pub struct InlineBlame { - message: String, - cursor: usize, + lines: HashMap, style: Style, } impl InlineBlame { - pub fn new(theme: &Theme, cursor: usize, message: String) -> Self { + pub fn new(theme: &Theme, lines: HashMap) -> Self { InlineBlame { style: theme.get("ui.virtual.inline-blame"), - message, - cursor, + lines, } } } @@ -29,10 +29,10 @@ impl Decoration for InlineBlame { pos: LinePos, virt_off: Position, ) -> Position { - // do not draw inline blame for lines other than the cursor line - if self.cursor != pos.doc_line { + let Some(blame) = self.lines.get(&pos.doc_line) else { + // do not draw inline blame for lines that have no content in them return Position::new(0, 0); - } + }; // where the line in the document ends let end_of_line = virt_off.col as u16; @@ -48,7 +48,7 @@ impl Decoration for InlineBlame { .set_string_truncated( renderer.viewport.x + start_drawing_at, pos.visual_line, - &self.message, + &blame, renderer.viewport.width.saturating_sub(start_drawing_at) as usize, |_| self.style, true, diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index c82eba536..5d930c113 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -188,8 +188,10 @@ pub enum InlineBlameBehaviour { /// /// When manually requesting the inline blame, it may take several seconds to appear. Disabled, - /// Show the inline blame - Visible, + /// Show the inline blame on the cursor line + CursorLine, + /// Show the inline blame on every other line + AllLines, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] From ac0e677fae088ef4ccf252411857346c649656cd Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 12:34:01 +0000 Subject: [PATCH 08/22] chore: appease clippy --- helix-term/src/ui/text_decorations/blame.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-term/src/ui/text_decorations/blame.rs b/helix-term/src/ui/text_decorations/blame.rs index df2669c36..69a1d2ac6 100644 --- a/helix-term/src/ui/text_decorations/blame.rs +++ b/helix-term/src/ui/text_decorations/blame.rs @@ -48,7 +48,7 @@ impl Decoration for InlineBlame { .set_string_truncated( renderer.viewport.x + start_drawing_at, pos.visual_line, - &blame, + blame, renderer.viewport.width.saturating_sub(start_drawing_at) as usize, |_| self.style, true, From d34074af1b552858d1fe6fdfec270ed725311a5b Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 12:51:47 +0000 Subject: [PATCH 09/22] perf: do not render inline blame on invisible lines --- helix-term/src/ui/editor.rs | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index faf85344c..c99b6ad6c 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -219,15 +219,29 @@ impl EditorView { }; } } else if config.inline_blame.behaviour == InlineBlameBehaviour::AllLines { - let blame_for_all_lines = (0..doc.text().len_lines()) + let text = doc.text(); + let len_lines = text.len_lines(); + let view_height = view.inner_height(); + let first_visible_line = + text.char_to_line(doc.view_offset(view.id).anchor.min(text.len_chars())); + let first_line = first_visible_line.saturating_sub(view_height); + let last_line = first_visible_line + .saturating_add(view_height.saturating_mul(2)) + .min(len_lines); + + // Compute ~3 times the current view height of inline blame, that way some scrolling + // will not show half the view with inline blame and half without while still being faster + // than rendering inline blame for the full file. + let blame_for_all_lines = (first_line..last_line) .filter_map(|line_idx| { - (doc.text().line(line_idx) != doc.line_ending.as_str()) - .then(|| { - doc.line_blame(line_idx as u32, &config.inline_blame.format) - .ok() - .map(|blame| (line_idx, blame)) - }) - .flatten() + // do not render inline blame for empty lines to reduce visual noise + if text.line(line_idx) != doc.line_ending.as_str() { + doc.line_blame(line_idx as u32, &config.inline_blame.format) + .ok() + .map(|blame| (line_idx, blame)) + } else { + None + } }) .collect(); decorations.add_decoration(InlineBlame::new(theme, blame_for_all_lines)); From b9f82262084174efc2250394c2bc85d6139c335a Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 13:03:20 +0000 Subject: [PATCH 10/22] refactor: remove `new_config` from EditorConfigDidChange event There is no need for it because we have access to `Editor::config()` --- helix-term/src/application.rs | 1 - helix-term/src/handlers/blame.rs | 2 +- helix-view/src/events.rs | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index b97cb29e4..c9cdd6507 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -367,7 +367,6 @@ impl Application { let mut app_config = (*self.config.load().clone()).clone(); helix_event::dispatch(EditorConfigDidChange { old_config: &app_config.editor, - new_config: &editor_config, editor: &mut self.editor, }); app_config.editor = *editor_config; diff --git a/helix-term/src/handlers/blame.rs b/helix-term/src/handlers/blame.rs index fdf53b8cc..9f6fe9201 100644 --- a/helix-term/src/handlers/blame.rs +++ b/helix-term/src/handlers/blame.rs @@ -77,7 +77,7 @@ pub(super) fn register_hooks(handlers: &Handlers) { register_hook!(move |event: &mut EditorConfigDidChange<'_>| { let has_enabled_inline_blame = event.old_config.inline_blame.behaviour == InlineBlameBehaviour::Disabled - && event.new_config.inline_blame.behaviour != InlineBlameBehaviour::Disabled; + && event.editor.config().inline_blame.behaviour != InlineBlameBehaviour::Disabled; if has_enabled_inline_blame { // request blame for all documents, since any of them could have diff --git a/helix-view/src/events.rs b/helix-view/src/events.rs index 58881d9ef..a54ec8186 100644 --- a/helix-view/src/events.rs +++ b/helix-view/src/events.rs @@ -19,7 +19,6 @@ events! { } EditorConfigDidChange<'a> { old_config: &'a Config, - new_config: &'a Config, editor: &'a mut Editor } DocumentDidClose<'a> { From 22f9571687bb73c8758e65e9c52b3516ef1be414 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 19:01:12 +0000 Subject: [PATCH 11/22] feat: split `inline-blame.behaviour` into two options _ _ --- book/src/editor.md | 18 +++++++++++++----- helix-term/src/commands.rs | 5 +++-- helix-term/src/commands/typed.rs | 8 ++++---- helix-term/src/handlers/blame.rs | 12 ++++++------ helix-view/src/document.rs | 9 +++------ helix-view/src/editor.rs | 27 +++++++++++++++------------ 6 files changed, 44 insertions(+), 35 deletions(-) diff --git a/book/src/editor.md b/book/src/editor.md index 29e1ac563..25d98b84b 100644 --- a/book/src/editor.md +++ b/book/src/editor.md @@ -164,16 +164,24 @@ The following statusline elements can be configured: ### `[editor.inline-blame]` Section +Inline blame is virtual text that appears at the end of a line, displaying information about the most recent commit that affected this line. + | Key | Description | Default | | ------- | ------------------------------------------ | ------- | -| `behaviour` | Choose the behaviour of inline blame | `"disabled"` | +| `behaviour` | Choose when to show inline blame | `"hidden"` | +| `compute` | Choose when inline blame should be computed | `"on-demand"` | | `format` | The format in which to show the inline blame | `"{author}, {time-ago} • {message} • {commit}"` | The `behaviour` can be one of the following: -- `"all-lines"`: Inline blame is turned on. Virtual text will appear at the end of each non-empty line, showing information about the latest commit for that line. -- `"cursor-line"`: Inline blame is turned on. Virtual text will appear at the end of the current cursor line, showing information about the latest commit for that line. -- `"background"`: Inline blame is turned off, but the blame is still requested in the background when opening and reloading files. This will have zero impact on performance, but will use slightly more resources in the background. This allows blame for line (`space + B`) to be retrieved instantaneously with zero delay. -- `"disabled"`: Inline blame is turned off, with no requests happening in the background. When you run `space + B` for the first time in a file, it will load the blame for this file. You may have to wait a little bit for the blame to become available, depending on the size of your repository. After it becomes available, for this file `space + B` will retrieve the blame for any line in this file with zero delay. If the file is reloaded, the process will repeat as the blame is potentially out of date and needs to be refreshed. +- `"all-lines"`: Inline blame is on every line. +- `"cursor-line"`: Inline blame is only on the line of the primary cursor. +- `"hidden"`: Inline blame is not shown. + +Inline blame will only show if the blame for the file has already been computed. + +The `compute` key determines under which circumstances the blame is computed, and can be one of the following: +- `"on-demand"`: Blame for the file is computed only when explicitly requested, such as when using `space + B` to blame the line of the cursor. There may be a little delay when loading the blame. When opening new files, even with `behaviour` not set to `"hidden"`, the inline blame won't show. It needs to be computed first in order to become available. This computation can be manually triggered by requesting it with `space + B`. +- `"background"`: Blame for the file is loaded in the background. This will have zero effect on performance of the Editor, but will use a little bit extra resources. Directly requesting the blame with `space + B` will be instant. Inline blame will show as soon as the blame is available when loading new files. `inline-blame-format` allows customization of the blame message, and can be set to any string. Variables can be used like so: `{variable}`. These are the available variables: diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index f8a29e1c3..df46b381d 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3480,8 +3480,9 @@ pub(crate) fn blame_line_impl(editor: &mut Editor, doc_id: DocumentId, cursor_li let line_blame = match doc.line_blame(cursor_line, &inline_blame_config.format) { result if (result.is_ok() && doc.is_blame_potentially_out_of_date) - || matches!(result, Err(LineBlameError::NotReadyYet) if inline_blame_config.behaviour - == helix_view::editor::InlineBlameBehaviour::Disabled) => + || matches!(result, Err(LineBlameError::NotReadyYet) if inline_blame_config.compute + == helix_view::editor::InlineBlameCompute::OnDemand + ) => { if let Some(path) = doc.path() { let tx = editor.handlers.blame.clone(); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 15792bc66..8960093ab 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1327,7 +1327,7 @@ fn reload(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> anyh } let scrolloff = cx.editor.config().scrolloff; - let inline_blame_behaviour = cx.editor.config().inline_blame.behaviour; + let inline_compute = cx.editor.config().inline_blame.compute; let (view, doc) = current!(cx.editor); doc.reload(view, &cx.editor.diff_providers).map(|_| { view.ensure_cursor_in_view(doc, scrolloff); @@ -1340,7 +1340,7 @@ fn reload(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> anyh .file_changed(path.clone()); } - if doc.should_request_full_file_blame(inline_blame_behaviour) { + if doc.should_request_full_file_blame(inline_compute) { if let Some(path) = doc.path() { helix_event::send_blocking( &cx.editor.handlers.blame, @@ -1380,7 +1380,7 @@ fn reload_all(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> }) .collect(); - let blame_behaviour = cx.editor.config().inline_blame.behaviour; + let blame_compute = cx.editor.config().inline_blame.compute; for (doc_id, view_ids) in docs_view_ids { let doc = doc_mut!(cx.editor, &doc_id); @@ -1410,7 +1410,7 @@ fn reload_all(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> } } - if doc.should_request_full_file_blame(blame_behaviour) { + if doc.should_request_full_file_blame(blame_compute) { if let Some(path) = doc.path() { helix_event::send_blocking( &cx.editor.handlers.blame, diff --git a/helix-term/src/handlers/blame.rs b/helix-term/src/handlers/blame.rs index 9f6fe9201..cf8410897 100644 --- a/helix-term/src/handlers/blame.rs +++ b/helix-term/src/handlers/blame.rs @@ -3,7 +3,7 @@ use std::{mem, time::Duration}; use helix_event::register_hook; use helix_vcs::FileBlame; use helix_view::{ - editor::InlineBlameBehaviour, + editor::InlineBlameCompute, events::{DocumentDidOpen, EditorConfigDidChange}, handlers::{BlameEvent, Handlers}, DocumentId, @@ -44,7 +44,7 @@ impl helix_event::AsyncHook for BlameHandler { return; }; doc.file_blame = Some(result); - if editor.config().inline_blame.behaviour == InlineBlameBehaviour::Disabled { + if editor.config().inline_blame.compute == InlineBlameCompute::OnDemand { if let Some(line) = line_blame { crate::commands::blame_line_impl(editor, doc_id, line); } else { @@ -61,7 +61,7 @@ impl helix_event::AsyncHook for BlameHandler { pub(super) fn register_hooks(handlers: &Handlers) { let tx = handlers.blame.clone(); register_hook!(move |event: &mut DocumentDidOpen<'_>| { - if event.editor.config().inline_blame.behaviour != InlineBlameBehaviour::Disabled { + if event.editor.config().inline_blame.compute != InlineBlameCompute::OnDemand { helix_event::send_blocking( &tx, BlameEvent { @@ -75,9 +75,9 @@ pub(super) fn register_hooks(handlers: &Handlers) { }); let tx = handlers.blame.clone(); register_hook!(move |event: &mut EditorConfigDidChange<'_>| { - let has_enabled_inline_blame = event.old_config.inline_blame.behaviour - == InlineBlameBehaviour::Disabled - && event.editor.config().inline_blame.behaviour != InlineBlameBehaviour::Disabled; + let has_enabled_inline_blame = event.old_config.inline_blame.compute + == InlineBlameCompute::OnDemand + && event.editor.config().inline_blame.compute == InlineBlameCompute::Background; if has_enabled_inline_blame { // request blame for all documents, since any of them could have diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 9c5437bca..d0007a1fd 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -42,7 +42,7 @@ use helix_core::{ ChangeSet, Diagnostic, LineEnding, Range, Rope, RopeBuilder, Selection, Syntax, Transaction, }; -use crate::editor::InlineBlameBehaviour; +use crate::editor::InlineBlameCompute; use crate::{ editor::Config, events::{DocumentDidChange, SelectionDidChange}, @@ -741,11 +741,8 @@ impl Document { } } - pub fn should_request_full_file_blame( - &mut self, - blame_behaviour: InlineBlameBehaviour, - ) -> bool { - if blame_behaviour == InlineBlameBehaviour::Disabled { + pub fn should_request_full_file_blame(&mut self, blame_fetch: InlineBlameCompute) -> bool { + if blame_fetch == InlineBlameCompute::OnDemand { self.is_blame_potentially_out_of_date } else { true diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 5d930c113..408efb72a 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -175,30 +175,32 @@ impl Default for GutterLineNumbersConfig { #[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub enum InlineBlameBehaviour { - /// Do not show inline blame - /// - /// Loads blame for the file in the background when the document is - /// opened and request it again when it is `:reload`ed. - /// - /// This allows instantaneous access to line blame with `space + B` and when - /// `:toggle inline-blame.enable` but for the cost of consuming more - /// resources in the background - Background, /// Do not show inline blame, and do not request it in the background /// /// When manually requesting the inline blame, it may take several seconds to appear. - Disabled, + Hidden, /// Show the inline blame on the cursor line CursorLine, /// Show the inline blame on every other line AllLines, } +#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum InlineBlameCompute { + /// Inline blame for a file will be fetched when a document is opened or reloaded, for example + Background, + /// Inline blame for a file will be fetched when explicitly requested, e.g. when using `space + B` + OnDemand, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default, deny_unknown_fields)] pub struct InlineBlameConfig { - /// Show inline blame for a line when cursor is on that line + /// How to show the inline blame pub behaviour: InlineBlameBehaviour, + /// Whether the inline blame should be fetched in the background + pub compute: InlineBlameCompute, /// How the inline blame should look like and the information it includes pub format: String, } @@ -206,8 +208,9 @@ pub struct InlineBlameConfig { impl Default for InlineBlameConfig { fn default() -> Self { Self { - behaviour: InlineBlameBehaviour::Disabled, + behaviour: InlineBlameBehaviour::Hidden, format: "{author}, {time-ago} • {message} • {commit}".to_owned(), + compute: InlineBlameCompute::OnDemand, } } } From a8097f1cdc1df111035c56abe6f41095b7ce1536 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 20:02:05 +0000 Subject: [PATCH 12/22] perf: use `Vec` instead of `HashMap` --- helix-term/src/ui/editor.rs | 41 ++++++++++++--------- helix-term/src/ui/text_decorations/blame.rs | 34 +++++++++++++---- 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index c99b6ad6c..56df7c44a 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -31,7 +31,7 @@ use helix_view::{ keyboard::{KeyCode, KeyModifiers}, Document, Editor, Theme, View, }; -use std::{collections::HashMap, mem::take, num::NonZeroUsize, path::PathBuf, rc::Rc}; +use std::{mem::take, num::NonZeroUsize, path::PathBuf, rc::Rc}; use tui::{buffer::Buffer as Surface, text::Span}; @@ -214,37 +214,44 @@ impl EditorView { { decorations.add_decoration(InlineBlame::new( theme, - HashMap::from([(cursor_line_idx, line_blame)]), + text_decorations::blame::LineBlame::OneLine((cursor_line_idx, line_blame)), )); }; } } else if config.inline_blame.behaviour == InlineBlameBehaviour::AllLines { let text = doc.text(); - let len_lines = text.len_lines(); + let text_line_count = text.len_lines(); let view_height = view.inner_height(); let first_visible_line = text.char_to_line(doc.view_offset(view.id).anchor.min(text.len_chars())); let first_line = first_visible_line.saturating_sub(view_height); let last_line = first_visible_line .saturating_add(view_height.saturating_mul(2)) - .min(len_lines); + .min(text_line_count); + + let mut blame_lines = vec![None; text_line_count]; // Compute ~3 times the current view height of inline blame, that way some scrolling // will not show half the view with inline blame and half without while still being faster // than rendering inline blame for the full file. - let blame_for_all_lines = (first_line..last_line) - .filter_map(|line_idx| { - // do not render inline blame for empty lines to reduce visual noise - if text.line(line_idx) != doc.line_ending.as_str() { - doc.line_blame(line_idx as u32, &config.inline_blame.format) - .ok() - .map(|blame| (line_idx, blame)) - } else { - None - } - }) - .collect(); - decorations.add_decoration(InlineBlame::new(theme, blame_for_all_lines)); + let blame_for_all_lines = (first_line..last_line).filter_map(|line_idx| { + // do not render inline blame for empty lines to reduce visual noise + if text.line(line_idx) != doc.line_ending.as_str() { + doc.line_blame(line_idx as u32, &config.inline_blame.format) + .ok() + .map(|blame| (line_idx, blame)) + } else { + None + } + }); + + for (line_idx, blame) in blame_for_all_lines { + blame_lines[line_idx] = Some(blame); + } + decorations.add_decoration(InlineBlame::new( + theme, + text_decorations::blame::LineBlame::ManyLines(blame_lines), + )); } render_document( diff --git a/helix-term/src/ui/text_decorations/blame.rs b/helix-term/src/ui/text_decorations/blame.rs index 69a1d2ac6..9b996f6df 100644 --- a/helix-term/src/ui/text_decorations/blame.rs +++ b/helix-term/src/ui/text_decorations/blame.rs @@ -1,5 +1,3 @@ -use std::collections::HashMap; - use helix_core::Position; use helix_view::theme::Style; @@ -8,13 +6,21 @@ use helix_view::Theme; use crate::ui::document::{LinePos, TextRenderer}; use crate::ui::text_decorations::Decoration; +pub enum LineBlame { + OneLine((usize, String)), + // Optimization: Use `Vec` insted of `HashMap` + // because we know that the amount of lines visible in the viewport X3 cannot be a very large number, + // most likely up to a few hundred. In the absolute extreme case, maybe 5,000. + ManyLines(Vec>), +} + pub struct InlineBlame { - lines: HashMap, + lines: LineBlame, style: Style, } impl InlineBlame { - pub fn new(theme: &Theme, lines: HashMap) -> Self { + pub fn new(theme: &Theme, lines: LineBlame) -> Self { InlineBlame { style: theme.get("ui.virtual.inline-blame"), lines, @@ -29,9 +35,23 @@ impl Decoration for InlineBlame { pos: LinePos, virt_off: Position, ) -> Position { - let Some(blame) = self.lines.get(&pos.doc_line) else { - // do not draw inline blame for lines that have no content in them - return Position::new(0, 0); + let blame = match &self.lines { + LineBlame::OneLine((line, blame)) => { + if line != &pos.doc_line { + // do not draw inline blame for lines that have no content in them + blame + } else { + return Position::new(0, 0); + } + } + LineBlame::ManyLines(lines) => { + if let Some(Some(blame)) = lines.get(pos.doc_line) { + blame + } else { + // do not draw inline blame for lines that have no content in them + return Position::new(0, 0); + } + } }; // where the line in the document ends From 00d168a78d4e6756101d352124f8f6e1f9b2db50 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 21:14:02 +0000 Subject: [PATCH 13/22] fix: funny boolean inversion --- helix-term/src/ui/text_decorations/blame.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-term/src/ui/text_decorations/blame.rs b/helix-term/src/ui/text_decorations/blame.rs index 9b996f6df..ada2b261f 100644 --- a/helix-term/src/ui/text_decorations/blame.rs +++ b/helix-term/src/ui/text_decorations/blame.rs @@ -37,7 +37,7 @@ impl Decoration for InlineBlame { ) -> Position { let blame = match &self.lines { LineBlame::OneLine((line, blame)) => { - if line != &pos.doc_line { + if line == &pos.doc_line { // do not draw inline blame for lines that have no content in them blame } else { From ab5663891c1a26eaed3c31cf7e7a9ce3ea7cdfb4 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 21:24:33 +0000 Subject: [PATCH 14/22] refactor: render inline blame in a separate Editor function --- helix-term/src/ui/editor.rs | 109 +++++++++++++++++++----------------- 1 file changed, 59 insertions(+), 50 deletions(-) diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 56df7c44a..56fd04241 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -25,7 +25,7 @@ use helix_core::{ use helix_view::{ annotations::diagnostics::DiagnosticFilter, document::{Mode, SCRATCH_BUFFER_NAME}, - editor::{CompleteAction, CursorShapeConfig, InlineBlameBehaviour}, + editor::{CompleteAction, CursorShapeConfig, InlineBlameBehaviour, InlineBlameConfig}, graphics::{Color, CursorKind, Modifier, Rect, Style}, input::{KeyEvent, MouseButton, MouseEvent, MouseEventKind}, keyboard::{KeyCode, KeyModifiers}, @@ -204,55 +204,7 @@ impl EditorView { config.end_of_line_diagnostics, )); - if config.inline_blame.behaviour == InlineBlameBehaviour::CursorLine { - let cursor_line_idx = doc.cursor_line(view.id); - - // do not render inline blame for empty lines to reduce visual noise - if doc.text().line(cursor_line_idx) != doc.line_ending.as_str() { - if let Ok(line_blame) = - doc.line_blame(cursor_line_idx as u32, &config.inline_blame.format) - { - decorations.add_decoration(InlineBlame::new( - theme, - text_decorations::blame::LineBlame::OneLine((cursor_line_idx, line_blame)), - )); - }; - } - } else if config.inline_blame.behaviour == InlineBlameBehaviour::AllLines { - let text = doc.text(); - let text_line_count = text.len_lines(); - let view_height = view.inner_height(); - let first_visible_line = - text.char_to_line(doc.view_offset(view.id).anchor.min(text.len_chars())); - let first_line = first_visible_line.saturating_sub(view_height); - let last_line = first_visible_line - .saturating_add(view_height.saturating_mul(2)) - .min(text_line_count); - - let mut blame_lines = vec![None; text_line_count]; - - // Compute ~3 times the current view height of inline blame, that way some scrolling - // will not show half the view with inline blame and half without while still being faster - // than rendering inline blame for the full file. - let blame_for_all_lines = (first_line..last_line).filter_map(|line_idx| { - // do not render inline blame for empty lines to reduce visual noise - if text.line(line_idx) != doc.line_ending.as_str() { - doc.line_blame(line_idx as u32, &config.inline_blame.format) - .ok() - .map(|blame| (line_idx, blame)) - } else { - None - } - }); - - for (line_idx, blame) in blame_for_all_lines { - blame_lines[line_idx] = Some(blame); - } - decorations.add_decoration(InlineBlame::new( - theme, - text_decorations::blame::LineBlame::ManyLines(blame_lines), - )); - } + Self::render_inline_blame(&config.inline_blame, doc, view, &mut decorations, theme); render_document( surface, @@ -295,6 +247,63 @@ impl EditorView { statusline::render(&mut context, statusline_area, surface); } + fn render_inline_blame( + inline_blame: &InlineBlameConfig, + doc: &Document, + view: &View, + decorations: &mut DecorationManager, + theme: &Theme, + ) { + if inline_blame.behaviour == InlineBlameBehaviour::CursorLine { + let cursor_line_idx = doc.cursor_line(view.id); + + // do not render inline blame for empty lines to reduce visual noise + if doc.text().line(cursor_line_idx) != doc.line_ending.as_str() { + if let Ok(line_blame) = doc.line_blame(cursor_line_idx as u32, &inline_blame.format) + { + decorations.add_decoration(InlineBlame::new( + theme, + text_decorations::blame::LineBlame::OneLine((cursor_line_idx, line_blame)), + )); + }; + } + } else if inline_blame.behaviour == InlineBlameBehaviour::AllLines { + let text = doc.text(); + let text_line_count = text.len_lines(); + let view_height = view.inner_height(); + let first_visible_line = + text.char_to_line(doc.view_offset(view.id).anchor.min(text.len_chars())); + let first_line = first_visible_line.saturating_sub(view_height); + let last_line = first_visible_line + .saturating_add(view_height.saturating_mul(2)) + .min(text_line_count); + + let mut blame_lines = vec![None; text_line_count]; + + // Compute ~3 times the current view height of inline blame, that way some scrolling + // will not show half the view with inline blame and half without while still being faster + // than rendering inline blame for the full file. + let blame_for_all_lines = (first_line..last_line).filter_map(|line_idx| { + // do not render inline blame for empty lines to reduce visual noise + if text.line(line_idx) != doc.line_ending.as_str() { + doc.line_blame(line_idx as u32, &inline_blame.format) + .ok() + .map(|blame| (line_idx, blame)) + } else { + None + } + }); + + for (line_idx, blame) in blame_for_all_lines { + blame_lines[line_idx] = Some(blame); + } + decorations.add_decoration(InlineBlame::new( + theme, + text_decorations::blame::LineBlame::ManyLines(blame_lines), + )); + } + } + pub fn render_rulers( editor: &Editor, doc: &Document, From 082ba4d741896499388f4592e897165675819a7e Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 21:27:14 +0000 Subject: [PATCH 15/22] refactor: `match` over `if` --- helix-term/src/ui/editor.rs | 90 +++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 56fd04241..585107e15 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -254,53 +254,57 @@ impl EditorView { decorations: &mut DecorationManager, theme: &Theme, ) { - if inline_blame.behaviour == InlineBlameBehaviour::CursorLine { - let cursor_line_idx = doc.cursor_line(view.id); + match inline_blame.behaviour { + InlineBlameBehaviour::Hidden => (), + InlineBlameBehaviour::CursorLine => { + let cursor_line_idx = doc.cursor_line(view.id); - // do not render inline blame for empty lines to reduce visual noise - if doc.text().line(cursor_line_idx) != doc.line_ending.as_str() { - if let Ok(line_blame) = doc.line_blame(cursor_line_idx as u32, &inline_blame.format) - { - decorations.add_decoration(InlineBlame::new( - theme, - text_decorations::blame::LineBlame::OneLine((cursor_line_idx, line_blame)), - )); - }; - } - } else if inline_blame.behaviour == InlineBlameBehaviour::AllLines { - let text = doc.text(); - let text_line_count = text.len_lines(); - let view_height = view.inner_height(); - let first_visible_line = - text.char_to_line(doc.view_offset(view.id).anchor.min(text.len_chars())); - let first_line = first_visible_line.saturating_sub(view_height); - let last_line = first_visible_line - .saturating_add(view_height.saturating_mul(2)) - .min(text_line_count); - - let mut blame_lines = vec![None; text_line_count]; - - // Compute ~3 times the current view height of inline blame, that way some scrolling - // will not show half the view with inline blame and half without while still being faster - // than rendering inline blame for the full file. - let blame_for_all_lines = (first_line..last_line).filter_map(|line_idx| { // do not render inline blame for empty lines to reduce visual noise - if text.line(line_idx) != doc.line_ending.as_str() { - doc.line_blame(line_idx as u32, &inline_blame.format) - .ok() - .map(|blame| (line_idx, blame)) - } else { - None + if doc.text().line(cursor_line_idx) != doc.line_ending.as_str() { + if let Ok(line_blame) = doc.line_blame(cursor_line_idx as u32, &inline_blame.format) + { + decorations.add_decoration(InlineBlame::new( + theme, + text_decorations::blame::LineBlame::OneLine((cursor_line_idx, line_blame)), + )); + }; } - }); + }, + InlineBlameBehaviour::AllLines => { + let text = doc.text(); + let text_line_count = text.len_lines(); + let view_height = view.inner_height(); + let first_visible_line = + text.char_to_line(doc.view_offset(view.id).anchor.min(text.len_chars())); + let first_line = first_visible_line.saturating_sub(view_height); + let last_line = first_visible_line + .saturating_add(view_height.saturating_mul(2)) + .min(text_line_count); - for (line_idx, blame) in blame_for_all_lines { - blame_lines[line_idx] = Some(blame); - } - decorations.add_decoration(InlineBlame::new( - theme, - text_decorations::blame::LineBlame::ManyLines(blame_lines), - )); + let mut blame_lines = vec![None; text_line_count]; + + // Compute ~3 times the current view height of inline blame, that way some scrolling + // will not show half the view with inline blame and half without while still being faster + // than rendering inline blame for the full file. + let blame_for_all_lines = (first_line..last_line).filter_map(|line_idx| { + // do not render inline blame for empty lines to reduce visual noise + if text.line(line_idx) != doc.line_ending.as_str() { + doc.line_blame(line_idx as u32, &inline_blame.format) + .ok() + .map(|blame| (line_idx, blame)) + } else { + None + } + }); + + for (line_idx, blame) in blame_for_all_lines { + blame_lines[line_idx] = Some(blame); + } + decorations.add_decoration(InlineBlame::new( + theme, + text_decorations::blame::LineBlame::ManyLines(blame_lines), + )); + }, } } From c101f372988e91756959c75960294a2340a4acef Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 21:31:08 +0000 Subject: [PATCH 16/22] style: fmt --- helix-term/src/ui/editor.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 585107e15..16f7d0635 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -261,15 +261,19 @@ impl EditorView { // do not render inline blame for empty lines to reduce visual noise if doc.text().line(cursor_line_idx) != doc.line_ending.as_str() { - if let Ok(line_blame) = doc.line_blame(cursor_line_idx as u32, &inline_blame.format) + if let Ok(line_blame) = + doc.line_blame(cursor_line_idx as u32, &inline_blame.format) { decorations.add_decoration(InlineBlame::new( theme, - text_decorations::blame::LineBlame::OneLine((cursor_line_idx, line_blame)), + text_decorations::blame::LineBlame::OneLine(( + cursor_line_idx, + line_blame, + )), )); }; } - }, + } InlineBlameBehaviour::AllLines => { let text = doc.text(); let text_line_count = text.len_lines(); @@ -300,11 +304,12 @@ impl EditorView { for (line_idx, blame) in blame_for_all_lines { blame_lines[line_idx] = Some(blame); } + decorations.add_decoration(InlineBlame::new( theme, text_decorations::blame::LineBlame::ManyLines(blame_lines), )); - }, + } } } From b3b1c88d27a634ea8423f02b71658adbdea9bdad Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 21:34:06 +0000 Subject: [PATCH 17/22] refactor: pass the `Style` instead of `Theme` --- helix-term/src/ui/editor.rs | 5 +++-- helix-term/src/ui/text_decorations/blame.rs | 5 ++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 16f7d0635..32a2a8a0f 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -254,6 +254,7 @@ impl EditorView { decorations: &mut DecorationManager, theme: &Theme, ) { + const INLINE_BLAME_SCOPE: &str = "ui.virtual.inline-blame"; match inline_blame.behaviour { InlineBlameBehaviour::Hidden => (), InlineBlameBehaviour::CursorLine => { @@ -265,7 +266,7 @@ impl EditorView { doc.line_blame(cursor_line_idx as u32, &inline_blame.format) { decorations.add_decoration(InlineBlame::new( - theme, + theme.get(INLINE_BLAME_SCOPE), text_decorations::blame::LineBlame::OneLine(( cursor_line_idx, line_blame, @@ -306,7 +307,7 @@ impl EditorView { } decorations.add_decoration(InlineBlame::new( - theme, + theme.get(INLINE_BLAME_SCOPE), text_decorations::blame::LineBlame::ManyLines(blame_lines), )); } diff --git a/helix-term/src/ui/text_decorations/blame.rs b/helix-term/src/ui/text_decorations/blame.rs index ada2b261f..372666597 100644 --- a/helix-term/src/ui/text_decorations/blame.rs +++ b/helix-term/src/ui/text_decorations/blame.rs @@ -1,7 +1,6 @@ use helix_core::Position; use helix_view::theme::Style; -use helix_view::Theme; use crate::ui::document::{LinePos, TextRenderer}; use crate::ui::text_decorations::Decoration; @@ -20,9 +19,9 @@ pub struct InlineBlame { } impl InlineBlame { - pub fn new(theme: &Theme, lines: LineBlame) -> Self { + pub fn new(style: Style, lines: LineBlame) -> Self { InlineBlame { - style: theme.get("ui.virtual.inline-blame"), + style, lines, } } From af3b670de6c92475a07390b0d4d1a38fdb2274f4 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Tue, 25 Mar 2025 21:35:45 +0000 Subject: [PATCH 18/22] refactor: move expression _ --- helix-term/src/ui/editor.rs | 3 +-- helix-term/src/ui/text_decorations/blame.rs | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 32a2a8a0f..e23ccb3f5 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -179,6 +179,7 @@ impl EditorView { } Self::render_rulers(editor, doc, view, inner, surface, theme); + Self::render_inline_blame(&config.inline_blame, doc, view, &mut decorations, theme); let primary_cursor = doc .selection(view.id) @@ -204,8 +205,6 @@ impl EditorView { config.end_of_line_diagnostics, )); - Self::render_inline_blame(&config.inline_blame, doc, view, &mut decorations, theme); - render_document( surface, inner, diff --git a/helix-term/src/ui/text_decorations/blame.rs b/helix-term/src/ui/text_decorations/blame.rs index 372666597..645a0462c 100644 --- a/helix-term/src/ui/text_decorations/blame.rs +++ b/helix-term/src/ui/text_decorations/blame.rs @@ -20,10 +20,7 @@ pub struct InlineBlame { impl InlineBlame { pub fn new(style: Style, lines: LineBlame) -> Self { - InlineBlame { - style, - lines, - } + InlineBlame { style, lines } } } From 95344a958506069af6b05f8142f20eb245574189 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Sat, 29 Mar 2025 23:59:06 +0000 Subject: [PATCH 19/22] perf: use string preallocations for string concatenation --- helix-stdx/src/lib.rs | 1 + helix-stdx/src/str.rs | 18 ++++++++++++++++++ helix-stdx/src/time.rs | 2 +- 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 helix-stdx/src/str.rs diff --git a/helix-stdx/src/lib.rs b/helix-stdx/src/lib.rs index eeaba0d81..7c0f25cf0 100644 --- a/helix-stdx/src/lib.rs +++ b/helix-stdx/src/lib.rs @@ -3,6 +3,7 @@ pub mod faccess; pub mod path; pub mod range; pub mod rope; +pub mod str; pub mod time; pub use range::Range; diff --git a/helix-stdx/src/str.rs b/helix-stdx/src/str.rs new file mode 100644 index 000000000..e79cbcccc --- /dev/null +++ b/helix-stdx/src/str.rs @@ -0,0 +1,18 @@ +/// Concatenates strings together. +/// +/// `concat!(a, " ", b, " ", c)` is: +/// - more performant than `format!("{a} {b} {c}")` +/// - more ergonomic than using `String::with_capacity` followed by a series of `String::push_str` +#[macro_export] +macro_rules! concat { + ($($value:expr),*) => {{ + // Rust does not allow using `+` as separator between value + // so we must add that at the end of everything. The `0` is necessary + // at the end so it does not end with "+ " (which would be invalid syntax) + let mut buf = String::with_capacity($($value.len() + )* 0); + $( + buf.push_str(&$value); + )* + buf + }} +} diff --git a/helix-stdx/src/time.rs b/helix-stdx/src/time.rs index cb72a9386..4df88c45c 100644 --- a/helix-stdx/src/time.rs +++ b/helix-stdx/src/time.rs @@ -71,5 +71,5 @@ pub fn format_relative_time(timestamp: i64, timezone_offset: i32) -> String { "from now" }; - format!("{value} {unit} {label}") + crate::concat!(value, " ", unit, " ", label) } From 1a0dad36b75e95e1b264536c5a5696186efece17 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Sun, 30 Mar 2025 11:42:43 +0100 Subject: [PATCH 20/22] perf: only render inline blame for visible lines when `all-lines` is set Previously, we rendereded the inline blame for lines in 3X the range of the viewport This is not necessary because when we scroll down, the rendering will occur before we see the new content --- helix-term/src/ui/editor.rs | 20 ++++---------------- helix-view/src/view.rs | 12 ++++++++++++ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index e23ccb3f5..0e40775e1 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -254,13 +254,14 @@ impl EditorView { theme: &Theme, ) { const INLINE_BLAME_SCOPE: &str = "ui.virtual.inline-blame"; + let text = doc.text(); match inline_blame.behaviour { InlineBlameBehaviour::Hidden => (), InlineBlameBehaviour::CursorLine => { let cursor_line_idx = doc.cursor_line(view.id); // do not render inline blame for empty lines to reduce visual noise - if doc.text().line(cursor_line_idx) != doc.line_ending.as_str() { + if text.line(cursor_line_idx) != doc.line_ending.as_str() { if let Ok(line_blame) = doc.line_blame(cursor_line_idx as u32, &inline_blame.format) { @@ -275,22 +276,9 @@ impl EditorView { } } InlineBlameBehaviour::AllLines => { - let text = doc.text(); - let text_line_count = text.len_lines(); - let view_height = view.inner_height(); - let first_visible_line = - text.char_to_line(doc.view_offset(view.id).anchor.min(text.len_chars())); - let first_line = first_visible_line.saturating_sub(view_height); - let last_line = first_visible_line - .saturating_add(view_height.saturating_mul(2)) - .min(text_line_count); + let mut blame_lines = vec![None; text.len_lines()]; - let mut blame_lines = vec![None; text_line_count]; - - // Compute ~3 times the current view height of inline blame, that way some scrolling - // will not show half the view with inline blame and half without while still being faster - // than rendering inline blame for the full file. - let blame_for_all_lines = (first_line..last_line).filter_map(|line_idx| { + let blame_for_all_lines = view.line_range(doc).filter_map(|line_idx| { // do not render inline blame for empty lines to reduce visual noise if text.line(line_idx) != doc.line_ending.as_str() { doc.line_blame(line_idx as u32, &inline_blame.format) diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index d6f10753a..b4863700d 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -190,6 +190,18 @@ impl View { self.docs_access_history.push(id); } + /// The range of lines in the document that the view sees + pub fn line_range(&self, doc: &Document) -> std::ops::Range { + let text = doc.text(); + let text_line_count = text.len_lines(); + let first_line = text.char_to_line(doc.view_offset(self.id).anchor.min(text.len_chars())); + let last_line = first_line + .saturating_add(self.inner_height()) + .min(text_line_count); + + first_line..last_line + } + pub fn inner_area(&self, doc: &Document) -> Rect { self.area.clip_left(self.gutter_offset(doc)).clip_bottom(1) // -1 for statusline } From e8d7e76c737396d002abc0eafcda5f8449a0db95 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+nik-rev@users.noreply.github.com> Date: Tue, 1 Apr 2025 12:17:48 +0100 Subject: [PATCH 21/22] fix: spelling error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sebastian Klähn <39526136+Septias@users.noreply.github.com> --- helix-vcs/src/git/blame.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-vcs/src/git/blame.rs b/helix-vcs/src/git/blame.rs index 0b83bd1d8..c670b21ee 100644 --- a/helix-vcs/src/git/blame.rs +++ b/helix-vcs/src/git/blame.rs @@ -30,7 +30,7 @@ pub struct FileBlame { } impl FileBlame { - /// Get the blame information corresponing to a line in file and diff for that line + /// Get the blame information corresponding to a line in the file and diff for that line #[inline] pub fn blame_for_line(&self, line: u32, inserted_lines: u32, removed_lines: u32) -> LineBlame { // Because gix_blame doesn't care about stuff that is not commited, we have to "normalize" the From c74fec4c6e77972da36e1c929bada17d3a643f34 Mon Sep 17 00:00:00 2001 From: Nik Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Wed, 2 Apr 2025 00:12:38 +0100 Subject: [PATCH 22/22] fix?: do not block on the main thread when acquiring diff handle not sure if this will work as I can't reproduce this but let's see! --- helix-vcs/src/diff.rs | 7 +++++++ helix-view/src/document.rs | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/helix-vcs/src/diff.rs b/helix-vcs/src/diff.rs index e49e171dd..95e13b594 100644 --- a/helix-vcs/src/diff.rs +++ b/helix-vcs/src/diff.rs @@ -75,6 +75,13 @@ impl DiffHandle { } } + pub fn try_load(&self) -> Option { + Some(Diff { + diff: self.diff.try_read()?, + inverted: self.inverted, + }) + } + /// Updates the document associated with this redraw handle /// This function is only intended to be called from within the rendering loop /// if called from elsewhere it may fail to acquire the render lock and panic diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index d0007a1fd..c99a4e75f 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1590,7 +1590,7 @@ impl Document { // file in the file system into what gix::blame knows about (gix::blame only // knows about commit history, it does not know about uncommitted changes) diff_handle - .load() + .try_load()? .hunks_intersecting_line_ranges(std::iter::once((0, cursor_line as usize))) .try_fold( (0, 0),