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.

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

Implement view swapping #2445

Merged
merged 2 commits into from
May 21, 2022
Merged

Conversation

zen3ger
Copy link
Contributor

@zen3ger zen3ger commented May 9, 2022

  • add Tree::swap_split_in_direction()
  • add swap_view_{left,down,up,right} commands, bound to H,J,K,L
    respectively in the Window menu(s)
  • add test for view swapping

helix-view/src/tree.rs Outdated Show resolved Hide resolved
Comment on lines +182 to +186
"L" => swap_view_right,
"K" => swap_view_up,
"H" => swap_view_left,
"J" => swap_view_down,
Copy link
Contributor

Choose a reason for hiding this comment

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

This will contradict with kakoune keys as in uppercase have different meanings but I think it is fine.

@zen3ger
Copy link
Contributor Author

zen3ger commented May 10, 2022

Well, it does mess up the selections when one file is really large compared to the other (RopeSlice out-of-bounds indexing) and closing a split jumps to the wrong buffer if you swap quite a lot. I'll fix things up after work...

EDIT: not anymore 🙂

Comment on lines 120 to 133
pub fn swap_content(view_a: &mut View, view_b: &mut View) {
std::mem::swap(&mut view_a.doc, &mut view_b.doc);
std::mem::swap(&mut view_a.offset, &mut view_b.offset);
std::mem::swap(&mut view_a.jumps, &mut view_b.jumps);
std::mem::swap(
&mut view_a.docs_access_history,
&mut view_b.docs_access_history,
);
std::mem::swap(
&mut view_a.last_modified_docs,
&mut view_b.last_modified_docs,
);
std::mem::swap(&mut view_a.gutters, &mut view_b.gutters);
}
Copy link
Member

@archseer archseer May 11, 2022

Choose a reason for hiding this comment

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

Rather than doing it this way, wouldn't it be better if you looked up the parent nodes for each view, then swapped the views there (just need to swap the two IDs)? That way there's no need to do all this extra work with swap_selection_in_views.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It ended up being a little more involved but it's still a lot less work.

@the-mikedavis the-mikedavis added the S-waiting-on-review Status: Awaiting review from a maintainer. label May 18, 2022
Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Great work! Tests are appreciated too :)

@archseer
Copy link
Member

Ah, looks like it needs a quick rebase

* add Tree::swap_split_in_direction()
* add swap_view_{left,down,up,right} commands, bound to H,J,K,L
  respectively in the Window menu(s)
* add test for view swapping
Instead of moving the Node contents on view swap if they have the same parent
reorder them to keep traversal order otherwise re-parent them.
@zen3ger
Copy link
Contributor Author

zen3ger commented May 21, 2022

Ah, looks like it needs a quick rebase

Done. :)

@the-mikedavis the-mikedavis merged commit 6bd8924 into helix-editor:master May 21, 2022
@zen3ger zen3ger deleted the feature-swap-views branch May 21, 2022 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants