Skip to content

Unexpected token appears to be included when I say 'flash every token this' #2889

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

Closed
jaresty opened this issue Apr 16, 2025 · 7 comments · Fixed by #2893
Closed

Unexpected token appears to be included when I say 'flash every token this' #2889

jaresty opened this issue Apr 16, 2025 · 7 comments · Fixed by #2893
Assignees

Comments

@jaresty
Copy link
Contributor

jaresty commented Apr 16, 2025

When I have several tokens selected in the tale of three lines, and I say 'flash every token this' it includes the first token in the last line which is not in the selection. I expected that it would only flash the tokens that were within the selection. I'm including a video so you can see what I'm talking about. Is this the expected behavior?

Screen.Recording.2025-04-15.at.8.29.40.PM.mov
@AndreasArvidsson
Copy link
Member

The expected behavior is that all tokens on all three lines are flashed. That is the behavior I get when I try this as well.

Let's break down "flash every token line"

  1. no mark specified. Start at selection.
    For each selection:
  2. line. Expand to line
  3. every token. Use every token
  4. flash: Flash final targets

@jaresty jaresty changed the title Unexpected token appears to be included when I say 'flash every token line' Unexpected token appears to be included when I say 'flash every token this' Apr 16, 2025
@jaresty
Copy link
Contributor Author

jaresty commented Apr 16, 2025

Sorry, I miswrote my original message - I was saying 'flash every token this'

@AndreasArvidsson
Copy link
Member

That is own purpose, but might be a bit esoteric. The idea is that if you have a single scope selected and issue and every we ignore your current selection.

Code:

if (
scopes.length === 1 &&
scopes[0].domain.contains(target.contentRange) &&
!target.hasExplicitScopeType
) {
// If the only scope that came back completely contains the input target
// range, we treat the input as if it had no explicit range, expanding
// to default iteration scope below
scopes = undefined;
}

Example test case:
https://github.com/cursorless-dev/cursorless/blob/3e8473a193f775bc3d0a763b4a1f21356b8b1746/data/fixtures/recorded/modifiers/everyScope/clearEveryToken.yml

When you only have a single selection this logic make totally sense, but the behavior with multiple selections is a bit unexpected. @pokey How do you feel about different behaviors depending on if you have one or many input targets?

@pokey
Copy link
Member

pokey commented Apr 16, 2025

Yeah I've always had mixed feelings about that heuristic. Multiple cursors is not a terrible idea for when to drop the heuristic, tho do we have that information easily available at the modifier stage?

@AndreasArvidsson
Copy link
Member

Not today. This would be a change to pass that additional information.

@jaresty
Copy link
Contributor Author

jaresty commented Apr 17, 2025

For what it's worth it does seem to preserve the selection for the first set of cursors and only ignores the selection for the last one (which agrees with your description above, but is definitely a little bit odd- as an experience)

@AndreasArvidsson
Copy link
Member

Yeah that is not good UX. Here is a fix
#2893

AndreasArvidsson added a commit that referenced this issue May 6, 2025
Unfortunately became quite a lot of changes to pass the modifier
options, but I think this is the only way. It also opens up for more
context aware modifiers in the future.

Fixes #2889
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants