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

Mirror cursors #213

Closed
wants to merge 1 commit into from
Closed

Mirror cursors #213

wants to merge 1 commit into from

Conversation

xorye
Copy link

@xorye xorye commented Dec 18, 2019

Here is some work for #130

Everything that works for the html mirror tags, should work with this PR.

Toggle is available with ctrl+shift+f2.

How it works:
There is a listener that listens for changes in the xml.mirrorCursorOnMatchingTag config. This listener is active as long as vscode-xml is activated.

If xml.mirrorCursorOnMatchingTag is turned on, a MirrorCursors object is created. This object creates more listeners.
When xml.mirrorCursorOnMatchingTag is turned off, the MirrorCursors object (if exists) is disposed, along with the listeners it created.

I must clean the code and play around with it more.
Please feel free to test things out as it is.

@angelozerr
Copy link
Contributor

angelozerr commented Dec 19, 2019

@xorye IMHO (but not sure if it's possible?), I think we should keep the original mirrorCursor.ts from vscode and update it by adding comments (// updated for vscode-xml) because:

  • it will be more easy to review your PR (just comparing original vscode mirrorCursor.ts with your mirrorCursor.ts)
  • when vscode will fix something in mirrorCursor.ts, it will be easy to fix it in vscode-xml if we have the same code.

@fbricon what do you think about that?

src/mirrorCursor.ts Outdated Show resolved Hide resolved
src/mirrorCursor.ts Outdated Show resolved Hide resolved
src/mirrorCursor.ts Outdated Show resolved Hide resolved
src/mirrorCursor.ts Outdated Show resolved Hide resolved
@fbricon
Copy link
Collaborator

fbricon commented Jan 28, 2020

new version seems way safer

@xorye
Copy link
Author

xorye commented Jan 28, 2020

fixed

@xorye xorye marked this pull request as ready for review January 28, 2020 15:52
@fbricon
Copy link
Collaborator

fbricon commented Jan 28, 2020

We still have that completion issue, that the html server doesn't have:
Jan-28-2020 17-11-24

@xorye
Copy link
Author

xorye commented Jan 29, 2020

The reason why it works for the HTML language server is that for their completion requests, the new text that is being inserted is only the tag name (ie, div).

LSP4XML provides the brackets, start tag and end tag on completion, (ie <div></div>). One solution would be to have LSP4XML do the same thing as the HTML language server and just send back the tag names, but maybe that's not feasible in the context of LSP4XML?

@angelozerr
Copy link
Contributor

As @xorye LSP4XML generate the start and end tag when completion is applied (like Eclipse WTP) although HTML Language server generates only the start tag.

An idea of fix is to check if mirror cursor is activated and send this settings to the LSP4XML. In this case, the end tag should not be generated and we will have the same context than HTML language server.

@fbricon
Copy link
Collaborator

fbricon commented Jan 30, 2020

I think you can check whether the tag is closed or not, whether mirror cursors is enabled or not

@angelozerr
Copy link
Contributor

I think you can check whether the tag is closed or not

Ideally we should do that, but I don't know if it's possible to do that, need more investigation.

@fbricon
Copy link
Collaborator

fbricon commented Feb 17, 2020

See related microsoft/vscode#88424

@xorye
Copy link
Author

xorye commented Aug 6, 2020

Closing in favour of #304, since HTML doesn't use the old mirror cursors implementation that this PR was based on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants