Bugfixes panic when resizing

[karthago1]

3ce8fac popup: bugfix: reset the position to the cursor when outside the viewport

while resizing, if the position is outside the view, then the program
panics. check the position and reset it to the cursor position

3577335 SignatureHelp: bugfix: introduce minimum width of 10

required_size shouldn't return None. Introduce minimum width so that the
height can be calculated

[pascalkuthe]

c783994 looks like a nice cleanup

1a17578 is a good catch. I think you again found a case where different people worked on the codebase that had a different understanding of what required size does (or didn't think of it too much). I think required_size was never supposed to work this way returning none is just the same as "unimplemented". IMO that is not good design tough. I checked and there is only a single place that return None from required_size which is the signature help. I think that call should just use remove its padding related check and use saturating_sub to avoid underflows. To make bugs like this less likely I think required size should be changed so that it just returns (u16, 16) and the default implementation would panic with unimplemented.

1595467 I am not sure aobut this one. I think this probably makes sense while resizing but I don't want to do this on every frame. Firstly popus are intentionally shown where they are triggered otherwise its too much movement/too noisy. Only updating the y and not the x position covers most of tht but if the cursor moves due to softwrap (or just other movement) I don't think the popups should follow. It also servers as a visual hint where autocompletion were trigerred for example. Recomputing the cursor position also has some overhead.

I think you can push c783994 into #10257. For 1a17578 if you make the changes I desribed (and keep them in a separate commit) that could go in there too.

I am not sure what to do about 1595467 right now so that should be separate

[karthago1]

I checked and there is only a single place that return None from required_size which is the signature help.

thanks somehow it didn't came to my mind to check it. I removed the None result. check it again, whether it is fine

1595467 I am not sure aobut this one. I think this probably makes sense while resizing but I don't want to do this on every frame. Firstly popus are intentionally shown where they are triggered otherwise its too much movement/too noisy. Only updating the y and not the x position covers most of tht but if the cursor moves due to softwrap (or just other movement) I don't think the popups should follow. It also servers as a visual hint where autocompletion were trigerred for example. Recomputing the cursor position also has some overhead.

I have a different opinion, I think it is an Improvement, but I removed it from this PR, since I want to fix the bugs first. and when this one is merged I will create another one with videos to show the difference 😸 .

Nevertheless keeping the position is buggy too, because if the position is outside the viewport, then we tell the paragraph to render somewhere with invalid x and y

thread 'main' panicked at 'Trying to access position outside the buffer: x=16, y=31, area=Rect { x: 0, y: 0, width: 95, height: 23 }', helix-tui/src/buffer.rs:235:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/panicking.rs:578:5
   1: core::panicking::panic_fmt
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/panicking.rs:67:14
   2: helix_tui::buffer::Buffer::index_of
             at /home/endless/Documents/helix/helix-tui/src/buffer.rs:235:9
   3: <helix_tui::buffer::Buffer as core::ops::index::IndexMut<(u16,u16)>>::index_mut
             at /home/endless/Documents/helix/helix-tui/src/buffer.rs:658:17
   4: helix_tui::buffer::Buffer::set_style
             at /home/endless/Documents/helix/helix-tui/src/buffer.rs:517:17
   5: <helix_tui::widgets::paragraph::Paragraph as helix_tui::widgets::Widget>::render
             at /home/endless/Documents/helix/helix-tui/src/widgets/paragraph.rs:134:9
   6: <helix_term::ui::lsp::SignatureHelp as helix_term::compositor::Component>::render
             at /home/endless/Documents/helix/helix-term/src/ui/lsp.rs:117:13
   7: <helix_term::ui::popup::Popup<T> as helix_term::compositor::Component>::render
             at /home/endless/Documents/helix/helix-term/src/ui/popup.rs:323:9

(this PR is based now on the master branch)

[karthago1]

closed, since #10573 is a better alternative