Quote completion improvements

[madskristensen]

Summary

The way C# handles quote completion differs from other editors like VS Code, but also from other languages in VS such as C++.

Background and Motivation

Visual Studio users have noticed that the behaviour around quotes is less than ideal. A feature request to improve the behavior has gotten 15 votes. The issue experienced by users is that in certain situations, VS should not auto-complete quotes. It’s frustrating to users when it does and affects their overall satisfaction with the IDE.

Here’s what some users say about the current behavior:

Proposed Feature

The quotation characters in question are double quote (") and single quote ('). The user has made no selection in the editor.

VS must not auto complete after user types a quotation character in these cases:

1. Cursor immediately after alphanumeric character – P1 a. Example: text<cursor>
2. Cursor immediately before alphanumeric character – P1 a. Example: <cursor>text
3. In code comments and strings – P1 a. Example: // comment <cursor> b. Current behavior – no change required

VS must auto complete in these cases:

1. Cursor surrounded by non-alphanumeric characters including whitespace – P1 a. Example 1: some <cursor> text b. Example 2: if (<cursor>) c. Current behavior – no change required
2. Cursor immediately after $ or @ in C# if followed by non-alphanumeric character – P2 a. Example: sb.AppendLine($<cursor>);

[CyrusNajmabadi]

@madskristensen the linked issue is for ts/js. The syntax:

is def not legal C# (there is no { ... } syntax to start an object literal in our langauge, like there is in js/ts). Should this move to ts/js?

[madskristensen]

That's from a Razor file.

The issue is in the C# language service. I just added a gif to the description that illustrates the issue. This is specific to C# since C++ and JS doesn't have this behavior

[CyrusNajmabadi]

I'm not sure how i feel about this change. The use case is very suspect. Namely:

Why would we believe that something is a string just because it starts with a identifier char. Take, for example: (this is a string). Why would that not have the same behavior?

I guess i'm just very skeptical that people start with string contents taht are initially not quoted, and then want to add quotes after.

[sitefinitysteve]

class="somename
<press quote>
class="somename"" 

You hit backspace, it deletes both quotes, so the only way forward is ArrowRight->backspace. So you've turned 1 keystroke into 3 and amped up the frustration.

At the moment it's actively fighting with the IDE to get what should be obvious, especially if there's a copilot brain somewhere trying to figure out what we're going to do.

Regardless at this point I'd be satisfied if this happened

class="somename
<press quote>
class="somename""
<backspace>
class="somename" (both qutoes are not deleted)

The above "string test = this is a test" is a bad example, people don't do that, but it IS an example where what you would expect to happen doesn't happen. I want one quote, then END, and put another quote, OR have it just assume the wrap (not ideal). It's like right now it's just DOUBLE QUOTE ALL THE THINGS! Oh wait you pressed backspace, you must not want ANY quotes, bloop, gone. No I do want a quote, HERE'S THE DOUBLE QUOTES AGAIN!

Obviously most things work fine when you are just flowing and doing var test = "something" or class="something", the quotes pop up, cursor inside, it's great. It's the NOT that scenario that's the problem.

..just hey you plopped in 2, I press delete, kill 1 of them and I can move on.

[CyrusNajmabadi]

@sitefinitysteve what language are you typing in? Can you give a more complete example? Thanks!

[madskristensen]

More info from one of the comments on a duplicate feature request https://developercommunity.visualstudio.com/t/Provide-options-to-control-quote-complet/1652026#T-N1705116

And here's another https://developercommunity.visualstudio.com/t/Quotation-Marks-Shouldnt-Double-When-Te/1590583

[CyrusNajmabadi]

@madskristensen I don't understand several of these reports. For example:

This then kicks off intellisense and that slows everything down as it cascades througout.

That doesn't sound like a roslyn case. Similarly, class="somename doesn't seem like roslyn either, as that doesn't look like code that would apply to us at all.

[madskristensen]

My proposed feature bullet list illustrates the use cases too. People keep telling us that quote completion is annoying for C#, it's not consistent with other languages in VS like C++, and it doesn't follow industry standards which breaks muscle memory.

[CyrusNajmabadi]

My proposed feature bullet list illustrates the use cases too

Cursor immediately before alphanumeric character – P1

As mentioned, i don't particularly agree with this approach. I don't even really see how you get into that state. I also don't know why alphanumerics are of particular interest, given that a string's contents could be anything.

it's not consistent with other languages in VS like C++

That's a separate discussion IMO. For all i know they may need to be more consistent with us.

and it doesn't follow industry standards

What industry standards exist for this?

which breaks muscle memory.

Muscle memory across disparate tools is not a goal. The same issues go in reverse. VS has behaved in certain ways for decades. But that doesn't mean other tools need to behave like us. As an example, vscode (while much newer) often behaves in very different ways. There's no right/wrong on particular typing metaphors in these disparate tools. If we want to have parity with them, we could consider options here (including asking someone explicitly if they want VS to behave like certain other tools). But we will have just as many problems breaking muscle memory here if we change our own behaviors (not speculative. when we do make those changes, we do get similar gnashing of teeth).

[CyrusNajmabadi]

@madskristensen My point is this: pointing to other languages, and how they are potentially getting things wrong (esp. wrt undo behavior) is just confusing here. Primarily because it makes it seem as if those issues are present with roslyn, even when tehy are specific to other tools. Reporting them here doesn't address anything because even if we make changes on our end, those issues in other components will still be present.

[madskristensen]

Here's an example of turning an expression into a string. I start by adding a quote after the expression. It auto-inserts a closing quote as well. That is not helpful. Then I do the same in the front of the expression, and the same thing happens. Not helpful.

This is adding negative value, because I would have been better off without the quote completion feature. The simple rules I proposed fixes use cases like these, brings the logic on par with C++ in VS and other editors like VS Code.

[CyrusNajmabadi]

Then I do the same in the front of the expression, and the same thing happens. Not helpful.

The presumption though is that you are attempting to turn an expression into a string. It could just as well be that you are trying to make var name = ""; and intending to push the next bit to the next line.

, brings the logic on par with C++ in VS and other editors like VS Code.

I get that. But the presumption is also that the valid use cases where this is helpful are ok to break. It would be just as valid to say: when i'm in C++/vscode, i want this to work so that i can easily write a string like the above, and not get interference.

In particular, C++ is not an editor we look to to try to have parity with.

--

The one area i could potentially see this being ok would be only the end of hte line (so no following content), and after something that can't legally be followed by a ". This would at least be a signal that there is possible intent of reverse wrapping with quotes.

[CyrusNajmabadi]

As an example, using your cases:

Cursor surrounded by non-alphanumeric characters including whitespace – P1 a. Example 1: some text

I don't see why this wouldn't have the same behavior as the alphanumeric case.

[madskristensen]

The presumption though is that you are attempting to turn an expression into a string. It could just as well be that you are trying to make var name = ""; and intending to push the next bit to the next line.

Theoretically yes, but you'd probably start with a blank space - especially once you know how this works. Status quo means that you'll have a high level of false positives. I believe it better to optimize for the most probable scenario and not every possible scenario.

[madskristensen]

I don't see why this wouldn't have the same behavior as the alphanumeric case.

The idea is to make it predictable with the simple rule that when cursor is surrounded by whitespace or braces/brackets, then it auto-completes, otherwise it won't.

[CyrusNajmabadi]

Theoretically yes, but you'd probably start with a blank space

I literally just wrote that in the last 20 minutes. Without a blank space. :)

