Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Make anchor_in_excerpt Optional (#8975)

Browse source 
We were seeing panics due to callers assuming they had valid
excerpt_ids, but that cannot easily be guaranteed across await points as
anyone may remove an excerpt.

Release Notes:

- Fixed a panic when hovering in a multibuffer
This commit is contained in:
Conrad Irwin 2024-03-06 18:55:36 -07:00 • committed by GitHub
parent ae5ec9224c
commit 0b87be71e6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 101 additions and 73 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/assistant/src/assistant_panel.rs
View file

@ -2413,7 +2413,9 @@ impl ConversationEditor {
.read(cx) .read(cx)
.messages(cx) .messages(cx)
.map(|message| BlockProperties { .map(|message| BlockProperties {
position: buffer.anchor_in_excerpt(excerpt_id, message.anchor), position: buffer
.anchor_in_excerpt(excerpt_id, message.anchor)
.unwrap(),
height: 2, height: 2,
style: BlockStyle::Sticky, style: BlockStyle::Sticky,
render: Arc::new({ render: Arc::new({

14
crates/diagnostics/src/diagnostics.rs
View file

@ -517,15 +517,15 @@ impl ProjectDiagnosticsEditor {
self.editor.update(cx, |editor, cx| { self.editor.update(cx, |editor, cx| {
editor.remove_blocks(blocks_to_remove, None, cx); editor.remove_blocks(blocks_to_remove, None, cx);
let block_ids = editor.insert_blocks( let block_ids = editor.insert_blocks(
blocks_to_add.into_iter().map(|block| { blocks_to_add.into_iter().flat_map(|block| {
let (excerpt_id, text_anchor) = block.position; let (excerpt_id, text_anchor) = block.position;
BlockProperties { Some(BlockProperties {
position: excerpts_snapshot.anchor_in_excerpt(excerpt_id, text_anchor), position: excerpts_snapshot.anchor_in_excerpt(excerpt_id, text_anchor)?,
height: block.height, height: block.height,
style: block.style, style: block.style,
render: block.render, render: block.render,
disposition: block.disposition, disposition: block.disposition,
} })
}), }),
Some(Autoscroll::fit()), Some(Autoscroll::fit()),
cx, cx,
@ -589,17 +589,19 @@ impl ProjectDiagnosticsEditor {
Ok(ix) | Err(ix) => ix, Ok(ix) | Err(ix) => ix,
}; };
if let Some(group) = groups.get(group_ix) { if let Some(group) = groups.get(group_ix) {
let offset = excerpts_snapshot if let Some(offset) = excerpts_snapshot
.anchor_in_excerpt( .anchor_in_excerpt(
group.excerpts[group.primary_excerpt_ix], group.excerpts[group.primary_excerpt_ix],
group.primary_diagnostic.range.start, group.primary_diagnostic.range.start,
) )
.to_offset(&excerpts_snapshot); .map(|anchor| anchor.to_offset(&excerpts_snapshot))
{
selection.start = offset; selection.start = offset;
selection.end = offset; selection.end = offset;
} }
} }
} }
}
editor.change_selections(None, cx, |s| { editor.change_selections(None, cx, |s| {
s.select(selections); s.select(selections);
}); });

27
crates/editor/src/hover_links.rs
View file

@ -13,7 +13,7 @@ use project::{
}; };
use std::ops::Range; use std::ops::Range;
use theme::ActiveTheme as _; use theme::ActiveTheme as _;
use util::TryFutureExt; use util::{maybe, TryFutureExt};
#[derive(Debug)] #[derive(Debug)]
pub struct HoveredLinkState { pub struct HoveredLinkState {
@ -424,12 +424,13 @@ pub fn show_link_definition(
TriggerPoint::Text(_) => { TriggerPoint::Text(_) => {
if let Some((url_range, url)) = find_url(&buffer, buffer_position, cx.clone()) { if let Some((url_range, url)) = find_url(&buffer, buffer_position, cx.clone()) {
this.update(&mut cx, |_, _| { this.update(&mut cx, |_, _| {
let start = snapshot.anchor_in_excerpt(excerpt_id, url_range.start); let range = maybe!({
let end = snapshot.anchor_in_excerpt(excerpt_id, url_range.end); let start =
( snapshot.anchor_in_excerpt(excerpt_id, url_range.start)?;
Some(RangeInEditor::Text(start..end)), let end = snapshot.anchor_in_excerpt(excerpt_id, url_range.end)?;
vec![HoverLink::Url(url)], Some(RangeInEditor::Text(start..end))
) });
(range, vec![HoverLink::Url(url)])
}) })
.ok() .ok()
} else if let Some(project) = project { } else if let Some(project) = project {
@ -449,12 +450,14 @@ pub fn show_link_definition(
.map(|definition_result| { .map(|definition_result| {
( (
definition_result.iter().find_map(|link| { definition_result.iter().find_map(|link| {
link.origin.as_ref().map(|origin| { link.origin.as_ref().and_then(|origin| {
let start = snapshot let start = snapshot.anchor_in_excerpt(
.anchor_in_excerpt(excerpt_id, origin.range.start); excerpt_id,
origin.range.start,
)?;
let end = snapshot let end = snapshot
.anchor_in_excerpt(excerpt_id, origin.range.end); .anchor_in_excerpt(excerpt_id, origin.range.end)?;
RangeInEditor::Text(start..end) Some(RangeInEditor::Text(start..end))
}) })
}), }),
definition_result.into_iter().map(HoverLink::Text).collect(), definition_result.into_iter().map(HoverLink::Text).collect(),

15
crates/editor/src/hover_popover.rs
View file

@ -294,18 +294,19 @@ fn show_hover(
let hover_popover = match hover_result { let hover_popover = match hover_result {
Some(hover_result) if !hover_result.is_empty() => { Some(hover_result) if !hover_result.is_empty() => {
// Create symbol range of anchors for highlighting and filtering of future requests. // Create symbol range of anchors for highlighting and filtering of future requests.
let range = if let Some(range) = hover_result.range { let range = hover_result
.range
.and_then(|range| {
let start = snapshot let start = snapshot
.buffer_snapshot .buffer_snapshot
.anchor_in_excerpt(excerpt_id, range.start); .anchor_in_excerpt(excerpt_id, range.start)?;
let end = snapshot let end = snapshot
.buffer_snapshot .buffer_snapshot
.anchor_in_excerpt(excerpt_id, range.end); .anchor_in_excerpt(excerpt_id, range.end)?;
start..end Some(start..end)
} else { })
anchor..anchor .unwrap_or_else(|| anchor..anchor);
};
let language_registry = let language_registry =
project.update(&mut cx, |p, _| p.languages().clone())?; project.update(&mut cx, |p, _| p.languages().clone())?;

22
crates/editor/src/inlay_hint_cache.rs
View file

@ -459,16 +459,17 @@ impl InlayHintCache {
cmp::Ordering::Less | cmp::Ordering::Equal => { cmp::Ordering::Less | cmp::Ordering::Equal => {
if !old_kinds.contains(&cached_hint.kind) if !old_kinds.contains(&cached_hint.kind)
&& new_kinds.contains(&cached_hint.kind) && new_kinds.contains(&cached_hint.kind)
{
if let Some(anchor) = multi_buffer_snapshot
.anchor_in_excerpt(*excerpt_id, cached_hint.position)
{ {
to_insert.push(Inlay::hint( to_insert.push(Inlay::hint(
cached_hint_id.id(), cached_hint_id.id(),
multi_buffer_snapshot.anchor_in_excerpt( anchor,
*excerpt_id,
cached_hint.position,
),
&cached_hint, &cached_hint,
)); ));
} }
}
excerpt_cache.next(); excerpt_cache.next();
} }
cmp::Ordering::Greater => return true, cmp::Ordering::Greater => return true,
@ -483,15 +484,18 @@ impl InlayHintCache {
let maybe_missed_cached_hint = &excerpt_cached_hints.hints_by_id[cached_hint_id]; let maybe_missed_cached_hint = &excerpt_cached_hints.hints_by_id[cached_hint_id];
let cached_hint_kind = maybe_missed_cached_hint.kind; let cached_hint_kind = maybe_missed_cached_hint.kind;
if !old_kinds.contains(&cached_hint_kind) && new_kinds.contains(&cached_hint_kind) { if !old_kinds.contains(&cached_hint_kind) && new_kinds.contains(&cached_hint_kind) {
if let Some(anchor) = multi_buffer_snapshot
.anchor_in_excerpt(*excerpt_id, maybe_missed_cached_hint.position)
{
to_insert.push(Inlay::hint( to_insert.push(Inlay::hint(
cached_hint_id.id(), cached_hint_id.id(),
multi_buffer_snapshot anchor,
.anchor_in_excerpt(*excerpt_id, maybe_missed_cached_hint.position),
&maybe_missed_cached_hint, &maybe_missed_cached_hint,
)); ));
} }
} }
} }
}
to_remove.extend( to_remove.extend(
shown_hints_to_remove shown_hints_to_remove
@ -1200,12 +1204,14 @@ fn apply_hint_update(
.allowed_hint_kinds .allowed_hint_kinds
.contains(&new_hint.kind) .contains(&new_hint.kind)
{ {
let new_hint_position = if let Some(new_hint_position) =
multi_buffer_snapshot.anchor_in_excerpt(query.excerpt_id, new_hint.position); multi_buffer_snapshot.anchor_in_excerpt(query.excerpt_id, new_hint.position)
{
splice splice
.to_insert .to_insert
.push(Inlay::hint(new_inlay_id, new_hint_position, &new_hint)); .push(Inlay::hint(new_inlay_id, new_hint_position, &new_hint));
} }
}
let new_id = InlayId::Hint(new_inlay_id); let new_id = InlayId::Hint(new_inlay_id);
cached_excerpt_hints.hints_by_id.insert(new_id, new_hint); cached_excerpt_hints.hints_by_id.insert(new_id, new_hint);
cached_excerpt_hints cached_excerpt_hints

4
crates/editor/src/items.rs
View file

@ -1154,8 +1154,8 @@ impl SearchableItem for Editor {
let end = excerpt let end = excerpt
.buffer .buffer
.anchor_before(excerpt_range.start + range.end); .anchor_before(excerpt_range.start + range.end);
buffer.anchor_in_excerpt(excerpt.id, start) buffer.anchor_in_excerpt(excerpt.id, start).unwrap()
..buffer.anchor_in_excerpt(excerpt.id, end) ..buffer.anchor_in_excerpt(excerpt.id, end).unwrap()
}), }),
); );
} }

8
crates/language_tools/src/syntax_tree_view.rs
View file

@ -227,8 +227,12 @@ impl SyntaxTreeView {
let multibuffer = editor_state.editor.read(cx).buffer(); let multibuffer = editor_state.editor.read(cx).buffer();
let multibuffer = multibuffer.read(cx).snapshot(cx); let multibuffer = multibuffer.read(cx).snapshot(cx);
let excerpt_id = buffer_state.excerpt_id; let excerpt_id = buffer_state.excerpt_id;
let range = multibuffer.anchor_in_excerpt(excerpt_id, range.start) let range = multibuffer
..multibuffer.anchor_in_excerpt(excerpt_id, range.end); .anchor_in_excerpt(excerpt_id, range.start)
.unwrap()
..multibuffer
.anchor_in_excerpt(excerpt_id, range.end)
.unwrap();
// Update the editor with the anchor range. // Update the editor with the anchor range.
editor_state.editor.update(cx, |editor, cx| { editor_state.editor.update(cx, |editor, cx| {

30
crates/multi_buffer/src/multi_buffer.rs
View file

@ -2788,7 +2788,13 @@ impl MultiBufferSnapshot {
} }
} }
pub fn anchor_in_excerpt(&self, excerpt_id: ExcerptId, text_anchor: text::Anchor) -> Anchor { /// Returns an anchor for the given excerpt and text anchor,
/// returns None if the excerpt_id is no longer valid.
pub fn anchor_in_excerpt(
&self,
excerpt_id: ExcerptId,
text_anchor: text::Anchor,
) -> Option<Anchor> {
let locator = self.excerpt_locator_for_id(excerpt_id); let locator = self.excerpt_locator_for_id(excerpt_id);
let mut cursor = self.excerpts.cursor::<Option<&Locator>>(); let mut cursor = self.excerpts.cursor::<Option<&Locator>>();
cursor.seek(locator, Bias::Left, &()); cursor.seek(locator, Bias::Left, &());
@ -2796,14 +2802,14 @@ impl MultiBufferSnapshot {
if excerpt.id == excerpt_id { if excerpt.id == excerpt_id {
let text_anchor = excerpt.clip_anchor(text_anchor); let text_anchor = excerpt.clip_anchor(text_anchor);
drop(cursor); drop(cursor);
return Anchor { return Some(Anchor {
buffer_id: Some(excerpt.buffer_id), buffer_id: Some(excerpt.buffer_id),
excerpt_id, excerpt_id,
text_anchor, text_anchor,
}; });
} }
} }
panic!("excerpt not found"); None
} }
pub fn can_resolve(&self, anchor: &Anchor) -> bool { pub fn can_resolve(&self, anchor: &Anchor) -> bool {
@ -3283,14 +3289,16 @@ impl MultiBufferSnapshot {
outline outline
.items .items
.into_iter() .into_iter()
.map(|item| OutlineItem { .flat_map(|item| {
Some(OutlineItem {
depth: item.depth, depth: item.depth,
range: self.anchor_in_excerpt(*excerpt_id, item.range.start) range: self.anchor_in_excerpt(*excerpt_id, item.range.start)?
..self.anchor_in_excerpt(*excerpt_id, item.range.end), ..self.anchor_in_excerpt(*excerpt_id, item.range.end)?,
text: item.text, text: item.text,
highlight_ranges: item.highlight_ranges, highlight_ranges: item.highlight_ranges,
name_ranges: item.name_ranges, name_ranges: item.name_ranges,
}) })
})
.collect(), .collect(),
)) ))
} }
@ -3310,14 +3318,16 @@ impl MultiBufferSnapshot {
.symbols_containing(anchor.text_anchor, theme) .symbols_containing(anchor.text_anchor, theme)
.into_iter() .into_iter()
.flatten() .flatten()
.map(|item| OutlineItem { .flat_map(|item| {
Some(OutlineItem {
depth: item.depth, depth: item.depth,
range: self.anchor_in_excerpt(excerpt_id, item.range.start) range: self.anchor_in_excerpt(excerpt_id, item.range.start)?
..self.anchor_in_excerpt(excerpt_id, item.range.end), ..self.anchor_in_excerpt(excerpt_id, item.range.end)?,
text: item.text, text: item.text,
highlight_ranges: item.highlight_ranges, highlight_ranges: item.highlight_ranges,
name_ranges: item.name_ranges, name_ranges: item.name_ranges,
}) })
})
.collect(), .collect(),
)) ))
} }