Skip to content

Commit

Permalink
Consistently maintain view position (helix-editor#10559)
Browse files Browse the repository at this point in the history
* replicate t-monaghan's changes

* remove View.offset in favour of Document.view_data.view_position

* improve access patterns for Document.view_data

* better borrow checker wrangling with doc_mut!()

* reintroduce ensure_cursor_in_view in handle_config_events

since we sorted out the borrow checker issues using partial borrows,
there's nothing stopping us from going back to the simpler implementation

* introduce helper functions on Document .view_offset, set_view_offset

* fix rebase breakage
  • Loading branch information
intarga authored and SofusA committed Aug 4, 2024
1 parent 43b5286 commit d9dbee6
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 84 deletions.
6 changes: 3 additions & 3 deletions helix-term/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,9 @@ impl Application {

// reset view position in case softwrap was enabled/disabled
let scrolloff = self.editor.config().scrolloff;
for (view, _) in self.editor.tree.views_mut() {
let doc = &self.editor.documents[&view.doc];
view.ensure_cursor_in_view(doc, scrolloff)
for (view, _) in self.editor.tree.views() {
let doc = doc_mut!(self.editor, &view.doc);
view.ensure_cursor_in_view(doc, scrolloff);
}
}

Expand Down
57 changes: 36 additions & 21 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,7 @@ fn goto_window(cx: &mut Context, align: Align) {
let count = cx.count() - 1;
let config = cx.editor.config();
let (view, doc) = current!(cx.editor);
let view_offset = doc.view_offset(view.id);

let height = view.inner_height();

Expand All @@ -1044,15 +1045,15 @@ fn goto_window(cx: &mut Context, align: Align) {
let last_visual_line = view.last_visual_line(doc);

let visual_line = match align {
Align::Top => view.offset.vertical_offset + scrolloff + count,
Align::Center => view.offset.vertical_offset + (last_visual_line / 2),
Align::Top => view_offset.vertical_offset + scrolloff + count,
Align::Center => view_offset.vertical_offset + (last_visual_line / 2),
Align::Bottom => {
view.offset.vertical_offset + last_visual_line.saturating_sub(scrolloff + count)
view_offset.vertical_offset + last_visual_line.saturating_sub(scrolloff + count)
}
};
let visual_line = visual_line
.max(view.offset.vertical_offset + scrolloff)
.min(view.offset.vertical_offset + last_visual_line.saturating_sub(scrolloff));
.max(view_offset.vertical_offset + scrolloff)
.min(view_offset.vertical_offset + last_visual_line.saturating_sub(scrolloff));

let pos = view
.pos_at_visual_coords(doc, visual_line as u16, 0, false)
Expand Down Expand Up @@ -1665,6 +1666,7 @@ pub fn scroll(cx: &mut Context, offset: usize, direction: Direction, sync_cursor
use Direction::*;
let config = cx.editor.config();
let (view, doc) = current!(cx.editor);
let mut view_offset = doc.view_offset(view.id);

let range = doc.selection(view.id).primary();
let text = doc.text().slice(..);
Expand All @@ -1681,15 +1683,19 @@ pub fn scroll(cx: &mut Context, offset: usize, direction: Direction, sync_cursor
let doc_text = doc.text().slice(..);
let viewport = view.inner_area(doc);
let text_fmt = doc.text_format(viewport.width, None);
let mut annotations = view.text_annotations(&*doc, None);
(view.offset.anchor, view.offset.vertical_offset) = char_idx_at_visual_offset(
(view_offset.anchor, view_offset.vertical_offset) = char_idx_at_visual_offset(
doc_text,
view.offset.anchor,
view.offset.vertical_offset as isize + offset,
view_offset.anchor,
view_offset.vertical_offset as isize + offset,
0,
&text_fmt,
&annotations,
// &annotations,
&view.text_annotations(&*doc, None),
);
doc.set_view_offset(view.id, view_offset);

let doc_text = doc.text().slice(..);
let mut annotations = view.text_annotations(&*doc, None);

if sync_cursor {
let movement = match cx.editor.mode {
Expand All @@ -1716,14 +1722,16 @@ pub fn scroll(cx: &mut Context, offset: usize, direction: Direction, sync_cursor
return;
}

let view_offset = doc.view_offset(view.id);

let mut head;
match direction {
Forward => {
let off;
(head, off) = char_idx_at_visual_offset(
doc_text,
view.offset.anchor,
(view.offset.vertical_offset + scrolloff) as isize,
view_offset.anchor,
(view_offset.vertical_offset + scrolloff) as isize,
0,
&text_fmt,
&annotations,
Expand All @@ -1736,8 +1744,8 @@ pub fn scroll(cx: &mut Context, offset: usize, direction: Direction, sync_cursor
Backward => {
head = char_idx_at_visual_offset(
doc_text,
view.offset.anchor,
(view.offset.vertical_offset + height - scrolloff - 1) as isize,
view_offset.anchor,
(view_offset.vertical_offset + height - scrolloff - 1) as isize,
0,
&text_fmt,
&annotations,
Expand Down Expand Up @@ -5124,7 +5132,7 @@ fn split(editor: &mut Editor, action: Action) {
let (view, doc) = current!(editor);
let id = doc.id();
let selection = doc.selection(view.id).clone();
let offset = view.offset;
let offset = doc.view_offset(view.id);

editor.switch(id, action);

Expand All @@ -5133,7 +5141,7 @@ fn split(editor: &mut Editor, action: Action) {
doc.set_selection(view.id, selection);
// match the view scroll offset (switch doesn't handle this fully
// since the selection is only matched after the split)
view.offset = offset;
doc.set_view_offset(view.id, offset);
}

fn hsplit(cx: &mut Context) {
Expand Down Expand Up @@ -5228,14 +5236,21 @@ fn align_view_middle(cx: &mut Context) {
return;
}
let doc_text = doc.text().slice(..);
let annotations = view.text_annotations(doc, None);
let pos = doc.selection(view.id).primary().cursor(doc_text);
let pos =
visual_offset_from_block(doc_text, view.offset.anchor, pos, &text_fmt, &annotations).0;
let pos = visual_offset_from_block(
doc_text,
doc.view_offset(view.id).anchor,
pos,
&text_fmt,
&view.text_annotations(doc, None),
)
.0;

view.offset.horizontal_offset = pos
let mut offset = doc.view_offset(view.id);
offset.horizontal_offset = pos
.col
.saturating_sub((view.inner_area(doc).width as usize) / 2);
doc.set_view_offset(view.id, offset);
}

fn scroll_up(cx: &mut Context) {
Expand Down Expand Up @@ -6117,7 +6132,7 @@ fn jump_to_word(cx: &mut Context, behaviour: Movement) {

// This is not necessarily exact if there is virtual text like soft wrap.
// It's ok though because the extra jump labels will not be rendered.
let start = text.line_to_char(text.char_to_line(view.offset.anchor));
let start = text.line_to_char(text.char_to_line(doc.view_offset(view.id).anchor));
let end = text.line_to_char(view.estimate_last_doc_line(doc) + 1);

let primary_selection = doc.selection(view.id).primary();
Expand Down
3 changes: 2 additions & 1 deletion helix-term/src/commands/lsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1299,7 +1299,8 @@ fn compute_inlay_hints_for_view(
// than computing all the hints for the full file (which could be dozens of time
// longer than the view is).
let view_height = view.inner_height();
let first_visible_line = doc_text.char_to_line(view.offset.anchor.min(doc_text.len_chars()));
let first_visible_line =
doc_text.char_to_line(doc.view_offset(view_id).anchor.min(doc_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))
Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/commands/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1587,7 +1587,7 @@ fn tree_sitter_highlight_name(
// Query the same range as the one used in syntax highlighting.
let range = {
// Calculate viewport byte ranges:
let row = text.char_to_line(view.offset.anchor.min(text.len_chars()));
let row = text.char_to_line(doc.view_offset(view.id).anchor.min(text.len_chars()));
// Saturating subs to make it inclusive zero indexing.
let last_line = text.len_lines().saturating_sub(1);
let height = view.inner_area(doc).height;
Expand Down
21 changes: 13 additions & 8 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ impl EditorView {
let theme = &editor.theme;
let config = editor.config();

let view_offset = doc.view_offset(view.id);

let text_annotations = view.text_annotations(doc, Some(theme));
let mut decorations = DecorationManager::default();

Expand All @@ -119,13 +121,13 @@ impl EditorView {
}

let syntax_highlights =
Self::doc_syntax_highlights(doc, view.offset.anchor, inner.height, theme);
Self::doc_syntax_highlights(doc, view_offset.anchor, inner.height, theme);

let mut overlay_highlights =
Self::empty_highlight_iter(doc, view.offset.anchor, inner.height);
Self::empty_highlight_iter(doc, view_offset.anchor, inner.height);
let overlay_syntax_highlights = Self::overlay_syntax_highlights(
doc,
view.offset.anchor,
view_offset.anchor,
inner.height,
&text_annotations,
);
Expand Down Expand Up @@ -203,7 +205,7 @@ impl EditorView {
surface,
inner,
doc,
view.offset,
view_offset,
&text_annotations,
syntax_highlights,
overlay_highlights,
Expand Down Expand Up @@ -259,11 +261,13 @@ impl EditorView {
.and_then(|config| config.rulers.as_ref())
.unwrap_or(editor_rulers);

let view_offset = doc.view_offset(view.id);

rulers
.iter()
// View might be horizontally scrolled, convert from absolute distance
// from the 1st column to relative distance from left of viewport
.filter_map(|ruler| ruler.checked_sub(1 + view.offset.horizontal_offset as u16))
.filter_map(|ruler| ruler.checked_sub(1 + view_offset.horizontal_offset as u16))
.filter(|ruler| ruler < &viewport.width)
.map(|ruler| viewport.clip_left(ruler).with_width(1))
.for_each(|area| surface.set_style(area, ruler_theme))
Expand Down Expand Up @@ -825,6 +829,7 @@ impl EditorView {
let inner_area = view.inner_area(doc);

let selection = doc.selection(view.id);
let view_offset = doc.view_offset(view.id);
let primary = selection.primary();
let text_format = doc.text_format(viewport.width, None);
for range in selection.iter() {
Expand All @@ -835,11 +840,11 @@ impl EditorView {
visual_offset_from_block(text, cursor, cursor, &text_format, text_annotations).0;

// if the cursor is horizontally in the view
if col >= view.offset.horizontal_offset
&& inner_area.width > (col - view.offset.horizontal_offset) as u16
if col >= view_offset.horizontal_offset
&& inner_area.width > (col - view_offset.horizontal_offset) as u16
{
let area = Rect::new(
inner_area.x + (col - view.offset.horizontal_offset) as u16,
inner_area.x + (col - view_offset.horizontal_offset) as u16,
view.area.y,
1,
view.area.height,
Expand Down
6 changes: 3 additions & 3 deletions helix-term/src/ui/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub fn raw_regex_prompt(
let (view, doc) = current!(cx.editor);
let doc_id = view.doc;
let snapshot = doc.selection(view.id).clone();
let offset_snapshot = view.offset;
let offset_snapshot = doc.view_offset(view.id);
let config = cx.editor.config();

let mut prompt = Prompt::new(
Expand All @@ -95,7 +95,7 @@ pub fn raw_regex_prompt(
PromptEvent::Abort => {
let (view, doc) = current!(cx.editor);
doc.set_selection(view.id, snapshot.clone());
view.offset = offset_snapshot;
doc.set_view_offset(view.id, offset_snapshot);
}
PromptEvent::Update | PromptEvent::Validate => {
// skip empty input
Expand Down Expand Up @@ -136,7 +136,7 @@ pub fn raw_regex_prompt(
Err(err) => {
let (view, doc) = current!(cx.editor);
doc.set_selection(view.id, snapshot.clone());
view.offset = offset_snapshot;
doc.set_view_offset(view.id, offset_snapshot);

if event == PromptEvent::Validate {
let callback = async move {
Expand Down
51 changes: 46 additions & 5 deletions helix-view/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@ use helix_core::{
ChangeSet, Diagnostic, LineEnding, Range, Rope, RopeBuilder, Selection, Syntax, Transaction,
};

use crate::editor::Config;
use crate::events::{DocumentDidChange, SelectionDidChange};
use crate::{DocumentId, Editor, Theme, View, ViewId};
use crate::{
editor::Config,
events::{DocumentDidChange, SelectionDidChange},
view::ViewPosition,
DocumentId, Editor, Theme, View, ViewId,
};

/// 8kB of buffer space for encoding and decoding `Rope`s.
const BUF_SIZE: usize = 8192;
Expand Down Expand Up @@ -130,6 +133,7 @@ pub struct Document {
pub(crate) id: DocumentId,
text: Rope,
selections: HashMap<ViewId, Selection>,
view_data: HashMap<ViewId, ViewData>,

/// Inlay hints annotations for the document, by view.
///
Expand Down Expand Up @@ -267,6 +271,7 @@ impl fmt::Debug for Document {
.field("selections", &self.selections)
.field("inlay_hints_oudated", &self.inlay_hints_oudated)
.field("text_annotations", &self.inlay_hints)
.field("view_data", &self.view_data)
.field("path", &self.path)
.field("encoding", &self.encoding)
.field("restore_cursor", &self.restore_cursor)
Expand Down Expand Up @@ -658,6 +663,7 @@ impl Document {
selections: HashMap::default(),
inlay_hints: HashMap::default(),
inlay_hints_oudated: false,
view_data: Default::default(),
indent_style: DEFAULT_INDENT,
line_ending,
restore_cursor: false,
Expand Down Expand Up @@ -1187,12 +1193,14 @@ impl Document {
self.set_selection(view_id, Selection::single(origin.anchor, origin.head));
}

/// Initializes a new selection for the given view if it does not
/// already have one.
/// Initializes a new selection and view_data for the given view
/// if it does not already have them.
pub fn ensure_view_init(&mut self, view_id: ViewId) {
if self.selections.get(&view_id).is_none() {
self.reset_selection(view_id);
}

self.view_data_mut(view_id);
}

/// Mark document as recent used for MRU sorting
Expand Down Expand Up @@ -1238,6 +1246,12 @@ impl Document {
.ensure_invariants(self.text.slice(..));
}

for view_data in self.view_data.values_mut() {
view_data.view_position.anchor = transaction
.changes()
.map_pos(view_data.view_position.anchor, Assoc::Before);
}

// if specified, the current selection should instead be replaced by transaction.selection
if let Some(selection) = transaction.selection() {
self.selections.insert(
Expand Down Expand Up @@ -1762,6 +1776,28 @@ impl Document {
&self.selections
}

fn view_data(&self, view_id: ViewId) -> &ViewData {
self.view_data
.get(&view_id)
.expect("This should only be called after ensure_view_init")
}

fn view_data_mut(&mut self, view_id: ViewId) -> &mut ViewData {
self.view_data.entry(view_id).or_default()
}

pub(crate) fn get_view_offset(&self, view_id: ViewId) -> Option<ViewPosition> {
Some(self.view_data.get(&view_id)?.view_position)
}

pub fn view_offset(&self, view_id: ViewId) -> ViewPosition {
self.view_data(view_id).view_position
}

pub fn set_view_offset(&mut self, view_id: ViewId, new_offset: ViewPosition) {
self.view_data_mut(view_id).view_position = new_offset;
}

pub fn relative_path(&self) -> Option<Cow<Path>> {
self.path
.as_deref()
Expand Down Expand Up @@ -2037,6 +2073,11 @@ impl Document {
}
}

#[derive(Debug, Default)]
pub struct ViewData {
view_position: ViewPosition,
}

#[derive(Clone, Debug)]
pub enum FormatterError {
SpawningFailed {
Expand Down
Loading

0 comments on commit d9dbee6

Please sign in to comment.