[CyrusNajmabadi]

The idea is to make it predictable with the simple rule that when cursor is surrounded by whitespace or braces/brackets, then it auto-completes, otherwise it won't.

I would be ok with this as an option. But it would likely need to be opt-in to ensure we're not breaking muscle memory on our own side.

[CyrusNajmabadi]

I believe it better to optimize for the most probable scenario

I genuinely don't see this as "more probable". IN other words, surrounding with quotes means that users are writing unquoted strings to start with, and then deciding to quote them. That seems less likely than them just entering new strings.

The case where this may differ is someone copying something like an expr, to pass to something like an assert call, and wanting to then have a quoted version of that expr. But that seems much rarer.

[CyrusNajmabadi]

Note: i don't like the proposed algorithm either. say i have the following:

Now i want to complete the string by typing in the space before 'info'. Because it is surrounded by whitespace, this would now complete out to two-quotes. But i think i should only get one quote here (which is what roslyn does today).

[madskristensen]

Note: i don't like the proposed algorithm either. say i have the following...

How can it be improved?

@mikadumont want to chime in?

[madskristensen]

My proposal might not be crisp enough. The general idea is to make auto-completion less aggressive. If there are situations where my proposal makes it more aggressive, then that wasn't the intension and should be improved.

[CyrusNajmabadi]

