Token scope prefers $ over other symbols #2048
Conversation
packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/TokenScopeHandler.ts
Outdated
Show resolved
Hide resolved
| : testRegex(identifierMatcher, document.getText(scopeB.domain)) | ||
| ? false | ||
| : undefined; | ||
| if (testRegex(identifierMatcher, textA)) { |
There was a problem hiding this comment.
I'm also a bit surprised by our logic here. Why do we immediately prefer a if it's an identifier; what if b is also an identifier? I would expect us to return undefined in that case 🤔. I guess you're just mirroring the same logic here but seems wrong to me
There was a problem hiding this comment.
True. It just isn't possible for two identifiers to be adjacent so it still works.
This function doesn't even need to return undefined. It's just a question if you should prefer the first one over the second. False does the same thing as undefined.
There was a problem hiding this comment.
Ohh right yeah I guess we never hit that with two identifiers
False and undefined are different I believe. False means prefer b; undefined means no preference so let modifier decide
I would argue we should be correct here now that we actually can have them adjacent
There was a problem hiding this comment.
I'm just saying that today we don't actually utilize that. We could simplify the function signature and still have the same behavior as today.
There was a problem hiding this comment.
But I have now updated the code to reflect the actual logic
Fixes #1321
Checklist