Skip to content

Conversation

nik-rev
Copy link
Contributor

@nik-rev nik-rev commented Mar 18, 2025

This PR adds Inline Git Blame.

While moving around in the editor, virtual text will appear to the right showing the latest commit information for this line. Showcase:

image

  • The git blame output is very customizable. See below.
  • There is a keybinding space + B to show blame for the current line in the status line. Useful if you don't have inline-blame.enable and just want to blame a single line

Closes #3035

Documentation

[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
show Choose when to show inline blame "never"
auto-fetch Choose when inline blame should be computed false
format The format in which to show the inline blame "{author}, {time-ago} • {title} • {commit}"

show can be one of the following:

  • "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 hidden.

Inline blame will only show if the blame for the file has already been fetched.

The auto-fetch key determines under which circumstances the blame is fetched, and can be one of the following:

  • false: Blame for the file is fetched 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 show set to "all-lines" or "cursor-line", the inline blame won't show. It needs to be fetched first in order to become available, which can be triggered manually with space + B.

  • true: Blame for the file is fetched 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.

Set a format string for format to customize the blame message displayed. Variables are text placeholders wrapped in curly braces: {variable}.

The following variables are available:

  • author: The author of the commit
  • date: When the commit was made
  • time-ago: How long ago the commit was made
  • title: The title of the commit
  • body: The body of the commit
  • commit: The short hex SHA1 hash of the commit
  • email: The email of the author of the commit

Notes for the reviewer

  • helix-term/src/handlers/blame.rs: I created a new BlameHandler that requests that a document's blame field is updated. This field has the information on which line numbers represent a specific commit
  • helix-term/src/ui/text_decrations/blame.rs: This module hosts a Decoration impl that is responsible for drawing a string (the blame output) at the end of a given line
  • helix-vcs/src/git/blame.rs: Holds the FileBlame struct that contains the logic on how blame is retrieved for a file and a specific line in the document, as well as how this blame is formatted into a string.

Run this PR using --release mode otherwise you may run into a panic caused by a failing debug_assert! in gix_blame. This was tested to not have any user impact. The issue for this is known: GitoxideLabs/gitoxide#1847

@Axlefublr
Copy link
Contributor

that's really neat! good job :3

@nik-rev nik-rev marked this pull request as ready for review March 18, 2025 19:38
Copy link
Contributor

@thomasaarholt thomasaarholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, I've been wanting this for a long time, and tried looking into it two weeks ago, but my rust skills just weren't up for it!

I wonder if opening the repo is the slow step here, which could mean that the following isn't necessary, see my comment below.

Could it would make sense to get the blame for the whole file at once instead of doing it on-demand for every line? That would be much faster, as you could just look up the stored info of what the blame is for a given line. But then we would ideally need to keep count of added/deleted lines in order to "keep count" when querying for the currently selected line.

The current behavior of this PR is basically that though - if I type something on a new line, the blame mentions the previous author, not me/"Not comitted yet" (not even after save, which is interesting, but beside my point) and the count is off, I think.

I notice that the blame info is computed on moving left and right. That isn't necessary.

@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 18, 2025

Awesome work, I've been wanting this for a long time, and tried looking into it two weeks ago, but my rust skills just weren't up for it!

I wonder if opening the repo is the slow step here, which could mean that the following isn't necessary, see my comment below.

Could it would make sense to get the blame for the whole file at once instead of doing it on-demand for every line? That would be much faster, as you could just look up the stored info of what the blame is for a given line. But then we would ideally need to keep count of added/deleted lines in order to "keep count" when querying for the currently selected line.

The current behavior of this PR is basically that though - if I type something on a new line, the blame mentions the previous author, not me/"Not comitted yet" (not even after save, which is interesting, but beside my point) and the count is off, I think.

I notice that the blame info is computed on moving left and right. That isn't necessary.

Stuff like not updating when moving left and right, or get the diff for the files are definitely optimizations we can look into

But the biggest one is figuring out how to compute the blame off the main thread. Even if you compute it just once per file, it can take 10 seconds for that to happen and the UI will be frozen in that time which isn't acceptable

@nik-rev nik-rev marked this pull request as draft March 18, 2025 21:39
@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 18, 2025

if I type something on a new line, the blame mentions the previous author, not me/"Not comitted yet"

Yeah this shouldn't be difficult to fix, once I figure out how to stop the freezes I'll also implement this

@noahfraiture
Copy link

Hey this looks great thank you ! Would it be possible to map it on a key press to show ? We'd avoid performing the computation every time the cursor change line, and it would reduced the visual pollution

@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 18, 2025

Hey this looks great thank you ! Would it be possible to map it on a key press to show ? We'd avoid performing the computation every time the cursor change line, and it would reduced the visual pollution

Yes, that is possible to do however the UI would still freeze for e.g. 10 seconds which isn't really good

edit: this message doesn't apply anymore, the problem was fixed. I implemented the keybinding to get the git blame for the current line

@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 18, 2025

YES! I finally figured it out. Now we're not doing the git blame on the main thread anymore :)

The solution is basically that we have a worker and we check it every 0.05 seconds if its done. If not, then try again later. If it is, then get its result.

The reason why this works now is that I am not doing any computation while having a &mut Editor. All work is done when I don't have it

@nik-rev nik-rev marked this pull request as ready for review March 18, 2025 23:53
@nik-rev nik-rev marked this pull request as draft March 19, 2025 00:22
@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 19, 2025

Hey this looks great thank you ! Would it be possible to map it on a key press to show ? We'd avoid performing the computation every time the cursor change line, and it would reduced the visual pollution

I think having a default keybinding to trigger this once is a good idea. Will add that to the TODO

edit: implemented

@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 19, 2025

It looks like this with Inlay Hints, and end of line diagnostics

image

@nik-rev nik-rev force-pushed the gix-blame branch 4 times, most recently from 54a0c02 to f830c76 Compare March 19, 2025 23:17
@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 19, 2025

Update:

  • Now the PR takes uncommitted lines into account.
  • Added several tests.
  • You can now customize the inline git blame using a string with variables.
  • Added a static command line_blame bound for key <space>B. This shows the blame for the current line using your config.

Customizing the inline git blame

[editor.version-control] Section

Key Description Default
inline-blame Show git blame output for the current line false
inline-blame-format The format in which to show the inline blame "{author}, {date} • {message} • {commit}"

For inline-blame-format, you can use specific variables like so: {variable}.

These are the available variables:

  • author: The author of the commit
  • date: When 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 inline-blame-format value:

  • If author is empty: "{date} • {message} • {commit}"
  • If date is empty: "{author} • {message} • {commit}"
  • If message is empty: "{author}, {date} • {commit}"
  • If commit is empty: "{author}, {date} • {message}"
  • If date and message is empty: "{author} • {commit}"
  • If author and message is empty: "{date} • {commit}"

Tests

side note: For the tests, probably wrote my most complicated declarative macro ever, but at least we can have easily readable test syntax for git blame:

Test Syntax for Git Blame
#[test]
pub fn blamed_lines() {
    assert_line_blame_progress! {
        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
        5 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
        6 no_commit =>
            "  six" insert,
            "  three" insert,
            "fn main() {" 1,
            "  five" insert,
            "  four" insert,
            "  two" 3,
            "  five" insert,
            "  four" insert,
            "}" 1,
            "  five" insert,
            "  four" insert;
    };
}

@daedroza
Copy link
Contributor

Would it be possible to open the git blame into a scratch buffer with git diff language style applied?

@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 20, 2025

Would it be possible to open the git blame into a scratch buffer with git diff language style applied?

it should be doable, but it would be out of scope for this PR.
Something like Lazygit would be more suited for seeing the git blame for the entire file

@thomasaarholt
Copy link
Contributor

I think the following could be desirable, but don't worry if you feel it is out of scope:

  • yank the blame commit
  • yank the formatted blame string
  • yank the blame commit url (this is harder, and can be repo-provider-dependent. I wrote a bash script last week for this, see my comment here and bash script here.
  • if getting the url is possible, add an option for adding a link to the blame formatted output.
  • maaaybe open-blame-url that directly opens in the browser. Not sure if there is any precedent in the helix project for opening urls programmatically (e.g. xdg-open).

@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 20, 2025

I think the following could be desirable, but don't worry if you feel it is out of scope:

* yank the blame commit

* yank the formatted blame string

* yank the blame commit url (this is harder, and can be repo-provider-dependent. I wrote a bash script last week for this, see my comment [here](https://github.com/helix-editor/helix/discussions/6421#discussioncomment-12502299) and [bash script here](https://gist.github.com/thomasaarholt/2659407306e83edde4262a398aaf9f43#file-blame-L125).

* if getting the url is possible, add an option for adding a link to the blame formatted output.

* maaaybe `open-blame-url` that directly opens in the browser. Not sure if there is any precedent in the helix project for opening urls programmatically (e.g. `xdg-open`).

for 5., there is gf which can open links iirc so Helix can definitely do that

That being said, these items would be good to add but I don't want to put everything into 1 PR as that just means it could take a very long time to get reviewed. I would rather just have the core functionality for now and then make a new PR to add the niceties

@thomasaarholt
Copy link
Contributor

thomasaarholt commented Mar 20, 2025

I got this panic when using the latest commit, on doing :log-open and moving the cursor one line down:

❯ hx .
thread 'main' panicked at helix-term/src/handlers/blame.rs:96:39:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0:     0x5610edff724b - std::backtrace_rs::backtrace::libunwind::trace::hbee8a7973eeb6c93
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/libunwind.rs:104:5
   1:     0x5610edff724b - std::backtrace_rs::backtrace::trace_unsynchronized::hc8ac75eea3aa6899
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x5610edff724b - std::sys_common::backtrace::_print_fmt::hc7f3e3b5298b1083
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:68:5
   3:     0x5610edff724b - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hbb235daedd7c6190
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x5610ed0d95a0 - core::fmt::rt::Argument::fmt::h76c38a80d925a410
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/rt.rs:142:9
   5:     0x5610ed0d95a0 - core::fmt::write::h3ed6aeaa977c8e45
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/mod.rs:1120:17
   6:     0x5610edff2fd3 - std::io::Write::write_fmt::h78b18af5775fedb5
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/io/mod.rs:1810:15
   7:     0x5610edff6fe4 - std::sys_common::backtrace::_print::h5d645a07e0fcfdbb
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x5610edff6fe4 - std::sys_common::backtrace::print::h85035a511aafe7a8
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x5610edff9560 - std::panicking::default_hook::{{closure}}::hcce8cea212785a25
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:272:22
  10:     0x5610edff927f - std::panicking::default_hook::hf5fcb0f213fe709a
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:292:9
  11:     0x5610edff9c2f - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::hbc5ccf4eb663e1e5
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/alloc/src/boxed.rs:2029:9
  12:     0x5610edff9c2f - std::panicking::rust_panic_with_hook::h095fccf1dc9379ee
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:783:13
  13:     0x5610edff9949 - std::panicking::begin_panic_handler::{{closure}}::h032ba12139b353db
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:649:13
  14:     0x5610edff7746 - std::sys_common::backtrace::__rust_end_short_backtrace::h9259bc2ff8fd0f76
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:171:18
  15:     0x5610edff96f0 - rust_begin_unwind
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
  16:     0x5610ecfc3855 - core::panicking::panic_fmt::h784f20a50eaab275
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
  17:     0x5610ecfc3913 - core::panicking::panic::hb837a5ebbbe5b188
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:144:5
  18:     0x5610ed98486a - helix_event::hook::ErasedHook::new::call::h11ad64f2206d9be2
  19:     0x5610edad813b - helix_event::registry::with::hd723a5bf64ad0323
  20:     0x5610ed727b0e - helix_term::ui::editor::EditorView::handle_keymap_event::{{closure}}::h0acda43659dfd283
  21:     0x5610ed727a15 - helix_term::ui::editor::EditorView::handle_keymap_event::hfea1af47727d2306
  22:     0x5610ed729492 - <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::handle_event::h108e44023d161bfe
  23:     0x5610ed7682a5 - helix_term::compositor::Compositor::handle_event::h49983d2fd645b11b
  24:     0x5610edd773f7 - hx::main_impl::{{closure}}::h05533ca0f2469032
  25:     0x5610edd75588 - tokio::runtime::park::CachedParkThread::block_on::h7f045a3adc92d673
  26:     0x5610edddda09 - tokio::runtime::context::runtime::enter_runtime::h7417e5403934ecd3
  27:     0x5610ede0574f - tokio::runtime::runtime::Runtime::block_on::h55b4bf63cf9912d5
  28:     0x5610eddbbd42 - hx::main::ha7b9158d45812ad4
  29:     0x5610eddec8a3 - std::sys_common::backtrace::__rust_begin_short_backtrace::h3fa1501b4bd3efc7
  30:     0x5610eddec9dd - std::rt::lang_start::{{closure}}::h6bfd917f3e71d59c
  31:     0x5610edfe9955 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h37600b1e5eea4ecd
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:284:13
  32:     0x5610edfe9955 - std::panicking::try::do_call::hb4bda49fa13a0c2b
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  33:     0x5610edfe9955 - std::panicking::try::h8bbf75149211aaaa
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  34:     0x5610edfe9955 - std::panic::catch_unwind::h8c78ec68ebea34cb
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  35:     0x5610edfe9955 - std::rt::lang_start_internal::{{closure}}::hffdf44a19fd9e220
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:48
  36:     0x5610edfe9955 - std::panicking::try::do_call::hcb3194972c74716d
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  37:     0x5610edfe9955 - std::panicking::try::hcdc6892c5f0dba4c
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  38:     0x5610edfe9955 - std::panic::catch_unwind::h4910beb4573f4776
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  39:     0x5610edfe9955 - std::rt::lang_start_internal::h6939038e2873596b
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:20
  40:     0x5610eddbbe35 - main
  41:     0x7fdaacb951ca - <unknown>
  42:     0x7fdaacb9528b - __libc_start_main
  43:     0x5610ed050625 - _start
  44:                0x0 - <unknown>

@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 20, 2025

I got this panic when using the latest commit, on doing :log-open and moving the cursor one line down:

❯ hx .
thread 'main' panicked at helix-term/src/handlers/blame.rs:96:39:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0:     0x5610edff724b - std::backtrace_rs::backtrace::libunwind::trace::hbee8a7973eeb6c93
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/libunwind.rs:104:5
   1:     0x5610edff724b - std::backtrace_rs::backtrace::trace_unsynchronized::hc8ac75eea3aa6899
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x5610edff724b - std::sys_common::backtrace::_print_fmt::hc7f3e3b5298b1083
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:68:5
   3:     0x5610edff724b - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hbb235daedd7c6190
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x5610ed0d95a0 - core::fmt::rt::Argument::fmt::h76c38a80d925a410
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/rt.rs:142:9
   5:     0x5610ed0d95a0 - core::fmt::write::h3ed6aeaa977c8e45
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/mod.rs:1120:17
   6:     0x5610edff2fd3 - std::io::Write::write_fmt::h78b18af5775fedb5
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/io/mod.rs:1810:15
   7:     0x5610edff6fe4 - std::sys_common::backtrace::_print::h5d645a07e0fcfdbb
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x5610edff6fe4 - std::sys_common::backtrace::print::h85035a511aafe7a8
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x5610edff9560 - std::panicking::default_hook::{{closure}}::hcce8cea212785a25
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:272:22
  10:     0x5610edff927f - std::panicking::default_hook::hf5fcb0f213fe709a
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:292:9
  11:     0x5610edff9c2f - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::hbc5ccf4eb663e1e5
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/alloc/src/boxed.rs:2029:9
  12:     0x5610edff9c2f - std::panicking::rust_panic_with_hook::h095fccf1dc9379ee
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:783:13
  13:     0x5610edff9949 - std::panicking::begin_panic_handler::{{closure}}::h032ba12139b353db
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:649:13
  14:     0x5610edff7746 - std::sys_common::backtrace::__rust_end_short_backtrace::h9259bc2ff8fd0f76
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:171:18
  15:     0x5610edff96f0 - rust_begin_unwind
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
  16:     0x5610ecfc3855 - core::panicking::panic_fmt::h784f20a50eaab275
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
  17:     0x5610ecfc3913 - core::panicking::panic::hb837a5ebbbe5b188
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:144:5
  18:     0x5610ed98486a - helix_event::hook::ErasedHook::new::call::h11ad64f2206d9be2
  19:     0x5610edad813b - helix_event::registry::with::hd723a5bf64ad0323
  20:     0x5610ed727b0e - helix_term::ui::editor::EditorView::handle_keymap_event::{{closure}}::h0acda43659dfd283
  21:     0x5610ed727a15 - helix_term::ui::editor::EditorView::handle_keymap_event::hfea1af47727d2306
  22:     0x5610ed729492 - <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::handle_event::h108e44023d161bfe
  23:     0x5610ed7682a5 - helix_term::compositor::Compositor::handle_event::h49983d2fd645b11b
  24:     0x5610edd773f7 - hx::main_impl::{{closure}}::h05533ca0f2469032
  25:     0x5610edd75588 - tokio::runtime::park::CachedParkThread::block_on::h7f045a3adc92d673
  26:     0x5610edddda09 - tokio::runtime::context::runtime::enter_runtime::h7417e5403934ecd3
  27:     0x5610ede0574f - tokio::runtime::runtime::Runtime::block_on::h55b4bf63cf9912d5
  28:     0x5610eddbbd42 - hx::main::ha7b9158d45812ad4
  29:     0x5610eddec8a3 - std::sys_common::backtrace::__rust_begin_short_backtrace::h3fa1501b4bd3efc7
  30:     0x5610eddec9dd - std::rt::lang_start::{{closure}}::h6bfd917f3e71d59c
  31:     0x5610edfe9955 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h37600b1e5eea4ecd
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:284:13
  32:     0x5610edfe9955 - std::panicking::try::do_call::hb4bda49fa13a0c2b
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  33:     0x5610edfe9955 - std::panicking::try::h8bbf75149211aaaa
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  34:     0x5610edfe9955 - std::panic::catch_unwind::h8c78ec68ebea34cb
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  35:     0x5610edfe9955 - std::rt::lang_start_internal::{{closure}}::hffdf44a19fd9e220
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:48
  36:     0x5610edfe9955 - std::panicking::try::do_call::hcb3194972c74716d
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  37:     0x5610edfe9955 - std::panicking::try::hcdc6892c5f0dba4c
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  38:     0x5610edfe9955 - std::panic::catch_unwind::h4910beb4573f4776
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  39:     0x5610edfe9955 - std::rt::lang_start_internal::h6939038e2873596b
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:20
  40:     0x5610eddbbe35 - main
  41:     0x7fdaacb951ca - <unknown>
  42:     0x7fdaacb9528b - __libc_start_main
  43:     0x5610ed050625 - _start
  44:                0x0 - <unknown>

Ah yes, I just fixed it

@noahfraiture
Copy link

noahfraiture commented Mar 20, 2025

I only see the commits and comments, but even if it doesn't affect the main thread, the performance is still a critical point, helix should stay light. Is a update every 150milliseconds necessary ?

@nik-rev
Copy link
Contributor Author

nik-rev commented Mar 20, 2025

I only see the commits and comments, but even if it doesn't affect the main thread, the performance is still a critical point, helix should stay light. Is a update every 150milliseconds necessary ?

Update: We don't do updates every N seconds anymore

Git blame for every line whenever you move is expensive especially on a large repo, so it's guaranteed Helix will consume more resources

I'll try my best to improve performance. I hope to at the very least achieve the performance that Zed has for its inline git blame implementation. It should be doable because from what I can tell Zed spawns a process for git blame while here we are using gix_blame crate which claims to be significantly faster

So far, I've done some experimentation on the Helix repo itself which has around 6,000 commits. Specifically these are the steps that run in the background every time we request the git blame

getting repo dir: 992ns
opening repo: 450.24µs
getting suspect: 119.356µs
creating commit traversal: 212.905224ms
diff resource cache for tree diff: 410.095µs
blame_line: 339.222459ms

Originally I was going to focus on caching the opening repo step however this is negligible in comparison to the time it takes to:

  • create commit traversal (0.21s) (which we should be able to cache for huge performance increase)
  • blame_line (0.34s). This step is out of our control, it is implemented by gix_blame crate. So we could think about how we can efficiently re-use git blame for lines when it was already computed

Co-authored-by: uncenter <[email protected]>
@NSPC911
Copy link

NSPC911 commented May 19, 2025

9uhf81
literally my reaction every time i see a commit

This reverts commit 41cb919.

I don't think it's worth to have this feature for the additional
complexity of allowing users to opt-out, plus considering the fact that
it might be inaccurate and cause confusion

Similar discussion: zed-industries/zed#10557
@jcmuller
Copy link
Contributor

This is looking great!
Quick question: does gitoxide support .git-blame-ignore-revs?

@cruessler
Copy link

This is looking great! Quick question: does gitoxide support .git-blame-ignore-revs?

Not yet. 😄

@eidellev
Copy link

I've been test-driving this PR for the last couple of weeks and it works great! what are the odds it gets merged any time soon?

@ahoneybun
Copy link

Damn sadly this didn't make it into the new release 😿

@alecmerdler
Copy link

This would be a neat addition. I was interested in seeing what the experience would be to have this feature, so I took the time to test it. I checked out this branch and built it (using the --release flag, as instructed) then opened Helix on this repo and it crashed. This was both with auto-fetch = true and auto-fetch = false.

@nik-rev
Copy link
Contributor Author

nik-rev commented Jul 16, 2025

This would be a neat addition. I was interested in seeing what the experience would be to have this feature, so I took the time to test it. I checked out this branch and built it (using the --release flag, as instructed) then opened Helix on this repo and it crashed. This was both with auto-fetch = true and auto-fetch = false.

I need to be able to reproduce your crash to be able to fix it

  • what was the repo?
  • what steps did you take?
  • what was the panic message?
  • what are the contents of the helix log file?

@noahfraiture
Copy link

Could we consider merging this PR? It appears to be stable for a few weeks, and the only comment mentioning a crash did not provide any further details, and it seems unreproducible. I believe it would be reasonable to merge it to master. If there is indeed an issue, it will be detected before the next release. It will always be possible to improve the feature later.

However, this is merely my opinion as a non-maintainer. If the maintainers have explicitly decided not to merge this PR, I completely understand their decision.

@ahoneybun
Copy link

Could we consider merging this PR? It appears to be stable for a few weeks, and the only comment mentioning a crash did not provide any further details, and it seems unreproducible. I believe it would be reasonable to merge it to master. If there is indeed an issue, it will be detected before the next release. It will always be possible to improve the feature later.

However, this is merely my opinion as a non-maintainer. If the maintainers have explicitly decided not to merge this PR, I completely understand their decision.

I agree, I think it needs to be rebased and should be good to go? I would love this feature!

gj1118 added a commit to gj1118/helix that referenced this pull request Aug 17, 2025
gj1118 added a commit to gj1118/helix that referenced this pull request Aug 17, 2025
dgoerger pushed a commit to dgoerger/dotfiles that referenced this pull request Aug 20, 2025
This is a workaround until/unless upstream pull request #13133 (feat:
Inline Git Blame) is merged:

https: //github.com/helix-editor/helix/pull/13133
Change-Id: Id3567787b5dcb1c0b876daccc40c7b15c6bbee57
Reviewed-on: https://review.daemonica.net/c/dgoerger/dotfiles/+/1185
Reviewed-by: David Goerger <[email protected]>
@noahfraiture
Copy link

@the-mikedavis Hey, I heard that the way the pipeline works might get maintainers to not be notified of the state of a PR. Is there something we can do to get this PR merged ?

@the-mikedavis
Copy link
Member

This PR isn't lost, it just needs us to find time to review it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git blame and URL