Feat: Sticky Context #6118

SoraTenshi · 2023-02-27T09:13:55Z

!! If your preferred colorscheme / language is not yet in this PR, please feel free to open a PR on my branch: `sticky-context` to add those! It helps me quite a lot, as i can't keep track of every single colorscheme / language that helix supports !!

This is supposed to be the "successor PR" of #3944 .

Since @matoous has no time to pick up on it, i have started to continue on it (as i have also previously worked a bit on it!).

This is for now working, but with very rough edges and needs some fine adjustments.

Features, in no particular order:

----- Bug Smashing: ------

Fix bug where nodes jitter
Redo the context.end logic, work with parameter captures instead of from func decl to end, there should be a way capture the byte range of all the parameters.
Fix the "line shrinking" logic when the cursor gets on a sticky node
Improve overall calculation performance
Figure out a way to improve calculation performance if cache is empty (currently tanks too much)
Somewhere, cache invalidation is still not correct
In some situations, unnecessary many nodes are rendered
Somewhere there's a crash (can't repr though)

I beg anyone to find another bug as mentioned here, to immediately notify me, even if it's minor, i am happy to investigate it (also; if possible, create a reproducer)

----- Overview: ------

The nodes are rendered by the following rules:
- The last node, if configured, is always the indicator
- Prioritize closer nodes more than much further nodes (e.g. functions are more prioritized than e.g. impl of structs
of 1 tree-sitter node, always the 1st line is taken, even if the node is let's say 3 lines long.
The sticky context may not render at all when the viewport is too small. Yes i tried to keep it as dynamic as possible, but at some point it's getting too small
Eventually it would be very beneficial, if me or someone else in the future can make argument on multi-line function declarations collapseable (e.g.

fn render(
	i: i32,
	x: u32,
	y: usize,
	z: Option<void>,
) {

should then be collapsed to:
fn render(...) {

Small gif:

closes #396

SoraTenshi · 2023-02-28T20:02:20Z

here is a preview of how it looks like at the moment.
With the new changes, a lot of things became much easier than expected.

SoraTenshi · 2023-03-01T09:55:34Z

i think in order to satisfy clippys complaint, i would have to use the #[allow(clippy::too_many_arguments)] macro, i could of course just wrap one of the parameter in a tuple, but what's the point in it.

Maybe someone else got an idea how to get the render_sticky_context function down to a max of 7 Arguments.

poliorcetics · 2023-03-03T12:48:23Z

i think in order to satisfy clippys complaint, i would have to use the #[allow(clippy::too_many_arguments)] macro, i could of course just wrap one of the parameter in a tuple, but what's the point in it.

Maybe someone else got an idea how to get the render_sticky_context function down to a max of 7 Arguments.

Don't worry too much about it, this one lint is helpful most of the time but it can't account for the context of the code, ignoring it is often ok (I'm on my phone so I won't review right now, I can't tell for your use case yet)

archseer · 2023-03-08T10:35:03Z

Note: the Neovim plugin this is based on is switching to queries for determining the nodes (nvim-treesitter/nvim-treesitter-context#198). We should do the same (similar to indents where we started with a toml config but now use proper queries)

SoraTenshi · 2023-03-08T11:56:10Z

Note: the Neovim plugin this is based on is switching to queries for determining the nodes (nvim-treesitter/nvim-treesitter-context#198). We should do the same (similar to indents where we started with a toml config but now use proper queries)

Do i understand it correctly, that you'd want seperate injection queries (tree-sitter) for the context?

If so, i think i can do that.

yerlaser · 2023-03-08T14:54:53Z

I would advocate not to depend on tree-sitter on this feature. There are still many languages that don't have TS.
I use such languages and I often miss even simple things, like match pairs because it also depends on TS.
Please be considerate to us, poor people, who have to deal with multiple languages.
A simple word-split is more than enough.

SoraTenshi · 2023-03-08T14:57:48Z

I would advocate not to depend on tree-sitter on this feature. There are still many languages that don't have TS. I use such languages and I often miss even simple things, like match pairs because it also depends on TS. Please be considerate to us, poor people, who have to deal with multiple languages. A simple word-split is more than enough.

in order for this feature to work without tree-sitter, i would have to manually parse scopes, which are VERY inconsistent throughout languages, therefore tree-sitter is, imo, the only way of doing this kind of properly.

You also could - of course - always create tree-sitter grammars as well as add languages to helix

pascalkuthe · 2023-03-08T15:01:51Z

I would advocate not to depend on tree-sitter on this feature. There are still many languages that don't have TS. I use such languages and I often miss even simple things, like match pairs because it also depends on TS. Please be considerate to us, poor people, who have to deal with multiple languages. A simple word-split is more than enough.

Helix is primarily a TS based editor and languages, I think supporting plaintext files for some basic editing operations (like pair matching) can make sense but not for features that require semantic information (as this one does). The right fix is to create a TS grammar for those languages instead of adding language specific code into helix. Creating a TS grammar is not that hard and most mainstream languages do have one

yerlaser · 2023-03-08T15:06:40Z

OK. I see your point. Unfortunately, for people like DevOps or SysAdmins it still means opening lots of text files (scripts, configs etc.) in weird languages that will never have a TS.
Another case is markup languages like markdown. As far as I understand, even if a TS existed for it, an object would be the whole text block which is too large of a target to go to.

But, again, it's just my concern and wish. This feature is so needed that I would be happy if it works at least for the supported languages.

pascalkuthe · 2023-03-08T15:14:54Z

Helix has grammars for markup and configuration languages too (like markdown). Any file that has syntax highlighting in helix has a TS grammar. We even have TS grammars for filetypes like git-rebase, ini, wit, passwd, hosts,.. . If you are missing a TS grammar for something it should be easy enough to create one (and you would be surprised how much is already covered).

erasin · 2023-03-09T04:22:14Z

I tested this pr and it shows a bit strange in golang. I also support the use of tree-sitter queries, nvim-treesitter-context/queries provides a lot of language , we can use it, other languages can be added later.

SoraTenshi · 2023-03-09T08:45:53Z

I tested this pr and it shows a bit strange in golang. I also support the use of tree-sitter queries, nvim-treesitter-context/queries provides a lot of language , we can use it, other languages can be added later.

What exactly looks strange?
Can you provide an example?

erasin · 2023-03-09T12:28:36Z

I have added the following configuration.

[[language]]
name = "go"
sticky-context-nodes = ["package_clause","type_declaration","method_declaration","function_declaration","for_statement","if_statement", "expression_switch_statement"]

I test with golang base library net/http/request.go.

Some times it is displayed correctly and some times it is not displayed.

SoraTenshi · 2023-03-09T12:46:06Z

sticky-context-nodes = ["package_clause","type_declaration","method_declaration","function_declaration","for_statement","if_statement", "expression_switch_statement"]

ahh maybe i know why.
I added that if a node occupies more than 1 half of the screen, it may not be sticked.

I will remove that, as it's an old artifact. Thanks for the finding! :)

erasin · 2023-03-10T06:25:22Z

When I have non-ascii in my code, the capture is incorrect. For example, the following insert method.

test file: https://gist.github.com/erasin/a15c4f250beff8c9f7e4aaeae4a1128c

2023-03-10.13.52.18.mov

I get a panic when using the mouse to scroll end of file. The error was not found in master.

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Char index out of bounds: char index 4184, Rope/RopeSlice char length 4173', /Users/erasin/.cargo/registry/src/github.com-1ecc6299db9ec823/ropey-1.6.0/src/slice.rs:379:41
stack backtrace:
   0:        0x109be68c6 - std::backtrace_rs::backtrace::libunwind::trace::h310cbd77a7d2ae59
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:        0x109be68c6 - std::backtrace_rs::backtrace::trace_unsynchronized::h5768bae568840507
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:        0x109be68c6 - std::sys_common::backtrace::_print_fmt::hd104a205649a2ffb
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:65:5
   3:        0x109be68c6 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h521420ec33f3769d
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:44:22
   4:        0x109c08c6a - core::fmt::write::h694a0d7c23f57ada
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/fmt/mod.rs:1208:17
   5:        0x109bdfb9c - std::io::Write::write_fmt::h1920a3973ad439e5
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/io/mod.rs:1682:15
   6:        0x109be66aa - std::sys_common::backtrace::_print::h75582c4ed1a04abb
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:47:5
   7:        0x109be66aa - std::sys_common::backtrace::print::hef1aa4dbdc07ee06
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:34:9
   8:        0x109be88d3 - std::panicking::default_hook::{{closure}}::h529701a1070b4ce0
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:267:22
   9:        0x109be8628 - std::panicking::default_hook::hfeeab2c667b2d7c2
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:286:9
  10:        0x1083df5c1 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h9afa8b8b4a6f4294
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/alloc/src/boxed.rs:2032:9
  11:        0x108406e1f - helix_term::application::Application::run::{{closure}}::{{closure}}::h59304607632a2a9a
                               at /Users/erasin/github/helix/helix-term/src/application.rs:1061:13
  12:        0x109be9027 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h23ed9dbfdf16f482
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/alloc/src/boxed.rs:2032:9
  13:        0x109be9027 - std::panicking::rust_panic_with_hook::h1b5245192f90251d
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:692:13
  14:        0x109be8dd4 - std::panicking::begin_panic_handler::{{closure}}::h3658f3a9566379d4
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:579:13
  15:        0x109be6d68 - std::sys_common::backtrace::__rust_end_short_backtrace::h9e01645d962f8882
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:137:18
  16:        0x109be8a9d - rust_begin_unwind
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:575:5
  17:        0x109c4bee3 - core::panicking::panic_fmt::h0097ad8ec0b07517
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/panicking.rs:64:14
  18:        0x109c4c365 - core::result::unwrap_failed::h2a0ffdcdbffb9262
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/result.rs:1791:5
  19:        0x10964bc3a - core::result::Result<T,E>::unwrap::h76fa627ea2ccb7e6
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/result.rs:1113:23
-  20:        0x10870149f - ropey::slice::RopeSlice::char_to_line::h83b3a9f53cafd361
-                               at /Users/erasin/.cargo/registry/src/github.com-1ecc6299db9ec823/ropey-1.6.0/src/slice.rs:379:9
-  21:        0x1087b4dac - helix_term::ui::editor::EditorView::calculate_sticky_nodes::hd44adebca1ce03c2
-                               at /Users/erasin/github/helix/helix-term/src/ui/editor.rs:857:24
-  22:        0x1087b0298 - helix_term::ui::editor::EditorView::render_view::h801f2329103ac9f0
-                               at /Users/erasin/github/helix/helix-term/src/ui/editor.rs:191:17
-  23:        0x1087b9fa4 - <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::render::h0d6ad5e880ce309a
-                               at /Users/erasin/github/helix/helix-term/src/ui/editor.rs:1604:13
-  24:        0x1087cb16e - helix_term::compositor::Compositor::render::h421b649aa5e8a5c7
-                               at /Users/erasin/github/helix/helix-term/src/compositor.rs:170:13
-  25:        0x108407ee7 - helix_term::application::Application::render::{{closure}}::h9128185baddbd711
-                               at /Users/erasin/github/helix/helix-term/src/application.rs:285:9
-  26:        0x108401061 - helix_term::application::Application::handle_terminal_events::{{closure}}::h452798700217d49a
-                               at /Users/erasin/github/helix/helix-term/src/application.rs:618:26
-  27:        0x1083ffbfe - helix_term::application::Application::event_loop_until_idle::{{closure}}::h8cc9ae38c69ea5ec
-                               at /Users/erasin/github/helix/helix-term/src/application.rs:326:55
-  28:        0x1083fd20c - helix_term::application::Application::event_loop::{{closure}}::hf15c5275827c0534
-                               at /Users/erasin/github/helix/helix-term/src/application.rs:302:57
-  29:        0x10840691d - helix_term::application::Application::run::{{closure}}::h64d788da4e75353b
-                               at /Users/erasin/github/helix/helix-term/src/application.rs:1064:38
-  30:        0x1083f5e1e - hx::main_impl::{{closure}}::h64babcb08100836d
-                               at /Users/erasin/github/helix/helix-term/src/main.rs:156:53
  31:        0x1083d659e - tokio::runtime::park::CachedParkThread::block_on::{{closure}}::h04680cd7055be189
                               at /Users/erasin/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.26.0/src/runtime/park.rs:283:63
  32:        0x1083d6403 - tokio::runtime::coop::with_budget::h3450751a913955ea
                               at /Users/erasin/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.26.0/src/runtime/coop.rs:107:5
  33:        0x1083d6403 - tokio::runtime::coop::budget::hfece1804ad294c58
                               at /Users/erasin/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.26.0/src/runtime/coop.rs:73:5
  34:        0x1083d6403 - tokio::runtime::park::CachedParkThread::block_on::h50891113bf07b160
                               at /Users/erasin/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.26.0/src/runtime/park.rs:283:31
  35:        0x1083ed336 - tokio::runtime::context::BlockingRegionGuard::block_on::h97a7cfd8b0f5191b
                               at /Users/erasin/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.26.0/src/runtime/context.rs:315:13
  36:        0x1083b9bd5 - tokio::runtime::scheduler::multi_thread::MultiThread::block_on::h6a41409b5c3a1748
                               at /Users/erasin/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.26.0/src/runtime/scheduler/multi_thread/mod.rs:66:9
  37:        0x108417cfb - tokio::runtime::runtime::Runtime::block_on::h72dfd34556937174
                               at /Users/erasin/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.26.0/src/runtime/runtime.rs:304:45
  38:        0x10840905e - hx::main_impl::h594c06858aac3bd2
                               at /Users/erasin/github/helix/helix-term/src/main.rs:158:5
  39:        0x108408f01 - hx::main::hbd7e18d2aacb3ddc
                               at /Users/erasin/github/helix/helix-term/src/main.rs:38:21
  40:        0x108409f7e - core::ops::function::FnOnce::call_once::h81bdc8e78b625ab3
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/ops/function.rs:507:5
  41:        0x1083df8a1 - std::sys_common::backtrace::__rust_begin_short_backtrace::h582282e1d3fcd9fa
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:121:18
  42:        0x1083e5d64 - std::rt::lang_start::{{closure}}::h8919f335f7d65c4f
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/rt.rs:166:18
  43:        0x109bd9a24 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h2302f1d25ef2ca9b
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/ops/function.rs:606:13
  44:        0x109bd9a24 - std::panicking::try::do_call::h6695e32a593de2cc
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:483:40
  45:        0x109bd9a24 - std::panicking::try::hd4a93095627721a9
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:447:19
  46:        0x109bd9a24 - std::panic::catch_unwind::he41b3dba63feca94
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panic.rs:137:14
  47:        0x109bd9a24 - std::rt::lang_start_internal::{{closure}}::hbf45583011495a61
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/rt.rs:148:48
  48:        0x109bd9a24 - std::panicking::try::do_call::ha3e6b3edab7da449
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:483:40
  49:        0x109bd9a24 - std::panicking::try::hd4e0f354bf7022b9
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:447:19
  50:        0x109bd9a24 - std::panic::catch_unwind::h1035b163871a4269
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panic.rs:137:14
  51:        0x109bd9a24 - std::rt::lang_start_internal::hd56d2fa7efb2dd60
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/rt.rs:148:20
  52:        0x1083e5d37 - std::rt::lang_start::h2981ed93936c2adf
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/rt.rs:165:17
  53:        0x1084090e8 - _main
  54:     0x7ff816549310 - <unknown>

archseer · 2023-03-10T06:28:06Z

Do i understand it correctly, that you'd want seperate injection queries (tree-sitter) for the context?

Yes, we should reuse the nvim-treesitter format for these queries too:

https://github.com/nvim-treesitter/nvim-treesitter-context/blob/master/queries/go/context.scm
https://github.com/nvim-treesitter/nvim-treesitter-context/blob/master/queries/rust/context.scm

This ways they are shareable across editors.

archseer · 2023-03-10T06:40:44Z

@yerlaser We leverage tree-sitter because it makes the implementation of various features easier and more performant. We'll sometimes implement fallbacks though, for example matchbrackets is getting one: #4288

helix-term/src/ui/editor.rs

SoraTenshi · 2023-03-10T09:24:03Z

Do i understand it correctly, that you'd want seperate injection queries (tree-sitter) for the context?

Yes, we should reuse the nvim-treesitter format for these queries too:

nvim-treesitter/nvim-treesitter-context@master/queries/go/context.scm nvim-treesitter/nvim-treesitter-context@master/queries/rust/context.scm

This ways they are shareable across editors.

Ok, have started working on it, and will continue these days.

helix-term/src/ui/editor.rs

poliorcetics · 2023-03-11T08:38:46Z

helix-term/src/ui/editor.rs

+ .take(max_nodes_amount)
+ .rev()
+ .enumerate()
+ .take_while(|(i, _)| *i + 1 != visual_cursor_pos as usize) // also only nodes that don't overlap with the visual cursor position
+ .map(|(i, node)| {
+ let mut new_node = node;
+ new_node.visual_line = i as u16;
+ new_node
+ })
+ .collect();


This will take max_node_amount and then remove elements again if they overlap, is that intended ?

The version below avoids one Vec allocation but I'm not convinced it would be faster, how big are you expecting context to be most of the time ?

context.reverse(); if context.len() > max_node_amount { // only take the nodes until 1 / 3 of the viewport is reached or the maximum amount of sticky nodes context.drain(max_node_amount..); } let mut idx = 0; let mut overlap_reached = 1 == visual_cursor_pos; context.retain_mut(|node| { if overlap_reached { return false; } overlap_reached = idx + 1 == visual_cursor_pos; node.visual_line = idx; idx += 1; true });

i would say that it will mostly sit around 3~5 nodes.
So i think performance there is not really that big of a deal, i would much rather avoid more of the text.x_to_y calls, as they're rather heavy. O(log n)

helix-view/src/editor.rs

SoraTenshi · 2023-03-11T09:17:19Z

@poliorcetics i appreciate that you already di a new review, but I wasn't finished ^^`

novusnota · 2024-03-24T22:30:48Z

@SoraTenshi Do you need help in adapting context tree-sitter queries from Neovim? If so, which branch should we send them to, sticky-context of your repo?

SoraTenshi · 2024-03-24T22:33:37Z

@SoraTenshi Do you need help in adapting context tree-sitter queries from Neovim? If so, which branch should we send them to, sticky-context of your repo?

Yes, sticky-context, please.

Also as a side-note, since i am already commenting:
Currently, i think, sticky context works as best as it ever been, i just fixed a nasty bug by removing an assumption of that all the previous nodes are "fine", which wasn't the case, now i just iterate through all of them and remove the invalid ones. Much easier to comprehend, and it fixed a pretty nasty bug.

I am actually quite happy with how far i've gotten, a year ago when i started, i never thought i will get this pr this far. Soon, almost all bugs are gone that i have witnessed.

Also; Buffers apparently aren't cleared from the sticky nodes (to my surprise?) splits keep their own seperate nodes, which is interesting imo

gitmalong · 2024-06-14T12:54:38Z

Are people still working on this?

TornaxO7 · 2024-06-14T14:38:59Z

Last commit is 2 weeks ago, so I'd say: Yes

SoraTenshi · 2024-06-14T15:07:46Z

Are people still working on this?

Yes i am still actively working on stuff, this feature has a lot of changes in place and this feature was also my first "bigger" thing in Rust so i also had to learn about Rust and fix quite a bit of issues. It still has issues, but i need to find ways to actively trigger those bugs, but that's not so easy.

But in the current state, it's VERY usable, no crashes for over 3 months (iirc) and it's also in my daily driver. Just sometimes it has issues where the sticky context just isn't perfect, but i am unsure yet how i can fix those.

daedroza · 2024-06-14T15:13:45Z

Are people still working on this?

Yes i am still actively working on stuff, this feature has a lot of changes in place and this feature was also my first "bigger" thing in Rust so i also had to learn about Rust and fix quite a bit of issues. It still has issues, but i need to find ways to actively trigger those bugs, but that's not so easy.

But in the current state, it's VERY usable, no crashes for over 3 months (iirc) and it's also in my daily driver. Just sometimes it has issues where the sticky context just isn't perfect, but i am unsure yet how i can fix those.

I am willing to try but I want this to be complete rebased over master branch instead of having merge tags, would it be possible for you to achieve this?

git fetch origin master; git rebase FETCH_HEAD; should do the trick, this will bring all your commits on the top on master branch.

SoraTenshi · 2024-06-14T15:15:38Z

I am willing to try but I want this to be complete rebased over master branch instead of having merge tags, would it be possible for you to achieve this?

git fetch origin master; git rebase FETCH_HEAD; should do the trick, this will bring all your commits on the top on master branch.

I will take a look once i find the time to do so.
I am not sure how difficult it will be to fully rebase every single commit, but it's gonna be substantial amount of work i guess.

daedroza · 2024-06-15T13:22:21Z

I am willing to try but I want this to be complete rebased over master branch instead of having merge tags, would it be possible for you to achieve this?
git fetch origin master; git rebase FETCH_HEAD; should do the trick, this will bring all your commits on the top on master branch.

I will take a look once i find the time to do so. I am not sure how difficult it will be to fully rebase every single commit, but it's gonna be substantial amount of work i guess.

I don't think merge tags will be useful and then getting this merged on master branch with those tags.

Instead you can create a single commit with all of your changes on master branch and add relevant comments (or create few important commits), I'm sure no one will check 115 commits and their description instead they will directly go for changes.

SoraTenshi · 2024-06-15T13:30:36Z

I am willing to try but I want this to be complete rebased over master branch instead of having merge tags, would it be possible for you to achieve this?

git fetch origin master; git rebase FETCH_HEAD; should do the trick, this will bring all your commits on the top on master branch.

I will take a look once i find the time to do so. I am not sure how difficult it will be to fully rebase every single commit, but it's gonna be substantial amount of work i guess.

I don't think merge tags will be useful and then getting this merged on master branch with those tags.

Instead you can create a single commit with all of your changes on master branch and add relevant comments (or create few important commits), I'm sure no one will check 115 commits and their description instead they will directly go for changes.

I am pretty sure if this feature is ever ready it will get squashed regardless. And yeah I will probably just squash everything to also be able to just rebase it more effectively.

SoraTenshi · 2024-07-15T20:50:56Z

@daedroza I have squashed all the commits down to a single commit, that happened with the help of @Velnbur.

Just as a side note for you :)

dpc · 2024-07-15T21:03:48Z

After bunch of other stuff I like already landed and seeing this rebase, I finally got to enabling this to drive it around, but I'm utterly confused about how it works.

It seems to be displaying wrong context: not where my cursor is, but either ... where it was ... ? Or maybe where the top of the window is? Maybe I need to wait until you complete the rebase, since I literally noticed it 15 minutes ago, but just letting you know @SoraTenshi .

Anyway, thank you for working on this, I really want it in Helix. Much appreciated.

Edit: Screenshot might help.

My cursor is on WalletRecoveryStateV0, but the context at the top seems to be displaying stuff that is half a window above.

Could it maybe be related to new paging that is like in neovim?

SoraTenshi · 2024-07-15T21:13:05Z

@dpc you actually found a missing docs o/
you want to set in the sticky-context section the following:
follow-cursor = true

This changes the behaviour based on topmost screen location to your cursor.

dpc · 2024-07-15T21:32:31Z

@SoraTenshi Oh. Thanks. Though, I enabled this one, and now it ... doesn't seems to show anything? Actually, no... it does show it if the block is both crossing the top level of the page and my cursor is still in it? Or maybe some bug is involved. It seems like inconsistent and confusing behavior one way or the other. If you think it's helpful and it should work OK now and rebase was OK, I can try capturing ascinama.

My current config:

[editor.sticky-context]
enable = true
indicator = true
max-lines = 10
follow-cursor = true

BTW. To give more feedback, I don't like the waste of space of that horizonal line separator, and was also expecting something more compact like:

crate_name > modulename > impl SomeStruct

on one virtual line, possibly with distinct color/background, instead of a hline separator. Not sure if that's in plans etc.

Edit: The recording: https://asciinema.org/a/Jb5mZ0MDMZBEZDFWcZazz5YKo

SoraTenshi · 2024-07-15T21:38:11Z

@SoraTenshi Oh. Thanks. Though, I enabled this one, and now it ... doesn't seems to show anything? Actually, no... it does show it if the block is both crossing the top level of the page and my cursor is still in it? Or maybe some bug is involved. It seems like inconsistent and confusing behavior one way or the other. If you think it's helpful and it should work OK now and rebase was OK, I can try capturing ascinama.

I'll take a look in a moment.

BTW. To give more feedback, I don't like the waste of space of that horizonal line separator, and was also expecting something more compact like:
Module > impl SomeStruct
on one virtual line, possibly with distinct color/background, instead of a hline separator. Not sure if that's in plans etc.

Not part of this pr.

SoraTenshi · 2024-07-15T21:59:40Z

@SoraTenshi Oh. Thanks. Though, I enabled this one, and now it ... doesn't seems to show anything? Actually, no... it does show it if the block is both crossing the top level of the page and my cursor is still in it? Or maybe some bug is involved. It seems like inconsistent and confusing behavior one way or the other. If you think it's helpful and it should work OK now and rebase was OK, I can try capturing ascinama.

I'll take a look in a moment.

Nope it does work as expected.
A "node" i only then added to the top of the screen if it falls out of the current view.
the default mode is, (when follow-cursor is deactivated) it pins a node on the top of the viewport if we're in scope in e.g. a function and it only shows the function. The ... is explained in the pr description.

When follow-cursor is enabled, it only takes nodes that are out of bounds, but instead of using the topmost viewport, it takes into account the cursor position, so if the cursor is in another function that isn't out of scope, it won't show that function signature.

Keep in mind that there are still some bugs.

dpc · 2024-07-15T22:10:48Z

@SoraTenshi Thanks for explanation.

Maybe I have different expectations, as I never used a feature like this in a IDE, etc. as I was always using some underfeatured CLI text editor (Vim, Kakoune, now Helix), but it sounds somewhat complex in its behavior, while I kind of expect a single line telling me the context where my cursor is, so I can easily tell what module & struct/trait I'm looking at.

TornaxO7

Hi! Just skimmed through the code and added some thoughts (I hope it's fine). Feel free to deny them and thank you for working on this feature :)

helix-term/src/ui/context.rs

dpc · 2024-07-15T23:57:40Z

Witnessed a panic twice now. Second time had a RUST_BACKTRACE=1.

Each time I selected and deleted a large section of code, selected from botton to top. Just tried again, same behavior (3rd panic). Selecting from top to bottom did not trigger it.

I'm editing near the end of the document, which I think is important.

thread 'main' panicked at /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0/ropey-1.6.1/src/s
lice.rs:324:41:
called `Result::unwrap()` on an `Err` value: Byte index out of bounds: byte index 7279, Rope/RopeSlice byte length 6411
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: helix_term::ui::context::render_sticky_context
   4: helix_term::ui::editor::EditorView::render_view
   5: <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::render
   6: helix_term::application::Application::render::{{closure}}
   7: hx::main_impl::{{closure}}
   8: tokio::runtime::park::CachedParkThread::block_on
   9: hx::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

SoraTenshi · 2024-07-16T00:20:46Z

Witnessed a panic twice now. Second time had a RUST_BACKTRACE=1.

Each time I selected and deleted a large section of code, selected from botton to top. Just tried again, same behavior (3rd panic). Selecting from top to bottom did not trigger it.

Can you tell me how to repr.?
(with an example file, or something?)

SoraTenshi · 2024-07-17T14:23:20Z

@TornaxO7 thx! :)

TornaxO7 · 2024-07-17T20:30:37Z

@TornaxO7 thx! :)

You're welcome

A lot more work has been put into this and those were 116 commits up to this point. I decided to squash all of them so that i will have an easier time rebasing in the future.

SoraTenshi force-pushed the sticky-context branch 4 times, most recently from 7ce93e9 to f1d26a6 Compare February 27, 2023 22:52

SoraTenshi force-pushed the sticky-context branch from 56c857f to 2e6c02b Compare March 2, 2023 17:24

SoraTenshi marked this pull request as ready for review March 2, 2023 17:29

SoraTenshi force-pushed the sticky-context branch from acbc1a0 to f22cbf9 Compare March 3, 2023 11:02

poliorcetics suggested changes Mar 10, 2023

View reviewed changes

helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved

helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved

helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved

helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved

SoraTenshi marked this pull request as draft March 10, 2023 21:43

SoraTenshi force-pushed the sticky-context branch from 49e657d to 7021fc4 Compare March 10, 2023 22:26

poliorcetics suggested changes Mar 11, 2023

View reviewed changes

SoraTenshi force-pushed the sticky-context branch from 861aa50 to 504d189 Compare March 24, 2024 22:29

SoraTenshi force-pushed the sticky-context branch from 504d189 to fb1a5af Compare March 24, 2024 22:31

SoraTenshi force-pushed the sticky-context branch from f8db923 to 5ba90a1 Compare July 15, 2024 20:45

SoraTenshi force-pushed the sticky-context branch from 5ba90a1 to 0860388 Compare July 15, 2024 20:58

TornaxO7 reviewed Jul 15, 2024

View reviewed changes

SoraTenshi force-pushed the sticky-context branch 3 times, most recently from 250781b to dd5bc8d Compare July 17, 2024 20:33

feat: implement sticky-context

a5a3b5f

A lot more work has been put into this and those were 116 commits up to this point. I decided to squash all of them so that i will have an easier time rebasing in the future.

SoraTenshi force-pushed the sticky-context branch from dd5bc8d to a5a3b5f Compare July 17, 2024 23:40

Feat: Sticky Context #6118

Are you sure you want to change the base?

Feat: Sticky Context #6118

Conversation

SoraTenshi commented Feb 27, 2023 • edited Loading

!! If your preferred colorscheme / language is not yet in this PR, please feel free to open a PR on my branch: sticky-context to add those! It helps me quite a lot, as i can't keep track of every single colorscheme / language that helix supports !!

SoraTenshi commented Feb 28, 2023

SoraTenshi commented Mar 1, 2023 • edited Loading

poliorcetics commented Mar 3, 2023

archseer commented Mar 8, 2023

SoraTenshi commented Mar 8, 2023

yerlaser commented Mar 8, 2023

SoraTenshi commented Mar 8, 2023

pascalkuthe commented Mar 8, 2023

yerlaser commented Mar 8, 2023

pascalkuthe commented Mar 8, 2023 • edited Loading

erasin commented Mar 9, 2023

SoraTenshi commented Mar 9, 2023

erasin commented Mar 9, 2023

SoraTenshi commented Mar 9, 2023 • edited Loading

erasin commented Mar 10, 2023

archseer commented Mar 10, 2023

archseer commented Mar 10, 2023

SoraTenshi commented Mar 10, 2023 • edited Loading

poliorcetics Mar 11, 2023

Choose a reason for hiding this comment

SoraTenshi Mar 11, 2023

Choose a reason for hiding this comment

SoraTenshi commented Mar 11, 2023

novusnota commented Mar 24, 2024 • edited Loading

SoraTenshi commented Mar 24, 2024 • edited Loading

gitmalong commented Jun 14, 2024

TornaxO7 commented Jun 14, 2024

SoraTenshi commented Jun 14, 2024

daedroza commented Jun 14, 2024

SoraTenshi commented Jun 14, 2024

daedroza commented Jun 15, 2024

SoraTenshi commented Jun 15, 2024

SoraTenshi commented Jul 15, 2024

dpc commented Jul 15, 2024 • edited Loading

SoraTenshi commented Jul 15, 2024

dpc commented Jul 15, 2024 • edited Loading

SoraTenshi commented Jul 15, 2024

SoraTenshi commented Jul 15, 2024 • edited Loading

dpc commented Jul 15, 2024

TornaxO7 left a comment • edited Loading

Choose a reason for hiding this comment

dpc commented Jul 15, 2024 • edited Loading

SoraTenshi commented Jul 16, 2024 • edited Loading

SoraTenshi commented Jul 17, 2024

TornaxO7 commented Jul 17, 2024

SoraTenshi commented Feb 27, 2023 •

edited

Loading

!! If your preferred colorscheme / language is not yet in this PR, please feel free to open a PR on my branch: `sticky-context` to add those! It helps me quite a lot, as i can't keep track of every single colorscheme / language that helix supports !!

SoraTenshi commented Mar 1, 2023 •

edited

Loading

pascalkuthe commented Mar 8, 2023 •

edited

Loading

SoraTenshi commented Mar 9, 2023 •

edited

Loading

SoraTenshi commented Mar 10, 2023 •

edited

Loading

novusnota commented Mar 24, 2024 •

edited

Loading

SoraTenshi commented Mar 24, 2024 •

edited

Loading

dpc commented Jul 15, 2024 •

edited

Loading

dpc commented Jul 15, 2024 •

edited

Loading

SoraTenshi commented Jul 15, 2024 •

edited

Loading

TornaxO7 left a comment •

edited

Loading

dpc commented Jul 15, 2024 •

edited

Loading

SoraTenshi commented Jul 16, 2024 •

edited

Loading