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

Review surroundingPairs in language config #3264

Closed
DanTup opened this issue Apr 8, 2021 · 5 comments
Closed

Review surroundingPairs in language config #3264

DanTup opened this issue Apr 8, 2021 · 5 comments
Labels
in editor Relates to code editing or language features is enhancement

Comments

@DanTup
Copy link
Member

DanTup commented Apr 8, 2021

See #3183 (comment).

@DanTup DanTup added this to the v3.22.0 milestone Apr 8, 2021
@DanTup DanTup added the in editor Relates to code editing or language features label Apr 8, 2021
@DanTup
Copy link
Member Author

DanTup commented Apr 26, 2021

After reviewing this and testing again, I'm not sure removing things like parens is the best idea. Other similar languages have these settings, and there is a VS Code user setting that allows changing these. If we remove the parens, users can't add them back - whereas if we leave it as-is, the user can switch to only quotes in the VS Code settings if they want.

So I think the best options are:

  • Don't change anything - allow users to just change the Auto Surround setting if they wish
  • Don't change the language options, but do override the default for AutoSurround for Dart to only quotes. This would initially have the same behaviour as updating surroundingPairs, but it would still allow the user to have the full set (by changing autoSurround back to languageDefined for ["dart"]).

@timsneath - any opinions on this (since you noted it at #3183 (comment))?

@timsneath
Copy link
Contributor

timsneath commented Apr 26, 2021

I don't have a ton of opinions here -- I trust your judgement! My own experiences were improved considerably when I turned off the experimental settings.

In general, I find it really useful to have the editor help me when I'm typing a line of code -- for example, when I type the open parenthesis on a new line of code and it sets me up with the parameters I need. But it's really frustrating when I'm editing an existing line of code and tab to complete a method only to have it insert or overwrite the remainder of the line. But perhaps others have either a better intuition for what the editor is about to do than I do...

@DanTup
Copy link
Member Author

DanTup commented Apr 26, 2021

I think there are two related, but different things here.

For the surroundingPairs (the behaviour when you have a selection and press (, " etc. which surrounds the text), unless you feel strongly I'm inclined to leave it as-is, since the editor has a setting to override this (and I think based on your other issue you may prefer to have it set to "quotes"):

Screenshot 2021-04-26 at 18 48 43

I think the other comments relate to completion, and when we insert the parens/argument placeholders automatically - and I agree there's room for improvement there too. There are some cases we try to handle to avoid inserting them when they're already there after the caret, though I'm not sure it covers enough cases (I saw some unwanted parents inserted just today). I've filed #3303 as a reminder to take a look. Thanks!

@DanTup
Copy link
Member Author

DanTup commented Apr 26, 2021

(I'll close this one and leave as-is, but if it comes up more, I'll consider the second option above, leaving the language to define all of the pairs, but defaulting to quotes instead of languageDefined so people can change it back).

@DanTup DanTup closed this as completed Apr 26, 2021
@DanTup DanTup removed this from the v3.22.0 milestone Apr 26, 2021
@DanTup
Copy link
Member Author

DanTup commented Apr 27, 2021

I just landed a fix for #3303 in dart-lang/sdk@8f2c0e9 that prevents inserting the parens/arg placeholders when they already exist for the target you're completing.

However, re-reading what you wrote:

only to have it insert or overwrite the remainder of the line.

I don't know if I misunderstood and there's something else going on here. Perhaps you mean this:

^foo(bah);

You complete at ^ and want to wrap the existing code in a new function call, but it thinks you want to replace foo with something else? If so, there's a VS code setting that controls whether to insert or replace by default (we default to replace). Holding down Ctrl or Cmd should change this, it will insert instead (this lets you change per-completion, without having to change the setting). If this is what you meant and that doesn't work for you, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features is enhancement
Projects
None yet
Development

No branches or pull requests

2 participants