Vscode: Adding close paren inside parent pair overwrites close (right) paren

Created on 31 Oct 2017  Ā·  59Comments  Ā·  Source: microsoft/vscode

  • VSCode Version: Code 1.17.2 (b813d12980308015bcd2b3a2f6efa5c810c33ba5, 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

editor-autoclosing feature-request on-testplan

Most helpful comment

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.

All 59 comments

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

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

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.

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

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.

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.

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

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.

2) is more bug than a feature IMHO

@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.

Ha! Although I disagree with you, I like your ranting spirit šŸ™ƒ

@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?

A few months ago, I wrote:

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.

Though no one responded to that, I did stumble upon what at least some people are doing, other than arrowing or mousing:

  1. Using the TabOut extension: #22864

  2. Rebinding some home-row key combination to do what the right arrow does (or the TabOut extension, if you like its behavior but don't like using the Tab key for this purpose): #8154

I find the current behavior particularly annoying when refactoring code. Something as simple as replacing a value argument with a call turns into a slow and painful hunt for the correct location to insert the closing brace without it being stolen again.
I wish it would at the very least stop disregarding me after I move the cursor back before the current closing brace and try again to insert the new closing brace. That is the behavior exhibited by most auto-complete software.

But why make things so complicated:) adding a prop "neverOverrideRightBrackets" or something like that will solve the problem for all the guys like me that don't want the editor to override their right brackets. Simple as that. When I want to skip over the right bracket I simply press the 'end' key (with my right pinky, for those who really like the juicy details ).
Then people could choose if they want this feature or not, and if they want it they at least understand why it overrides their right brackets- because they ask for it.

After solving the simple case, we can continue and think about a smarter prop, like "smartOverrideRightBrackets". But currently it is very very annoying to do refactoring, and there is no way around it.

not easy to use compared to atom

I agree, this is a extremely annoying "feature". Please add on option to disable this. Give me a text editor with content assist. If I want to go to the right of a ) then I will press the right arrow key. You know, the key designed to go to the right. Not a totally different key that even requires to press a modifier key to type that special character. Its like "Hey, you already typed that, let me just do a totally different thing". What if you misspelled a word that contains a double letter and try to fix it?eg "mispelled" -> "misspelled" and when you try to enter another "s" it just jumps over the existing one "Hey, its already there!".
This feature caused me more headache than benefit.
Automatically opening/closing parenthesis, brackets, etc are good and a totally different feature.

This is so incredibly annoying.
Also the other way around: Why the hell does it delete the both parentheses when I type () and remove only the left one.
Who ever thought this was a good idea or helpful in any way?

Overwriting close (right) parens is a very annoying and unnecessary feature. Many of my colleagues put a closing bracket right after opening a little less than never.

UIs in general should not try to second guess users by disregarding user input.
That is a sorry trend in Microsoft Word which I sincerely hope will not infect vscode too.

Why :clap: is :clap: this :clap: still :clap: an :clap: issue

Annoyed

A year and a half (and some more) later...

A note for future readers: if you propose a solution to the issues I have listed in https://github.com/microsoft/vscode/issues/37315#issuecomment-397482291, @-mention me and I'll take a look at implementing said solution. Until then, I see no path forward that is inline with the goals of the product, and saying "me too" won't make such a path appear.

I am a volunteer contributor, but I am invested in getting this right to the extent that I will take a look at any proposed solutions, and implement any that are viable. For a solution to be "viable", it must not rely on maintaining any complicated state at the cursor layer.

Aside: I no longer am employed by Microsoft, and as such I do not speak for the VSCode team. That being said, I worked on features related to this during my time there, and I have a feel for the amount of additional complexity the team is willing to maintain to support "sane" behavior in this edge case. That amount is proportional to the number of upvotes on this issue, so if you're passionate about this, but don't have an idea of how to implement it, your best bet is upvoting the root issue.

Aside 2: I've created a partial implementation based on suggestions to disable the swallowing feature and leave the rest alone. You can see the issues that creates here.

@JacksonKearl Would my "disabling bracket swallowing after arrows and clicks" idea be considered "maintaining any complicated state at the cursor layer" and therefore not be a viable solution?

On a different note, it seems that those of us that don't type a closing character to advance the cursor NEVER do it, so adding an option to completely disable it would be fine in my opinion.

@jkyeung to actually answer your question

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.

I usually advance the cursor with the tab key while using snippets. Object and array literals tend to be multiline, so there's no issue there. Making empty [] {} and "" are probably the only time I would think of just typing the closing character. (I'll look into that TabOut extension, thanks)

Clearly, there are some people who like the existing behavior, at least well enough. I daresay most Sublime Text users (many of whom actually paid real money for it) like it, which no doubt contributed to the VS Code team deciding to try to replicate it in the first place. So, folks who are in favor of some kind of bracket-swallowing are probably reasonably happy with it already, and are not dying to refine its behavior, especially if it would be difficult to develop and maintain. And the chance of introducing some kind of bug just seems overwhelmingly high to me.

