compulim / vscode-closetag

Quickly close last opened HTML/XML tag in Visual Studio Code
14 stars 5 forks source link

Keep cursor position after inserting closing tag in closeHTMLTagInPlace #12

Closed rbolsius closed 6 years ago

rbolsius commented 7 years ago

Proposed fix for https://github.com/compulim/vscode-closetag/issues/11. Tested in VS Code 1.13.1 with single and multiple cursors, both with and without empty selections.

Now when closeTag.closeHTMLTagInPlace is called with an empty selection (just a cursor), the cursor stays in place before the closing tag as expected. Also, if there is a non-empty selection and the selected text is replaced with the closing tag, then the closing tag will remain selected.

rbolsius commented 6 years ago

The modified fix that was actually merged to 1.2.0 is not working either. It appears that assigning directly to the individual elements of textEditor.selections does not trigger a selection changed event in VS Code.

          prevSelections.forEach((selection, index) => {
            // If a range of text was selected previously, then just keep it
            // since the new replacement text will be selected already.
            // Otherwise, restore the previous cursor position.

            if (selection.isEmpty) {
              textEditor.selections[index] = selection;
            }
          });

The proposed fix worked around this limitation/behavior by assigning all the selections to a separate variable (currentSelections) and then assigning modified selections array back to textEditor.selections.

      const currentSelections = textEditor.selections; 
      // Assign the non-empty selections to currentSelections[index]
      textEditor.selections = currentSelections;

Not sure if there is a cleaner way, but I had also tried simply assigning each non-empty selection to textEditor.selections[index] originally, but I found that VS Code would not recognize the selections unless textEditor.selections itself is assigned.

rbolsius commented 6 years ago

Although the tests in extension.test.js do confirm that textEditor.selections has the correct values after the command is run, unfortunately, the selection that the editor displays does not match what is in the selections array. So, even though the tests are passing, the command is still not working the way it is intended.

It looks like VS code only recognizes the textEditor.selections changes if textEditor.selections itself is reassigned. Assigning directly to the elements (textEditor.selections[i]) but not textEditor.selections does not seem to influence what the editor shows as selected.

I tried the following change on top of the 1.2.0 code to make the closeHTMLTagInPlace command work the way it was intended:

          const currentSelections = textEditor.selections;

          prevSelections.forEach((selection, index) => {
            // If a range of text was selected previously, then just keep it
            // since the new replacement text will be selected already.
            // Otherwise, restore the previous cursor position.

            if (selection.isEmpty) {
              currentSelections[index] = selection;
            }
          });

          textEditor.selections = currentSelections;

This has the intended effect of changing the selections so that the cursors are inside the tags.

As far as I can tell, there is no easy way to update the tests to recognize what the editor actually thinks the selections are. So, the current tests will pass with either fix regardless whether VS Code actually recognizes the selection changes.