correct text navigation in VB and C# strings #945
Conversation
|
test this please |
|
Tagging @Pilchie @DustinCampbell @rchande @dpoeschl @jasonmalinowski @balajikris @basoundr as potential reviewers. Also, it looks like Jenkins messed itself trying to build. I'll keep trying periodically. |
| Dim Span = New Span(position.Position, 1) | ||
| Return New TextExtent(New SnapshotSpan(position.Snapshot, Span), isSignificant:=True) | ||
| Else | ||
| Return MyBase.GetExtentOfWordFromToken(token, position) |
There was a problem hiding this comment.
Since this (and the stringliteraltoken case of IsWithinNaturalLanguage) are essentially the same in C# and VB, is there a way we can just push this up into the base type?
There was a problem hiding this comment.
Since (C#)SyntaxKind.StringLiteralToken != (VB)SyntaxKind.StringLiteralToken I didn't see a clean way of doing this so I opted to keep them separate, but I could certainly be talked either way.
|
👍 |
| return position > token.SpanStart; | ||
| // This, in combination with the override of GetExtentOfWordFromToken() below, treats the closing | ||
| // quote as a separate token. This maintains behavior with VS2013. | ||
| if (position == token.Span.End - 1 && token.Text.EndsWith("\"")) |
There was a problem hiding this comment.
Hmm, makes me want to have an overload of EndsWith that takes a char.
brettfo
force-pushed
the
vb-string-navigation
branch
from
b987a4c to
00c79a7
Compare
|
Important test here: make sure you can Ctrl+Left arrow and Ctrl+Right arrow back and forth across the file. I'm sure you've already done that, but you really want to make sure you don't break that... |
|
@jasonmalinowski yep, that works. Truth be told I fiddled with it until I could navigate like I expected then I just made the tests pass to match. |
| @@ -254,7 +254,7 @@ public void String() | |||
| TestString, | |||
| pos: TestString.LastIndexOf('"'), | |||
| isSignificant: true, | |||
| start: startOfString + lengthOfStringIncludingQuotes - 1, length: 2); | |||
| start: startOfString + lengthOfStringIncludingQuotes - 1, length: 1); | |||
|
|
|||
| AssertExtent( | |||
| TestString, | |||
There was a problem hiding this comment.
We already had tests for C# interpolated strings that were correct; only regular/raw strings needed to be updated to treat the closing quote as a single token.
|
👍 |
| return position > token.SpanStart; | ||
| // This, in combination with the override of GetExtentOfWordFromToken() below, treats the closing | ||
| // quote as a separate token. This maintains behavior with VS2013. | ||
| if (position == token.Span.End - 1 && token.Text.EndsWith("\"", StringComparison.Ordinal)) |
There was a problem hiding this comment.
Consider replacing with a single 'return'.
There was a problem hiding this comment.
I tried changing this to a single return statement but since the condition needed to be inverted it was difficult to read/understand, so for readability's sake I'm going to leave this.
brettfo
force-pushed
the
vb-string-navigation
branch
from
00c79a7 to
c4a4da2
Compare
correct text navigation in VB and C# strings
Correct text navigation in VB and C# strings to match that of the old language service (i.e., VS 2013.) Now when navigating inside of strings with Ctrl+LeftArrow, RightArrow the cursor will move on word/token boundaries and Ctrl+Delete, Backspace behave appropriately as well as double-click word selection.
Fixes #551.