zed icon indicating copy to clipboard operation
zed copied to clipboard

Do not select target range going to definition

Open Congyuwang opened this issue 1 year ago • 4 comments

Release Notes:

-Fixed #11347 , do not select target range going to definition. Just place the cursor at the start of target range.

Congyuwang avatar May 11 '24 07:05 Congyuwang

Seems to me that when using VIM mode, only the first character got selected anyway. So that letting the cursor go to the beginning of the range of definition, seems more consistent across VIM and non-VIM mode.

Congyuwang avatar May 11 '24 09:05 Congyuwang

@Congyuwang thanks for this!

It seems like there's a few cases to consider:

  • If you jump to definition of a non-module (e.g. a function, field, type, variable). It currently highlights the name of the definition not the whole definition.
  • If you jump to the definition of a rust module it currently highlights the entire file (#11347)

I think this change will change the behavior in both of these cases; which might be ok, but I wonder if we should more narrowly special case this to just work-around the bug in rust analyzer. i.e. if the returned range is > 1 line, make it just highlight the first character, otherwise keep the selection?

Otherwise we could deliberately not highlight the definition when clicking on it, but it is kind of nice that that behavior matches search (even if it differs from vim).

What do you think?

ConradIrwin avatar May 14 '24 22:05 ConradIrwin

I think that would be good.

Congyuwang avatar May 15 '24 06:05 Congyuwang

It now works nicely!

Congyuwang avatar May 15 '24 07:05 Congyuwang

Just noticed that the old release note is not very accurate.

Congyuwang avatar May 15 '24 15:05 Congyuwang

Just noticed that the old release note is not very accurate.

About to build the release notes for today's release - what are you thinking the release note for this item should be?

JosephTLyons avatar May 15 '24 15:05 JosephTLyons

Maybe the following:

Release Notes:

-Fixed https://github.com/zed-industries/zed/issues/11347 , Do not select target range going to definition when the range has more than one line.

Congyuwang avatar May 15 '24 16:05 Congyuwang