Make a clearer distinction between experimental + preview settings

[timsneath]

Describe the bug
Typing ==( generated overeager completion

To Reproduce
See attached movie file
https://user-images.githubusercontent.com/2319867/110220049-f1ed4980-7e77-11eb-9804-10415d1bc655.mov

Expected behavior
No insertion of autocomplete results

Versions (please complete the following information):

• VS Code version: 1.54.1
• Dart extension version: 3.20.1
• Dart/Flutter SDK version: 2.0
• LSP preview enabled

[DanTup]

Do you happen to have the dart.previewCommitCharacters setting enabled? I can only repro this that ticked.

Right now, that setting is known to be a bit eager - VS Code/LSP specs don't have a lot of control when it comes to commit characters (which is why it's intended to be behind a preview flag, in the hope that improves).

[timsneath]

Yes, I routinely turn previews on in the hope that I can help uncover issues before our customers :) I turned it off. I wonder if we can be clearer somehow of the difference between "preview == early release: please try it out" and "preview == the best we can do for now, we're not planning to make this the default"? It seems like this is the latter, rather than the former.

BTW, having turned this off, DartCode now seems very overeager with parentheses (if I select something and type (, it wraps the selection with parentheses, rather than overwriting the selection with what I typed).

[DanTup]

I wonder if we can be clearer somehow of the difference between "preview == early release: please try it out" and "preview == the best we can do for now, we're not planning to make this the default"?

Yes, I think that's fair. It's really the same for previewFlutterUiGuidelines - it's a preview flag but has enough significant quirks that it wouldn't be great to default on.

The commit characters setting does have "EXPERIMENTAL:" in the description, but I wonder if it should also be in the setting name so it's more visible if editing the JSON. We could use "preview" as a prefix for things in the first category you described, and experimental for the second?

Eg. rename dart.previewCommitCharacters -> dart.experimentalCommitCharacters? It's probably a bit late to change it for the flutterUiGuidelines though, since there are many more people with that enabled already.

DartCode now seems very overeager with parentheses (if I select something and type (, it wraps the selection with parentheses, rather than overwriting the selection with what I typed).

This shouldn't depend on that flag, but it is configured at the language level for "auto surround". This is intended to make it easy to surround expressions with brackets/quotes/etc.

The current pairs are:

"surroundingPairs": [
	["{", "}"],
	["[", "]"],
	["(", ")"],
	["<", ">"],
	["'", "'"],
	["\"", "\""],
	["`", "`"]
]

I'm not sure there was any significant logic to what's included there (in fact some of them - like backticks - look somewhat bogus like this was just copied from TypeScript). I don't use this feature a lot at all, but my feeling is that everything other than the quotes is probably reasonable to drop (users can override this in their VS Code settings if they disagree). If you think that seems fair, lmk and I'll change it and file an issue to ensure it's called out in the release notes.

[github-actions]

This issue has been marked stale because it is tagged awaiting-info for 30 days with no activity. Remove the stale label or comment to prevent the issue being closed in 10 days.

[DanTup]

Renaming previewCommitCharacters is actually a little tricky - in addition to people using it, it's in the LSP server now (renaming it would mean we'd have to declare both in Dart-Code, as settings are declared in the manifest and can't be changed dynamically based on the SDK version you have).

So, I'll make it clear from the descriptions that they're experimental, and going forwards try to be more consistent with the naming:

• experimentalX - something that may have quirks and would not ship in its current form
• previewX - something planned to ship and just initially opt-in for collecting additional feedback before becoming default

[DanTup]

I've also opened #3264 to review the surroundingPairs.

[DanTup]

For existing "preview" settings that are really experimental, I've prefixed the descriptions with EXPERIMENTAL: to make it clearer. For future settings, I'll aim to include this in the setting names so it's also more obvious there.

Existing "preview" flags that are now described as experimental include:

• previewCommitCharacters
• previewFlutterUiGuidelines
• previewFlutterUiGuidesCustomTracking