-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix crash on opening jumplist #6672
Fix crash on opening jumplist #6672
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change doesn't fully solve the issue: you can still cause a panic with the sequence [<space>:new<ret><C-w>v:buffer-next<ret>d<space>j
. The current change syncs the view for each focused doc but the docs in the jumplist may not be focused. That sequence panics because the first scratch buffer isn't focused in the first view so it isn't synced.
We need to sync changes between each view and all of the docs in view.jumps()
Should be fixed, according to my testing |
helix-view/src/editor.rs
Outdated
let doc = doc_mut!(self, &view.doc); | ||
view.sync_changes(doc); | ||
} | ||
self.sync_views(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to sync unfocused docs here so using a shared function does more work than we need. Let's eliminate the shared sync_views
function and instead sync the views as we create the items for the file picker (currently this block)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the new meta
closure captures &cx.editor
, it is impossible to also use &mut cx.editor
to create the &mut Doc
that is needed to sync the view with a doc so i have opted to sync the views earlier in the function. Thank you for your patience as i become more familiar with this repository by the way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Co-authored-by: Esra Fokker <[email protected]>
Co-authored-by: Esra Fokker <[email protected]>
Co-authored-by: Esra Fokker <[email protected]>
Hopefully solves #6628.
Moved the code to a separate function as was recommended in the issue.