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

Replace_document_in_view() applies the last known view for documents #7414

Closed
wants to merge 7 commits into from

Conversation

t-monaghan
Copy link

This PR is to address issue #7355

helix-view/src/editor.rs Outdated Show resolved Hide resolved
dead10ck
dead10ck previously approved these changes Jun 25, 2023
Copy link
Member

@dead10ck dead10ck left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks!

@pascalkuthe pascalkuthe self-requested a review June 25, 2023 15:50
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

This only works with a single view, not multiple views. Not only would they smudge last_view_position but the view offset can shift by various edits so that the position doesn't shift when the file is edited in another view. So the right way to fix this would be to store view offsets on the document per view and map them through changes. At that point, it doesn't make sense to differentiate between the last view position and the current view position. Instead, just store them all on the document. This is a general pattern we have been seeing (storing stuff on the document instead of view to map them through changes). Its sort of starting to bloat the document tough so we likely want some kind of shared ViewData struct for that. There was some discussion about that in #6198

@t-monaghan
Copy link
Author

@pascalkuthe I'm not quite across the intended behaviour for multiple views. Could I ask you to have a look at this recording and let me know if it demonstrates the smudging/shifting that you've raised? https://asciinema.org/a/8F4fvSJB4rY0tgtwJvXuJXUCy

@kirawi kirawi added the A-helix-term Area: Helix term improvements label Jun 30, 2023
@pascalkuthe
Copy link
Member

the intended behavoiur is that every view has its own position in the document. Basically views should not change others positions. so if I have file X open in views A and B at positions F and G respectively and switch to file Y in both views and then bach to X then A should endup at position F and B at position G again.

@the-mikedavis
Copy link
Member

Let's close this in favor of #7568

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to not centre the cursor when switching between buffers
6 participants