TudbuT/helix-mods
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Reorganize Document::apply_impl (#11304)

Browse source 
These changes are ported from
<https://redirect.github.com/helix-editor/helix/pull/9801>. It's a
cleanup of `Document::apply_impl` that uses some early returns to
reduce nesting and some reordering of the steps. The early returns
bail out of `apply_impl` early if the transaction fails to apply or
if the changes are empty (in which case we emit the SelectionDidChange
event). It's a somewhat cosmetic refactor that makes the function easier
to reason about but it also makes it harder to introduce bugs by mapping
positions through empty changesets for example.

Co-authored-by: Pascal Kuthe <pascalkuthe@pm.me>
This commit is contained in:
Michael Davis 2024-07-26 02:32:02 -05:00 • committed by GitHub
parent 9b65448f53
commit a1e20a3426
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 150 additions and 141 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

77
helix-view/src/document.rs
View file

@ -1222,17 +1222,27 @@ impl Document {
use helix_core::Assoc;
let old_doc = self.text().clone();
let changes = transaction.changes();
if !changes.apply(&mut self.text) {
return false;
}
let success = transaction.changes().apply(&mut self.text);
if success {
if emit_lsp_notification {
helix_event::dispatch(DocumentDidChange {
if changes.is_empty() {
if let Some(selection) = transaction.selection() {
self.selections.insert(
view_id,
selection.clone().ensure_invariants(self.text.slice(..)),
);
helix_event::dispatch(SelectionDidChange {
doc: self,
view: view_id,
old_text: &old_doc,
});
}
return true;
}
self.modified_since_accessed = true;
self.version += 1;
for selection in self.selections.values_mut() {
*selection = selection
@ -1249,28 +1259,6 @@ impl Document {
.map_pos(view_data.view_position.anchor, Assoc::Before);
}
// if specified, the current selection should instead be replaced by transaction.selection
if let Some(selection) = transaction.selection() {
self.selections.insert(
view_id,
selection.clone().ensure_invariants(self.text.slice(..)),
);
helix_event::dispatch(SelectionDidChange {
doc: self,
view: view_id,
});
}
self.modified_since_accessed = true;
}
if !transaction.changes().is_empty() {
self.version += 1;
// start computing the diff in parallel
if let Some(diff_handle) = &self.diff_handle {
diff_handle.update_document(self.text.clone(), false);
}
// generate revert to savepoint
if !self.savepoints.is_empty() {
let revert = transaction.invert(&old_doc);
@ -1300,7 +1288,11 @@ impl Document {
}
}
let changes = transaction.changes();
// TODO: all of that should likely just be hooks
// start computing the diff in parallel
if let Some(diff_handle) = &self.diff_handle {
diff_handle.update_document(self.text.clone(), false);
}
// map diagnostics over changes too
changes.update_positions(self.diagnostics.iter_mut().map(|diagnostic| {
@ -1334,9 +1326,8 @@ impl Document {
true
});
self.diagnostics.sort_by_key(|diagnostic| {
(diagnostic.range, diagnostic.severity, diagnostic.provider)
});
self.diagnostics
.sort_by_key(|diagnostic| (diagnostic.range, diagnostic.severity, diagnostic.provider));
// Update the inlay hint annotations' positions, helping ensure they are displayed in the proper place
let apply_inlay_hint_changes = |annotations: &mut Vec<InlineAnnotation>| {
@ -1365,6 +1356,24 @@ impl Document {
apply_inlay_hint_changes(padding_after_inlay_hints);
}
helix_event::dispatch(DocumentDidChange {
doc: self,
view: view_id,
old_text: &old_doc,
});
// if specified, the current selection should instead be replaced by transaction.selection
if let Some(selection) = transaction.selection() {
self.selections.insert(
view_id,
selection.clone().ensure_invariants(self.text.slice(..)),
);
helix_event::dispatch(SelectionDidChange {
doc: self,
view: view_id,
});
}
if emit_lsp_notification {
// TODO: move to hook
// emit lsp notification
@ -1381,8 +1390,8 @@ impl Document {
}
}
}
}
success
true
}
fn apply_inner(