cmd+click symbol definition to find all references does not work in vim mode

[Congyuwang]

Check for existing issues

• Completed

Describe the bug / provide steps to reproduce it

When "vim_mode": true, cmd + click only GoToDefinition but does not FindAllReference when clicking on def (in INSERT or NORMAL mode).

If vim mode is disabled, however, the bidirectional cmd + click works properly.

2024-04-11.09.24.52.mov

__

Environment

Zed: v0.131.1 (Zed Preview)
OS: macOS 14.4.1
Memory: 16 GiB
Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

Bidirectional cmd + click should work also in vim mode. Related #9259 .

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

No response

[Congyuwang]

I wonder whether this is reproducible by others?

[mrnugget]

Yeah, I can reproduce it.

[Congyuwang]

I investigated the code. The current implementation: after cmd+clicking, if definition is found, check whether the selected definition range covers where the cursor previously was before the definition is revealed (if definition is not revealed elsewhere, it is revealed right here, so I am clicking right on the definition, so let's do FindAllReference).

zed/crates/editor/src/hover_links.rs

Lines 215 to 253 in c81eb41

	fn revealed_elsewhere(
	editor: &mut Editor,
	before_revealing: Range<Anchor>,
	cx: &mut ViewContext<'_, Editor>,
	) -> bool {
	let multi_buffer_snapshot = editor.buffer().read(cx).snapshot(cx);
	let selection_after_revealing = editor.selections.newest::<Point>(cx);
	let after_revealing_head = selection_after_revealing.head();
	let after_revealing_tail = selection_after_revealing.tail();
	let after_revealing = match after_revealing_tail.cmp(&after_revealing_head) {
	cmp::Ordering::Equal | cmp::Ordering::Less => {
	multi_buffer_snapshot.anchor_after(after_revealing_tail)
	..multi_buffer_snapshot.anchor_before(after_revealing_head)
	}
	cmp::Ordering::Greater => {
	multi_buffer_snapshot.anchor_after(after_revealing_head)
	..multi_buffer_snapshot.anchor_before(after_revealing_tail)
	}
	};
	let before_intersects_after_range = (before_revealing
	.start
	.cmp(&after_revealing.start, &multi_buffer_snapshot)
	.is_ge()
	&& before_revealing
	.start
	.cmp(&after_revealing.end, &multi_buffer_snapshot)
	.is_le())
	|| (before_revealing
	.end
	.cmp(&after_revealing.start, &multi_buffer_snapshot)
	.is_ge()
	&& before_revealing
	.end
	.cmp(&after_revealing.end, &multi_buffer_snapshot)
	.is_le());
	!before_intersects_after_range
	}

This is working fine when vim mode is false. However, when vim mode is enabled, let selection_after_revealing = editor.selections.newest::<Point>(cx); does not select the whole definition range, but only the first character of the definition, which does not overlap with the old cursor position, and so revealed_elsewhere = ! before_intersects_after_range is now true, and FindAllReference is not performed.

[Congyuwang]

Should vim go to VISUAL mode when jumping to definition?

[mrnugget]

Should vim go to VISUAL mode when jumping to definition?

Hmmm, I'm not sure but that sounds wrong. I wonder whether we could change the check to say that if vim mode { if current_cursor already within the range of the definition } or something like that.