On the other side of the fence, people who don't like it would probably be happy enough if there was no bracket-swallowing whatsoever. And presumably this should be relatively safe and straightforward to implement as a user setting.

So I agree with @Mcmartelle that an option to completely disable would be fine. And I'm actually against spending any effort on making the bracket-swallowing any more "clever" than it already is.


Aside to @Mcmartelle - perhaps interestingly, the multiline case is actually why I personally disable bracket autoclose (and thus also bracket-swallowing). There is no way to turn off the feature where pressing Enter to split an empty matched pair of brackets inserts a newline both before and after the cursor simultaneously; and when that happens, there is no easy, touch-typing-flow-preserving way to even get the cursor to the closing bracket so that it can then be swallowed by typing over it.


And living without bracket autoclose in VS Code makes me realize another thing: The primary function of bracket autoclose in VS Code is probably to minimize disruptions to the syntax highlighting. The secondary function is to help prevent forgetting to close a bracket. And way, way, way down in a very distant third place is to save having to type a closing bracket.

@Mcmartelle and @jkyeung see my branch here for a config option that allows you to never swallow characters. It breaks a few key patterns that to me make it a no-go:
1) Breaks creating empty pairs (the empty string, an empty arg-list, the empty array, etc), you get """, ()), and []], respectively. You could get around this by testing if the prior character is the opener, but this is just more awkward special casing for limited benefit. (ETA: I wouldn't be totally opposed to trying that as a partial solution though, if that sounds reasonable to people)
2) Causes weird behavior while deleting: if you have "", then you place a cursor in the middle, you'll get """. So far so good. But if you then press backspace, you get ". Not good. This is what I was getting at when I said that the 3 components of autoclosing are all tied together, and removing any one causes weirdness. You could get around this by removing the pair-deletion logic, but then typing ( then backspace leaves you with ). So thats a no-go too.

Re: disabling after arrows and clicks, I had forgotten about that suggestion. From my memory of the structuring of the code, that could be easier said than done. I will however look into it. Disabling after arrow keys might be a good, simple, partial implementation.

Edited to change case 2 to use quotes rather than brackets, as the behavior may not be as described with brackets, but should be as described with quotes.

How does the editor decide when to insert a closing character and when to skip doing it? I think it should only override the closing character when it was inserted as a consequence of the opening character being typed. Then, maybe with a bit more work on deciding when to insert the closing character (personally I'm OK with the way it's currently handled), we could have a solution to this problem.


And way, way, way down in a very distant third place is to save having to type a closing bracket.

I always type over the auto-inserted closing characters anyway, and I assume there are others who do so as well. To me, this behavior has one specific function: it keeps the code more readable for me when I'm typing it.

I think it should only override the closing character when it was inserted as a consequence of the opening character being typed

Yes, but keeping track of why each bracket was inserted is exactly the kind of state I’d like to avoid maintaining.

A stack based approach would be interesting to explore and in theory pretty simple, but it (crucially) needs to be flushed at the right times (whenever you leave the inside of the auto-closed region, probably). @Mcmartelle suggested clearing it in arrow keys or clicks, this would mostly work but the implementation isn’t as simple as one would like.

Aside: if someone could figure out how Atom does it, that night he worth taking a look at. I tried once, a while ago, but I couldn’t find anything in the base repo. I might have missed it, or it might be part of a built in extension.

@JacksonKearl I don't follow. Doesn't the editor already maintain state in the form of a mapping of opening characters to corresponding closing ones? What I proposed is possible by simply keeping the new opening character out of this mapping.


So I just tried it, and it seems no such state is maintained. But I think implementing it should be simple enough.

@JacksonKearl Hi, wouldn't simply checking whether all the enclosing brackets (or pairs of that type) are currently matched be sufficient (and fast) for most cases? I.e., if all the enclosing brackets are currently balanced, then adding a close bracket would skip, otherwise it'd insert one. Similarly for deleting an opening bracket: if the enclosing brackets are balanced, then delete the closing bracket, otherwise don't.

I’m of the opinion that we should never ever swallow keystrokes. I acknowledge some people like the behavior so it should probably be an option not to upset those that rely on it.

A simple option like editor.swallowKeystrokes = false or something.

Even if I open a bracket and editor auto-closes it, the ui feedback is immediate, I know I don’t have to put a closing one.

Let’s keep it simple please. This has been open for more than an Year and is probably the most annoying experience in every day usage.

@JacksonKearl - First let me say I really appreciate the level of thought and care you are giving to this issue. Regardless of what actual implementation changes result from this thread, if any, I recognize your investment.

Re: Your latest points about the brokenness and/or weirdness of disabling swallowing in isolation from autoclosing and pair-deletion:

It breaks a few key patterns that to me make it a no-go:

1. Breaks creating empty pairs (the empty string, an empty arg-list, the empty array, etc), you get `"""`, `())`, and `[]]`, respectively.

