Fix #49777 - Emmet balance In after balance out should go back to initial selection and not first child #49996
Conversation
|
|
There was a problem hiding this comment.
Hi @Heldenkrieger01,
Thanks for the PR and your first contribution! Its a good start. I have left 2 comments for cases when the fix will not work as the stack being maintained gets stale.
My proposal:
- Have a variable that will contain the selections that are applied at the end of every balance out command
- When a new balance in/out command is run, compare current selections with the above. If they are the same, then they all belong to the same stack, else clear the stack.
- When a balance in command is run and the stack is empty, then clear the above variable and use the
getRangeToBalanceInmethod to determine the selections
Thoughts?
extensions/emmet/src/balance.ts
Outdated
| @@ -56,30 +73,3 @@ function getRangeToBalanceOut(document: vscode.TextDocument, selection: vscode.S | |||
| } | |||
| return selection; | |||
| } | |||
|
|
|||
| function getRangeToBalanceIn(document: vscode.TextDocument, selection: vscode.Selection, rootNode: HtmlNode): vscode.Selection { | |||
There was a problem hiding this comment.
This function getRangeToBalanceIn is needed.
Take the case when the user selects an html node and starts balancing inwards. Then the beforeEmmetSelection array will be empty and the command would fail. In this case, we should use getRangeToBalanceIn
extensions/emmet/src/balance.ts
Outdated
| @@ -7,6 +7,10 @@ import * as vscode from 'vscode'; | |||
| import { HtmlNode } from 'EmmetNode'; | |||
| import { getNode, parseDocument, validate } from './util'; | |||
|
|
|||
| let balanceStack: Array<vscode.Selection[]> = []; | |||
There was a problem hiding this comment.
In the current code, the balanceStack is never reset. Take the case, where the user runs a series of balance out operations. Then they continue with some other work. After some time, they select some text and try to balance in. The current code will pop the stack and give wrong selections
Or take the case of a series of balance out, escape, then another series of balance out in a separate part of the file. Now when you balance in, the stack has selections from the first set which would be wrong
|
@Heldenkrieger01 I have made the necessary changes |
This should fix #49777 according to the instructions of @ramya-rao-a.
This is my first contribution, I hope everything is alright 😃.