Restore view offset when switching buffers with multiple windows

[t-monaghan]

This pull request is in response to changes requested by @pascalkuthe in #7414 looking to solve the issue #7355.

The changes made are to ensure views can store and access their own positioning for documents by way of including the field view_data: HashMap<ViewId, ViewData> in Document.

I haven't yet addressed the point of not differentiating between last view position and current view position. I'm unsure how to proceed with this goal.

Fixes https://github.com/helix-editor/helix/issues/7355

[the-mikedavis]

For reference see where we update selections when there are changes to a document: https://github.com/helix-editor/helix/blob/d570c29ce37ffbb46a9c49708c31dfd81daa27cf/helix-view/src/document.rs#L1160-L1167

diagnostics: https://github.com/helix-editor/helix/blob/d570c29ce37ffbb46a9c49708c31dfd81daa27cf/helix-view/src/document.rs#L1222-L1252

and inlay hints: https://github.com/helix-editor/helix/blob/d570c29ce37ffbb46a9c49708c31dfd81daa27cf/helix-view/src/document.rs#L1262-L1270

[intarga]

Since it doesn't seem like @t-monaghan is coming back for this, and I'm blocked on it for #9143, I decided to take a stab at it. I successfully implemented mapping the ViewPositions in Document.view_data through changes, and entirely removed View.offset in favour of Document.view_data.

It works, and solves the issue, but I'm not happy with it. Now every time any function needs to access a ViewPosition, it needs to do a hashmap lookup, which has a performance cost. But worse, I feel that it really clouds the abstraction boundary between View and Document to have ViewPositions natively live in Documents, it means that View methods that change the ViewPosition suddenly need a mutable reference to the doc of that view, and would arguably make more sense as methods on Document...

I'm starting to think that creating ViewData on Document was the wrong start on this problem, and that we'd be better served putting this history of ViewPositions on View instead. That way, the view abstraction doesn't leak onto Document, View can keep a .offset field for the position on the current document, which can be accessed without a hashmap lookup, and we should still be able to map through changes by finding views relevant to a document from Document.selections, and updating the relevant view position history value on them.

@the-mikedavis @pascalkuthe, I would appreciate if you have any guidance/insights to share on this

[pascalkuthe]

we should still be able to map through changes by finding views relevant to a document from Document.selections, and updating the relevant view position history value on them.

this is not possible you can't access views from apply

I plan to remove document and view as separate structs entirely in the future. Separating them to begin with was fairly pointless since nearly all fields are public so there is little to no encapsulation to begin with (for example the jumplist is a gift that keeps on giving more and more crashes because the laz4y mapping is just not robusta and it can't be encapsulated because it needs to be accessed from document).

I want to refactor that so Document and View only become ids and all their data is stored in separate containers (slotmap or hashmap) instead of one big sprawling struct (so a more entity component-like system which in my experience is a better data model for large rust applications). The data would be grouped into files/substructs where encapsulation actually makes sense/is possible (litmus test: all fields can and should be private). These changes will be required anyway for plugins/making the event system more useful (and the config refactor). But it's a really large set of changes that I will take on down the line.

I am not concerned about the extra hashmap lookup, that is basically irrelevant. HasMaps are really really fast.

[intarga]

this is not possible you can't access views from apply

Good point.

I plan to remove document and view as separate structs entirely in the future.

In that case what I've implemented makes more sense. I'll clean it up and file a new PR. Thanks for the info!

[the-mikedavis]

Closing in favor of #10559