accept count for goto_window #1033
Conversation
also fix view is not fullfilled issue
helix-term/src/commands.rs
Outdated
| let scrolloff = | ||
| count.unwrap_or_else(|| cx.editor.config.scrolloff.min(height.saturating_sub(1) / 2)); |
There was a problem hiding this comment.
I think it would better to follow the vim behavior here of having count starting from scrolloff point, otherwise 1gt would result in moving the screen by scrolloff.
But vim behavior here is good I think. If the screen is at the corner of the screen (0 or file end), 1H will go to the first line without scrolloff (same for the end), and if the screen is being scrolled to the middle, 1H will go to the first scrolloff position rather than the top. This is the nicer behavior but I think it is a bit more complicated to implement.
And with large scrolloff count it doesn't move the screen, so <n>H in vim won't change the screen in any bits, it will move to the last scrolloff point.
There was a problem hiding this comment.
done, please take a another look
Co-authored-by: Ivan Tham <pickfire@riseup.net>
Co-authored-by: Ivan Tham <pickfire@riseup.net>
helix-term/src/commands.rs
Outdated
| Align::Bottom => last_line.saturating_sub(scrolloff), | ||
| Align::Top => (view.offset.row + scrolloff + count), | ||
| Align::Center => (view.offset.row + ((last_line - view.offset.row) / 2)), | ||
| Align::Bottom => last_line.saturating_sub((scrolloff + count).min(last_line)), |
There was a problem hiding this comment.
| Align::Bottom => last_line.saturating_sub((scrolloff + count).min(last_line)), | |
| Align::Bottom => last_line.saturating_sub((scrolloff + count)), |
saturating_sub should already protect against overflow here.
helix-term/src/commands.rs
Outdated
| } | ||
| .min(last_line.saturating_sub(scrolloff)); | ||
| .min((view.offset.row + height - 1).saturating_sub(scrolloff)) |
There was a problem hiding this comment.
What was wrong with the previous bound? I don't think this is correct, since view.offset + height - 1 can be past last_line of the document
There was a problem hiding this comment.
I already used last_line in the before computation , I think we need ensure we won't go beyond the scroll off line of the bottom. For example if the content is only half page, we subtract last_line with scroll off is pointless because it's far from scrolling.
also fix view is not fulfilled issue.
9gtwould go to the 9th line from window top;3gbwould go to the 3th line from window bottom;