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

Auto Rename Tag #130

Closed
bmix opened this issue Apr 28, 2019 · 16 comments · Fixed by #207
Closed

Auto Rename Tag #130

bmix opened this issue Apr 28, 2019 · 16 comments · Fixed by #207
Assignees
Labels
completion enhancement New feature or request
Milestone

Comments

@bmix
Copy link

bmix commented Apr 28, 2019

I just realized, with the help of @angelozerr at lsp4xml/#362, that synced renaming and deletion of two tags, that belong together, may best be done via some simple text manipulation on the client side. I now have installed a little extension into VSCode, that does just that (minus paired delete, sadly), but it may well fit also into your project, so to keep things together.

@fbricon
Copy link
Collaborator

fbricon commented Nov 29, 2019

@angelozerr @xorye @NikolasKomonen that feature is now available for HTML tags: microsoft/vscode#47069. Would be nice to extend it to XML as well.

@angelozerr
Copy link
Contributor

angelozerr commented Nov 29, 2019

Thanks @fbricon for this great news! It seems we should support

  • a custom request MatchingTagPositionRequest on LSP4XML (server side)
  • that we should consume on vscode-xml (on client side).

See commit at microsoft/vscode@0d25d0a

But pay attention with this feature, it seems that there are some trouble microsoft/vscode#85715

@octref
Copy link

octref commented Dec 6, 2019

@angelozerr The bugs you pointed are already fixed. To implement this feature for XML, you can support MatchingTagPositionRequest on the server and reference the implementation in our HTML server.

@angelozerr
Copy link
Contributor

The bugs you pointed are already fixed.

Thanks for the information!

To implement this feature for XML, you can support MatchingTagPositionRequest on the server and reference the implementation in our HTML server.

What do you mean with reference the implementation in our HTML server ?

@fbricon I think we should really implement this very cool feature.

@octref
Copy link

octref commented Dec 6, 2019

@angelozerr https://github.com/microsoft/vscode/blob/master/extensions/html-language-features/client/src/mirrorCursor.ts

There's nothing in LSP that correspond to adding a cursor. So if you want to implement this, the best you can do is to use VS Code's API like the html language client is currently doing.

Please note that MatchingTagPositionRequest is not an official name.

NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Dec 9, 2019
Under the preference xml.autoSelectMatchingTags

Fixes #redhat-developer/vscode-xml#130

Signed-off-by: Nikolas Komonen <[email protected]>
NikolasKomonen added a commit to NikolasKomonen/vscode-xml that referenced this issue Dec 9, 2019
Under the preference xml.autoSelectingMatchingTags

Fixes redhat-developer#130

Signed-off-by: Nikolas Komonen <[email protected]>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Dec 9, 2019
Under the preference xml.autoSelectMatchingTags

Fixes #redhat-developer/vscode-xml#130

Signed-off-by: Nikolas Komonen <[email protected]>
@fbricon fbricon added this to the 0.10.0 milestone Dec 10, 2019
NikolasKomonen added a commit to NikolasKomonen/vscode-xml that referenced this issue Dec 10, 2019
Under the preference xml.autoSelectingMatchingTags

Fixes redhat-developer#130

Signed-off-by: Nikolas Komonen <[email protected]>
NikolasKomonen added a commit to NikolasKomonen/vscode-xml that referenced this issue Dec 10, 2019
Under the preference xml.autoSelectingMatchingTags

Fixes redhat-developer#130

Signed-off-by: Nikolas Komonen <[email protected]>
NikolasKomonen added a commit to NikolasKomonen/vscode-xml that referenced this issue Dec 11, 2019
Under the preference xml.autoSelectingMatchingTags

Fixes redhat-developer#130

Signed-off-by: Nikolas Komonen <[email protected]>
NikolasKomonen added a commit to NikolasKomonen/vscode-xml that referenced this issue Dec 11, 2019
Under the preference xml.autoSelectingMatchingTags

Fixes redhat-developer#130

Signed-off-by: Nikolas Komonen <[email protected]>
NikolasKomonen added a commit to NikolasKomonen/vscode-xml that referenced this issue Dec 11, 2019
Under the preference xml.autoSelectingMatchingTags

Fixes redhat-developer#130

Signed-off-by: Nikolas Komonen <[email protected]>
NikolasKomonen added a commit to NikolasKomonen/vscode-xml that referenced this issue Dec 11, 2019
Under the preference xml.autoSelectingMatchingTags

Fixes redhat-developer#130

Signed-off-by: Nikolas Komonen <[email protected]>
@angelozerr
Copy link
Contributor

angelozerr commented Dec 12, 2019

Please note that MatchingTagPositionRequest is not an official name.

@octref you mean that this request type could belong to the LSP specification ?

@octref
Copy link

octref commented Dec 12, 2019

I was saying the opposite: it’s NOT lsp, just a custom request now. If enough people want it we can propose to add it, but it should look very different than the current implementation.

@angelozerr
Copy link
Contributor

I was saying the opposite: it’s NOT lsp,

Yes sure I understood that, but I love this feature and I hope really this request type will be a LSP specification. In this case we could have this feature in any LSP client (my wish is to have this support inside Eclipse IDE with LSP4E /cc @mickaelistria)

@mickaelistria
Copy link

the Language Server already has ability to send TextEdit to the client to apply on client-side. So this can be an option, when turned on, that sending a text change inside a tag sends an applyEdit notification for the other tag in the client.
I don't think we need some extra thing to make it work on client side.

fbricon pushed a commit that referenced this issue Dec 13, 2019
Under the preference xml.autoSelectingMatchingTags

Fixes #130

Signed-off-by: Nikolas Komonen <[email protected]>
fbricon pushed a commit to eclipse-lemminx/lemminx that referenced this issue Dec 13, 2019
Under the preference xml.autoSelectMatchingTags

Fixes #redhat-developer/vscode-xml#130

Signed-off-by: Nikolas Komonen <[email protected]>
@fbricon
Copy link
Collaborator

fbricon commented Dec 18, 2019

Feature doesn't work (see #211 )

@fbricon fbricon reopened this Dec 18, 2019
@fbricon fbricon removed this from the 0.10.0 milestone Dec 18, 2019
@xorye xorye mentioned this issue Dec 18, 2019
@octref
Copy link

octref commented Dec 20, 2019

I can't repro the problem in HTML though. Are you doing anything XML specific?

@xorye
Copy link

xorye commented Dec 20, 2019

One feature that was specific to vscode-xml was that we added the ability to toggle the xml.mirrorCursorOnMatchingTag feature with Ctrl/Cmd + shift + f2.

A new listener was added to detect changes to xml.mirrorCursorOnMatchingTag and called the onDidChangeTextEditorSelection() function in order to update the cursor immediately on toggle.

I'm guessing that we had problems because we didn't have

if (event.textEditor.document?.languageId !== 'xml' ) {
     return;
}

in our onDidChangeTextEditorSelection() function.

@bmix
Copy link
Author

bmix commented Mar 10, 2020

There is also some discussion going on here: VSCode: Improve mirror cursor implementation with Synced Regions #88424

@angelozerr
Copy link
Contributor

Thanks @bmix for this great information!

@angelozerr
Copy link
Contributor

@xorye we should manage synced region in LemMinx side and consume it. See HTML Language server commit microsoft/vscode@01e01b1 but I suggest you see for each files the master code (if there are some fixes)

@angelozerr
Copy link
Contributor

@angelozerr angelozerr added this to the 0.17.0 milestone Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completion enhancement New feature or request
Projects
None yet
6 participants