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.

Sign up for GitHub

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

Jump to bottom

Bind XSD, DTD with CodeLens #494

Merged
merged 1 commit into from
Jun 16, 2021
Merged

Bind XSD, DTD with CodeLens #494

merged 1 commit into from
Jun 16, 2021

Conversation

angelozerr
Copy link
Contributor

Bind XSD, DTD with CodeLens

Fixes #395

Signed-off-by: azerr azerr@redhat.com

@angelozerr
Copy link
Contributor Author

This PR requires the LemMinx PR eclipse-lemminx/lemminx#1049

@angelozerr
Copy link
Contributor Author

angelozerr commented Jun 4, 2021 •
edited
Loading

Here a little demo with XSD, DTD codelens binding

CodeLensBindingDemo

Please note I need to improve:

  • generate absolute path
  • generate xsi:schemaLocation in the case when XSD declares a namespace (I don't know if it's easy to do).

@AlexXuChen
Copy link
Contributor

Will this only bound the file from the absolute path on the local machine? (e.g. c:\Users\..) If the developer is making a xml front end in production, and forgets to change the path, they will likely run into problems. Is this feature intended just for local development?

@datho7561
Copy link
Contributor

I think Angelo mentioned that he was going to try to implement relative paths eventually

@angelozerr angelozerr force-pushed the associate-with-codelens branch from 4c8bb47 to ac40e94 Compare June 6, 2021 09:16
@angelozerr
Copy link
Contributor Author

angelozerr commented Jun 7, 2021 •
edited
Loading

I have improved the PR:

  • relative path should be now supported.
  • xsi:schemaLocation is generated now when XSD defines a targetNamespace.

@datho7561
Copy link
Contributor

Here is a more detailed explanation of the performance issue I am facing (under Linux):

When the open file dialog opens, my cursor is set to the index finger pointing up style cursor. When I attempt any mouse interaction with the open file dialog or the instance of VS Code that is debugging the extension, nothing happens. While this is happening, keyboard interaction with the dialog window works fine. I can get mouse interaction with the open file dialog to work again if I click on a text entry field in another window, then go back tot he open file dialog.

I will investigate if this happens with all extensions by coding a quick demo extension, and file upstream in VS Code/Electron if this is the case.

@angelozerr angelozerr force-pushed the associate-with-codelens branch 3 times, most recently from e4c2a32 to 6ae3c46 Compare June 10, 2021 15:09
@datho7561
Copy link
Contributor

I haven't gotten any further in to figuring out why the Open File dialog doesn't work as expected under Linux.

@angelozerr angelozerr force-pushed the associate-with-codelens branch 2 times, most recently from fcaa51f to cd13082 Compare June 11, 2021 09:43
@angelozerr angelozerr marked this pull request as ready for review June 11, 2021 11:01
@angelozerr
Copy link
Contributor Author

I haven't gotten any further in to figuring out why the Open File dialog doesn't work as expected under Linux.

Could you retry please.

@angelozerr angelozerr force-pushed the associate-with-codelens branch from cd13082 to a39ca88 Compare June 11, 2021 11:46
@datho7561 datho7561 self-requested a review June 11, 2021 12:55
@datho7561
Copy link
Contributor

I still have the same issue.

@datho7561
Copy link
Contributor

If I use the option "files.simpleDialog.enable": true, this PR works fine, and it gives a file picker similar to the Ctrl+P one. Maybe we can temporarily enable/disable this when the command is run? (It will be a bit hacky though).

@datho7561
Copy link
Contributor

This appears to be the same issue, but I can no longer reproduce the issue with the steps given in the issue: microsoft/vscode#62704

@angelozerr angelozerr force-pushed the associate-with-codelens branch from a39ca88 to 3188305 Compare June 11, 2021 17:13
@datho7561
Copy link
Contributor

See microsoft/vscode#126088

datho7561
datho7561 requested changes Jun 11, 2021
View reviewed changes
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. The issue I found with the PR is an upstream (VS Code or Electron) issue.

src/commands/commandConstants.ts Outdated Show resolved Hide resolved
src/commands/registerCommands.ts Outdated Show resolved Hide resolved
@angelozerr angelozerr force-pushed the associate-with-codelens branch from 3188305 to 5a933fc Compare June 12, 2021 07:10
@datho7561 datho7561 self-requested a review June 14, 2021 13:05
@angelozerr angelozerr force-pushed the associate-with-codelens branch 2 times, most recently from 48da324 to 3db1024 Compare June 15, 2021 08:36
datho7561
datho7561 requested changes Jun 15, 2021
View reviewed changes
src/commands/registerCommands.ts Outdated Show resolved Hide resolved
@angelozerr
Copy link
Contributor Author

@datho7561 since de open a combo with binding type encore opening file dialogues is it fixing tour issue with dialog freeze on Linux?

@angelozerr angelozerr force-pushed the associate-with-codelens branch from 3db1024 to 1181ad2 Compare June 16, 2021 15:19
@angelozerr
Bind XSD, DTD with CodeLens
1181ad2 
Fixes redhat-developer#395

Signed-off-by: azerr <azerr@redhat.com>
@datho7561 datho7561 merged commit cfe58d3 into redhat-developer:master Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datho7561 datho7561 datho7561 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bind XSD, DTD with CodeLens
3 participants
@angelozerr @AlexXuChen @datho7561