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

Adding close paren inside parent pair overwrites close (right) paren #37315

Closed
glennpeters opened this issue Oct 31, 2017 · 59 comments
Closed

Adding close paren inside parent pair overwrites close (right) paren #37315

glennpeters opened this issue Oct 31, 2017 · 59 comments
Assignees
Labels
editor-autoclosing Editor automatic closing of parens / brackets / etc. feature-request Request for new features or functionality on-testplan
Milestone

Comments

@glennpeters
Copy link

  • VSCode Version: Code 1.17.2 (b813d12, 2017-10-16T13:59:46.104Z)
  • OS Version: Windows_NT x64 10.0.14393
  • Extensions:
Extension Author (truncated) Version
vue liu 0.1.5
csharp ms- 1.12.1
sublime-keybindings ms- 3.0.2
debugger-for-chrome msj 3.4.0
vetur oct 0.10.1

Steps to Reproduce:

  1. Enter a close paren structure inside existing parentheses, e.g. :
    if (reportGroup.filters) {
    to
    if (Array.isArray(reportGroup.filters) {

image

  1. Typing a close paren ")" after "filters" consistently overwrites the existing close paren, leaving the code in a broken state. The code is only fixed after typing an additional second paren after the cursor has moved past the first.

I can't think of any reason for this behavior to remain in the editor, it seems completely contrary to what I would expect in a smart editor.

Reproduces without extensions: Yes/No

@vscodebot vscodebot bot added editor editor-bracket-matching Editor brace matching labels Oct 31, 2017
@octref
Copy link
Contributor

octref commented Oct 31, 2017

/cc @alexandrudima Don't know if this belongs to #11222

@alexdima alexdima added editor-autoclosing Editor automatic closing of parens / brackets / etc. and removed editor-bracket-matching Editor brace matching labels Nov 1, 2017
@alexdima
Copy link
Member

alexdima commented Nov 1, 2017

@glennpeters You can turn this behaviour off via "editor.autoClosingBrackets": false

@glennpeters
Copy link
Author

Thank you. But I didn't want to turn off auto-closing of parens, I just wanted to turn off the overwriting of existing parens.

@alexdima alexdima added the feature-request Request for new features or functionality label Nov 13, 2017
@alexdima alexdima added this to the Backlog milestone Nov 13, 2017
@alexdima alexdima removed the editor label Nov 23, 2017
@alexdima alexdima removed their assignment Apr 17, 2018
@JacksonKearl JacksonKearl self-assigned this Jun 14, 2018
@JacksonKearl
Copy link
Contributor

JacksonKearl commented Jun 15, 2018

There are three components to autoClosingBrackets:

  1. When you type an opening bracket, the closing one immediately appears after your cursor

This means that you can type function foo( and have the paren automatically be closed. However, if by habit you type the closing paren anyways, you would have two parens, and end up with function foo( ... )). This gives rise to:

  1. If you type a closing bracket when the character immediately after your cursor is that same closing bracket, it will overwrite rather than duplicate.

On the other hand, if you decide to delete function foo(, after the closing paren had been added, you would have an extra character at the end of the line. This gives rise to the third component:

  1. If you delete an opening bracket when a corresponding closing bracket is immediately right of your cursor, that bracket will be deleted as well.

It'd be easy enough to add options to micro-configure these three components, but I'm not sure that the feature makes sense without any single component.

It seems to me that in order to address the problems of unintentional swallowing and hyper-aggressive deleting, the editor would need to maintain some sort of hidden state pairing all auto-closed brackets and their corresponding opening brackets. I feel that would give rise to more confusion than it's worth, but if someone wants to take a look at providing a decent solution, relevant lines are:

Part 1:
https://github.com/Microsoft/vscode/blob/be4cb382d46cf735edb497d57409f0f5a4be5985/src/vs/editor/common/controller/cursorTypeOperations.ts#L569

Part 2:
https://github.com/Microsoft/vscode/blob/be4cb382d46cf735edb497d57409f0f5a4be5985/src/vs/editor/common/controller/cursorTypeOperations.ts#L482

Part 3:
https://github.com/Microsoft/vscode/blob/be4cb382d46cf735edb497d57409f0f5a4be5985/src/vs/editor/common/controller/cursorDeleteOperations.ts#L82

@jkyeung
Copy link

jkyeung commented Jun 20, 2018

I want to amplify JacksonKearl's point 1, which was

This means that you can type function foo( and have the paren automatically be closed. However, if by habit you type the closing paren anyways, you would have two [close] parens
I would argue that many people who type the closing paren "anyways" don't do it by habit per se, it's because it's the most natural way to advance to the next thing you have to type. Let me ask a question for the people who don't type the close paren:

How the heck do you advance the cursor past the close paren that is already there due to autoclose? Do you take your hand off the home row and press the right arrow key? Do you take your hand off the home row and press End? Do you take your hand off the home row and click with the mouse? Are you seeing a theme here? I'm a touch typist, and I am genuinely curious if people really find that leaving the home row is somehow faster or preserves flow better than just typing the close paren. Or maybe there is some other fast, convenient, and logical way to advance the cursor that I don't know about, and if that's the case, I'd like someone to clue me in on that too.

Personally, I think it's more useful to think of autoclose primarily as a way to help prevent you from forgetting to close your opening paren, and if sometimes it saves you from having to type the close paren, that's a bonus.

@Mcmartelle
Copy link

The three components to autoClosingBrackets make sense when typing new code, but when there is a missing closing bracket for whatever reason and the cursor is moved to the left of an existing closing bracket by arrow keys or a mouse, the cursor moving past the existing closing bracket is frustrating. A setting might be something like "editor.autoClosingBrackets.SkipAfterCursorMovement": false, If the previous key pressed was an arrow key or mouse click -> don't skip it.

@JacksonKearl
Copy link
Contributor

@Mcmartelle I do like that idea of disabling bracket swallowing after arrows and clicks. Doesn’t get all the cases but it does help.

@nojvek
Copy link
Contributor

nojvek commented Sep 15, 2018

I experience this almost everyday and rant on vscode. Why they hell is it swallowing my keystrokes? The mac keyboard is already badly engineered that it doesn't send the keystrokes until you hit it hard, vscode swallowing it is just an 😡😡😡 experience .

a(b(c(d(<cursorhere>))) - Closing the parenthesis here requires four ) keystrokes before vscode finally gets it.

The design should be simple. DON'T SWALLOW KEY STROKES. It makes the users feel stupid.

autoClosing is an amazing feature. It is orthogonal to this. autoClosing helps me save so many keystrokes. Once you switch it on, the brain knows not to type the closing ones. BUT sometimes you need to type it. Respect the user's decision please. Immediately show the character on the screen. The user's brain will auto adjust when it gets immediate feedback.

  1. is more bug than a feature IMHO

@jkyeung
Copy link

jkyeung commented Sep 15, 2018

@nojvek - I am a bit perplexed why you gave me a thumbs up, but then you rant against everything that I wrote. I am guessing you really didn't understand me at all.

@nojvek
Copy link
Contributor

nojvek commented Sep 16, 2018

Ha! Although I disagree with you, I like your ranting spirit 🙃

@jkyeung
Copy link

jkyeung commented Sep 16, 2018

@nojvek - I can respect that. But it appears I no longer have your thumbs up, so does that mean I wasn't "ranty" enough, or that you disagreed with the content too strongly after all to reward my spirit?

@jkyeung
Copy link

jkyeung commented Jul 1, 2019

@esr360 - I understand your frustration, but I think it's important to understand that the feature DOES work as designed. It's not disingenuous to claim that at all. Keep in mind that the behavior is deliberately modeled after what Sublime Text does. Sublime Text is not only a popular editor that is several years older than VS Code, it is a commercial product that people have paid real money for.

Now, it may well be that you think both Sublime Text and VS Code are stupid for doing this. It is very easy to find people who agree with you. But ultimately, there are also plenty of people who disagree with you (and have voted with their money, in the case of Sublime Text). Jackson Kearl has graciously offered his time and effort to provide a way to turn the bracket-swallowing behavior off, so you can help yourself, and him, and fellow VS Code users who agree with you, by trying out his modifications (at PR #74686) and providing feedback.

@esr360
Copy link

esr360 commented Jul 1, 2019

@jkyeung I guess being in this thread acted like an echo chamber for me where I felt more people had issue with this than didn't (given that no one has disliked the initial comment, can you blame me?)

I'm not sure it's productive to use the fact that this bug feature existed in Sublime Text as justification that people actually want it. If more people prefer it this way, then who am I to argue that? I'm not convinced they do, though. From where I'm sitting, this is an annoyance that has been around for so long that people merely tolerate it without questioning it (again, I'm happy to consider I'm wrong here).

I'd love to help out, but beyond providing rationals, my open-source backlog is already impossible to keep on-top of, so I won't be able to make this a priority. Hopefully my opinions are valued nonetheless.

@jkyeung
Copy link

jkyeung commented Jul 1, 2019

@esr360 - I'm not justifying anything. I'm explaining. Given that Sublime Text is popular, it is perfectly reasonable for later software to emulate its behavior. It is also certainly reasonable for some people not to like it, and for other software not to emulate its behavior.

This thread is an echo chamber, which is why I periodically post to it, to try to attenuate the reverberations. The people who are happy with the status quo most likely don't know about this thread and have no reason to look for it. So it's only natural that most commenters to the thread will be those unhappy with the status quo. More generally, people who have a complaint (about whatever) are usually the loudest (about that thing).

I do get the feeling Jackson Kearl is taking everyone's opinion seriously and working in good faith to improve the situation. As far as I understand, PR #74686 does what is asked (provide a setting to opt out of the unwanted behavior). At this point, I think it's mainly just a matter of what people want it to look like in their settings file. For example, Niryo has essentially (not in so many words) made the following suggestion:

"editor.autoClosingBrackets": true; // default
"editor.autoOverrideBrackets": false;

The current title of this issue also serves to suggest "editor.autoOverwriteBrackets": false.

Anyone who has thoughts on naming is welcome to provide them. (I don't know whether it would be preferable to post them here or at #74686. I would guess at the PR, since that is where the code is, but maybe @JacksonKearl will clarify.)

@JacksonKearl
Copy link
Contributor

If you want the "disable swallowing always" option to exist, you should upvote #74686 and, if possible, leave some ideas about what the setting name/interface/docs/etc. should be.

I think this issue should be left open and dedicated to discussing "smart" strategies.

@Arshia001
Copy link

@JacksonKearl smart as in "keep stack of character pairs, empty the stack when cursor moves out"?

@louis925
Copy link

louis925 commented Jul 3, 2019

I remember the old good day Visual Studio on Windows doesn't have this issue and still can auto close bracket properly (smartly). Perhaps, microsoft can ask their different team how that works?

@JoeCreates
Copy link

This drives me mad, too, but the solution isn't to remove swallowing altogether. As some have noted this wouldn't be correct for those who like to type the closing bracket.

The solution is to keep track of which characters are auto-added. Only swallow an auto-added character. Clear the record of auto-added characters once you click or move off the line.

@alexdima alexdima self-assigned this Jul 25, 2019
@alexdima alexdima modified the milestones: Backlog, July 2019 Jul 25, 2019
@alexdima
Copy link
Member

Thanks for your patience. I've tried to tackle this one, and the only solution I could come up with is to make auto-closing pairs stateful.

So the editor now remembers which exact characters it has auto-inserted and only the characters inserted automatically by the editor will be overtyped. Furthermore, if the cursor leaves the precise area between the starting and the closing character, the auto-inserted character will be considered "confirmed" and it will no longer be overtyped.

Here are some examples:

1. simple overtype -- ) gets auto inserted and is then overtyped:

2. multiple overtypes -- multiple ) are auto inserted and are then overtyped

3. does not overtype characters it hasn't inserted

4. leaving the area confirms the closing char

@JacksonKearl
Copy link
Contributor

@alexandrudima oh my goodness this is incredible. Will there be a config option?

@alexdima
Copy link
Member

I would try to avoid a config option. Tomorrow's insiders build will have the change and I hope to gather some feedback until next week to see if the new behavior is upsetting to anyone and a setting is needed for the old behavior.

@louis925
Copy link

Wonderful!

@JoeCreates
Copy link

JoeCreates commented Jul 25, 2019 via email

@Arshia001
Copy link

Perfect! Thanks!

@turbobuilt
Copy link

I think this feature is pretty cool sometimes, but it just needs to be syntax aware. So if I type a close parenthesis, it should detect if a close parenthesis is needed and if so actually type a new parenthesis. Otherwise, if no parenthesis is needed, it shouldn't be swallowed

@rhyek
Copy link

rhyek commented Aug 20, 2019

@alexandrudima I'm currently running 1.37.1 and I believe I am experiencing the new behavior you describe. I believe this new feature should be optional since I find I regularly navigate back to modify string parameters to like a console.log() or something, for example, and I'd just prefer the closing parenthesis to always be overwritten. It allows me to write code faster instead of having to reach for the right arrow key. To me it was perfect how it was before and feel quite sluggish now when writing code.

I think there should be an option to opt out of this new behavior. Thank you for your help!

@alexdima
Copy link
Member

@rhyek Let's track in #78833

@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-autoclosing Editor automatic closing of parens / brackets / etc. feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

No branches or pull requests