-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Delete matching bracket upon backspace #1267
Conversation
Bringing fork up to date
and here is the vscode issue |
I didn't know that. If you still prefer to wait, I won't hem and haw about it. |
I will let @johnfn decide, I think it would resolve a few open issues The problem is without the proper vscode fix, multicursor stuff is tougher I think, do you know if your fix works with multi cursors? Also, your way is nice! |
Is there a way that you can wrap the functionality using the editor option of whether a user has the "editor.autoClosingBrackets": true? I think that this is great, might be worth adding a comment to the vscode issue to see what the status is or if they have any thoughts |
It now checks for |
I would also add single and double quotes as well for this behavior. And < is not included in the default vscode behavior |
I added double, single and weird-thing ( |
regarding weird-thing (back-tick) and <, what I meant was if you disable the vim plugin, and try various ones, only some of them actually auto delete no big deal, would just leave it as is for now until @johnfn can give input |
First of all, thanks for the PR. This is definitely an issue that has been annoying a lot of people for a long time. Unfortunately, this problem is actually an annoying one. How does it work with stuff like you type in "(blah)" but then backspace blah and the (? We will somehow need to distinguish a ( that was just entered from a ( which was entered a few keystrokes ago. The reason that I have held off solving this is that the correct solution to the problem is to perform all text edits and deletes via vscode api, rather than in the manual way that we are currently doing. Sadly, I believe this would require a large refactor to do correctly, so it's probably better to just manually cover these cases for now. |
You're welcome! In this case @johnfn, it behaves the same if you enter "()" and backspace the "(", as well as "(blah)" and backspace the "(blah". In both cases, it removes the matching bracket. I believed this to be the desired behavior. |
Whoops, I never realized that. Alright, this seems good to me then. :) |
src/matching/matcher.ts
Outdated
// Don't delete bracket unless autoClosingBrackets is set | ||
if (!vscode.workspace.getConfiguration().get("editor.autoClosingBrackets")) { return undefined; } | ||
|
||
let deleteRange = new vscode.Range(currentPosition, currentPosition.getLeftThroughLineBreaks()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make all these declarations const
.
src/matching/matcher.ts
Outdated
|
||
if ("{[(\"'`".indexOf(deleteText) > -1) { | ||
let matchPosition = currentPosition.add(new PositionDiff(0, 1)); | ||
if (matchPosition) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need this if
statement?
Oops, I always forget to release my comments. I have some small suggestions for improvement, but we should be fine here for the most part. |
Thanks for suggestions @johnfn. Done. |
this also fixes #1241 |
@johnfn Is there something I am missing with this? |
Nope, it's fine :) Dunno if I mentioned this, but apologies for any late responses - I am currently on vacation, hopping all around and not really using my laptop all too much. |
#1228
When in insert mode, if a user is deleting an opening bracket, and a matching closing bracket is immediately after it, also delete the closing bracket. Corresponding test included.