I'm not sure what you mean. Pretty much everyone wants to always keep autoclose enabled. So yes, with swallowing disabled, if you type an opening bracket and then a closing bracket, then the proper and expected behavior is that you wind up with one opening bracket followed by two closing brackets, with the cursor between the two closing brackets. Honestly, the people clamoring for a way to completely disable swallowing are calling for precisely this behavior. They don't want the bracket to be swallowed UNDER ANY CIRCUMSTANCES. Of course, approximately 100% of these people will not type the closing bracket anyway, and thus will not wind up with two closing brackets, just the one which was provided by autoclose. So this is a nonissue.

2. Causes weird behavior while deleting: if you have `""`, then you place a cursor in the middle [and type another quotation mark], you'll get `"""`. So far so good. But if you then press backspace, you get `"`. Not good. This is what I was getting at when I said that the 3 components of autoclosing are all tied together, and removing any one causes weirdness. You could get around this by removing the pair-deletion logic, but then typing `(` then backspace leaves you with `)`. So thats a no-go too.

I get that you think of autoclose, bracket-swallowing, and pair-deletion as three aspects of a unified concept. But there are other people who do not. They see three distinct, orthogonal, uncoupled behaviors. Or they'll couple two of those three and not the other, and I think which two will depend on who you ask. I'm sure there are some people who think pair-deletion is a logical complement to autoclose and others who think it is weird and disconcerting (why did I lose two characters when I pressed backspace only once? I want my keystrokes to be respected, dammit! Except when I type an opening bracket, in which case I want the editor to give me two for one).

I haven't delved into the implementation of VS Code, so I don't really know for sure what's easy to do and what's not. I know from experience that sometimes a feature sounds easy, but because of various architectural choices, is actually quite difficult. So if it is reasonably feasible, my recommendation is to just go ahead and make the three behaviors micro-configurable. It doesn't matter that not every possible combination of settings makes sense to you. I'm confident every combination will make sense to someone.

@jkyeung, @nojvek are you able to build the branch I created to demo the implementation? If not I can build a binary for you if you give me your operating system.

https://github.com/JacksonKearl/vscode/tree/broken-autoclosing-swallowing-fix-demo

I can try it out by building it. Any chance you can create a PR on Vscode so we can see the diff ?

I'd rather not post a PR to the main repo, given it's just a test branch (with myriad poor names and lacking documentation to boot). But you can see the diff here. You can add ".patch" or ".diff" to the end of that url to get it in a different format.

Edit: eh what the heck. Posted a PR. Link should be below.

As I and other users already suggested, The only simple, easy, and complete solution will be adding a settings option to never override closing brackets. it will solve the problem for those how hate this behaviour, and those who like it will not be effected.

@Niryo and others who want a never swallow option, that’s what the PR #74686 is. You can test it out and leave your feedback in the PR.

Setting would help solve the problem for everyone. people who are used to advance via typing closing bracket and some us who prefer hitting the cursor key. the thing is currently causes bugs. it becomes typical for me to expect it upon compilation/run time failure oh must be a missing parenthesis. Here is a related discussion on stackexchange: https://stackoverflow.com/questions/45573132/how-to-make-vscode-stop-overriding-closing-parentheses-on-insertion it shows there are two sides to this so setting will likely help.

There was a previous suggestion:

Re: disabling after arrows and clicks, I had forgotten about that suggestion. From my memory of the structuring of the code, that could be easier said than done. I will however look into it. Disabling after arrow keys might be a good, simple, partial implementation.

Were you able to look into this approach at all? To me this sounds like exactly what I would want.

If you can't ship a working feature, don't ship the feature. Pretending the existing behaviour is by design and saying "we can't do anything until someone comes up with a solution" is disingenuous; just disable the auto-closing brackets feature until you can do it in a way that the users want - if you can't, don't offer it as a feature. It's extremely annoying to myself and seemingly many other users when I go to type a bracket and VS-Code wrongly assumes I don't know what I'm doing.

@esr360 you may disable ā€œeditor.autoClosingBracketsā€ in your preferences. I’d like it to be better too, but many are fine with the current implementation, myself included.

@JacksonKearl I will certainly consider disabling it, but my point is that it's enabled by default, and it doesn't work properly. Surely it would be better to opt-in to something like this.

@JacksonKearl , the autoClosingBrackets feature is not the problem. the problem is with the autoOverrideBrackets. It's annoying, and sadly we don't have an autoOverrideBrackets setting to turn off.

An autoClosingBrackets feature should do exactly what it named for- putting a closing bracket immediately after typing an open bracket. the override thing should be considered as a bug.

@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.

@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.

@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.)

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.

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

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?

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.

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

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

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.

Wonderful!

This is great news. Thanks!

Perfect! Thanks!

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

@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!

@rhyek Let's track in #78833

Was this page helpful?
0 / 5 - 0 ratings

Related issues

chrisdias picture chrisdias  Ā·  3Comments

curtw picture curtw  Ā·  3Comments

lukehoban picture lukehoban  Ā·  3Comments

trstringer picture trstringer  Ā·  3Comments

sijad picture sijad  Ā·  3Comments