Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #6087 #6088

Closed
wants to merge 1 commit into from
Closed

fix #6087 #6088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions helix-term/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4310,7 +4310,12 @@ fn expand_selection(cx: &mut Context) {
// check if selection is different from the last one
if *current_selection != selection {
// save current selection so it can be restored using shrink_selection
view.object_selections.push(current_selection.clone());
if let Some(object_selections) = view.object_selections.get_mut(&doc.id()) {
object_selections.push(current_selection.clone());
} else {
view.object_selections
.insert(doc.id(), vec![current_selection.clone()]);
}

doc.set_selection(view.id, selection);
}
Expand All @@ -4325,7 +4330,11 @@ fn shrink_selection(cx: &mut Context) {
let (view, doc) = current!(editor);
let current_selection = doc.selection(view.id);
// try to restore previous selection
if let Some(prev_selection) = view.object_selections.pop() {
if let Some(prev_selection) = view
.object_selections
.get_mut(&doc.id())
.and_then(|e| e.pop())
{
if current_selection.contains(&prev_selection) {
// allow shrinking the selection only if current selection contains the previous object selection
doc.set_selection(view.id, prev_selection);
Expand Down
4 changes: 2 additions & 2 deletions helix-view/src/view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ pub struct View {
// two last modified docs which we need to manually keep track of
pub last_modified_docs: [Option<DocumentId>; 2],
/// used to store previous selections of tree-sitter objects
pub object_selections: Vec<Selection>,
pub object_selections: HashMap<DocumentId, Vec<Selection>>,
Copy link
Member

Choose a reason for hiding this comment

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

Storing per document data on the view is not a good idea. If the document is closed that data is essentially leaked and it also preculdes you from updating data on edits. If we are going to be storing this per-view and per-document then this should be stored on the Document as HashMap<ViewId,..> just like selections are and inserted cleared in remove_view and ensure_view_init

/// all gutter-related configuration settings, used primarily for gutter rendering
pub gutters: GutterConfig,
/// A mapping between documents and the last history revision the view was updated at.
Expand Down Expand Up @@ -151,7 +151,7 @@ impl View {
jumps: JumpList::new((doc, Selection::point(0))), // TODO: use actual sel
docs_access_history: Vec::new(),
last_modified_docs: [None, None],
object_selections: Vec::new(),
object_selections: HashMap::new(),
gutters,
doc_revisions: HashMap::new(),
}
Expand Down