Skip to content
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

Correct highlight of replaced variables/splats with braces #984

Merged

Conversation

msftrncs
Copy link
Collaborator

@msftrncs msftrncs commented Jul 31, 2019

  • Prevent adjustment of userCompletionText in FindUserCompletionTextPosition when the match contains { in CompletionText[1] if the userCompletionText is not longer than 1 char
  • Preserve the splat sigil at start of completions of type Variable.

Adjusting userCompletionText blindly led to ArgumentOutOfRangeException on incorrectly braced splats on input @?.
Loss of the sigil of splats led to the splat sigil always being marked as replaced, and in situations where the PowerShell completion logic returned incorrect completions of splats with braces, led to a ArgumentOutOfRangeException.

Fix #983.

@msftrncs
Copy link
Collaborator Author

msftrncs commented Aug 1, 2019

I think I figured out what the check for a { is doing … I'll probably implement a different correction.

The detection for a { appears to be to handle the situation where @en is the input, and then when @{env:blah(blah)} is the completion, the selection becomes 'v:blah(blah)}`, not selecting the first brace.

Prevent adjustment of userCompletionText while finding userCompletion text
position when match contains `{` in CompletionText[1] if the
userCompletionText is not longer than 1 char, and preserve the splat sigil
at start of completions of type `Variable`.

Adjusting userCompletionText blindly led to ArgumentOutOfRangeException on
incorrectly braced splats on input `@?`.
@msftrncs msftrncs force-pushed the FixVarAndSplatBraceHighlight branch from 6dee015 to 70d0d34 Compare August 1, 2019 02:29
@msftrncs
Copy link
Collaborator Author

msftrncs commented Aug 1, 2019

I've amended the commit for this PR. Its very possible that either of the two changes would stop the ArgumentOutOfRangeException from occuring. The correction for maintaining the sigil of the returned completions is probably the item that solves the problem, but the other change is a safety net. (See comments below)

Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@daxian-dbw
Copy link
Member

The correction for maintaining the sigil of the returned completions is probably the item that solves the problem, but the other change is a safety net.

The correction for maintaining the sigil of the returned userCompletionText is the item that solves the ArgumentOutOfRangeException, but the other change is needed to have the correct selection for ${$} when typing echo $Ctrl+Space.

@daxian-dbw daxian-dbw merged commit 76f9c4c into PowerShell:master Sep 6, 2019
@msftrncs msftrncs deleted the FixVarAndSplatBraceHighlight branch September 7, 2019 03:59
@msftrncs
Copy link
Collaborator Author

msftrncs commented Sep 7, 2019

@daxian-dbw, you are right, I was so focused on the @ sigil, I overlooked the importance the other change (to FindUserCompletionTextPosition) had on the $ selection when the { was present in the completion.

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 this pull request may close these issues.

ArgumentOutOfRangeException: Completion of Splat, but Not $
2 participants