Thanks!

[madskristensen]

I'm thinking that an experiment with real users trying it might be the best way to iterate toward a final proposal. I'll look into making an extension next week to start the process

[madskristensen]

Here's what I'm starting out with https://github.com/madskristensen/QuoteCompletionFix/blob/master/src/TypingCommandHandler.cs

[CyrusNajmabadi]

can you make sure the use case of typing """ to get """""" (a raw string, with the caret in the middle) is not broken?

similarly for continuing to type quotes in the middle of that to have that raw string grow on both ends.

[madskristensen]

Yes, absolutely.

[Peter-Juhasz]

Sharing my thoughts about the subject:

Copying other editors Going in this direction would result in a significantly worse experience, since VS and VS Code play in a different league.

In VS, especially with Roslyn, there is a state-of-the-art syntax model, and fine-grained control over brace completion.

While in VS Code (more broadly LSP), even the most trivial language service features like syntax highlighting are based on regular expressions by design, rather than a parser (which is just ridiculous imo). So is automatic completion of braces/quotes, they are controlled by the editor itself regardless the language or the actual location in the syntax tree, whether the completion is valid or not, completely out of context.

So, copying something inferior where not even the infrastructure exists to do it the right way, just would not make sense. (Although I get the trade-offs for the LSP design, but for finetuning, which is the requirement here, this is not the way to go.)

Language specific Continuing the previous point, but from a language perspective instead of the editor: I strongly believe that all language features, including quote completion should be specific to each language (and thus controlled by the language service itself), rather than trying to hack a text-based general solution for "all languages". Because it is specific to the grammar of the language, while adjusting a general solution back-and-forth would render it buggy for all languages at the end of the day.

If there is room for improvement, it should be implemented by the Roslyn language service for C#/VB, using the already available and robust syntax model.

The same way as if the reported issues are for Razor, or C++, or even a need for generalizing behaviors across languages, they should be solved there locally, not in general without proper syntax context.

Regarding the use case As a longtime VS user, in my experience, VS+C#/Roslyn provides by far the most fluent typing experience (including committing completions fluently, automatic formatting, etc.) ever built, and already superior to other editors.

Quoting a C#/VB expression (or even an unquoted "to be" string) to be a string literal is a very unlikely use case, and the general quote completion experience should not be optimized for this edge case. (Providing a better way for the example provided is even more difficult, because the next tokens are keywords.)

For Razor, quotes are far from the biggest issues. I would see more value if resource allocation would be considered for significantly improving the Razor editor experience in general.

[CyrusNajmabadi]

Several fixes made in https://github.com/dotnet/roslyn/pull/72217. Still keeping this open for the general case where the user is typing in a place where a string is legal, but existing code is there. We'd like to explore making this contextual, so if it makes sense that existing code would be fine to push-to-the-right (like in an argument list), that we still complete the stirng. Buf it doesn't make sense to push-to-the-right (like in an initializer) then we don't complete the string.

[Dean-NC]

For Razor, quotes are far from the biggest issues. I would see more value if resource allocation would be considered for significantly improving the Razor editor experience in general.

I completely agree with this. There are times when the automatic closing of quotes is not what I want (mostly in Razor but also in .cs files), but the constant fighting that the Razor editor does with intellisense and also overwriting the HTML attribute or tag name I just typed (when I press space), is very obstructive to the point I can no longer just type without watching to see if what I previously typed will be replaced.

[CyrusNajmabadi]

@Dean-NC contributions welcome :-)