-
Notifications
You must be signed in to change notification settings - Fork 220
Open stereotype definition JSON #1663
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
Conversation
|
|
||
| context.subscriptions.push(commands.registerCommand("vscode-spring-boot.structure.grouping", async (node: StereotypedNode) => { | ||
| const projectName = node.getProjectId(); | ||
| const projectName = node.getNodeId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I introduced this change to use the project ID instead of the node ID here:
2702658
I think we need to use the project ID, since the node ID could be something that is a very special label. For example for restbucks (when using Modulith), the label for the project looks something like restbucks (de.odrotbohm.restbucks) - which is not the exact project element name. Using the node ID (which includes the project label) fails here, since the project in the language server is called restbucks. Therefore I introduced (for project nodes) a dedicated attribute in the JSON to have the exact project name at hand and use that when identifying projects between the UI and the language server.
| // Reference is a specific URL that should be passed to java.open.file command | ||
| commands.executeCommand('java.open.file', reference); | ||
| const uri = Uri.parse(reference); | ||
| if (uri.scheme === 'jdt') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not distinguish between those different URLs here and assume instead that we just use vscode.open to open the URI - and that's it. The language server can then produce the newly introduced spring-boot-ls URLs for all the references to JSON catalog files and we are done.
| this.project = project; | ||
| } | ||
|
|
||
| public static Optional<String> getDefaultStereotypePath(URL url) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this method is necessary (see the comment for the mechanism above)
| if (params.getArguments().size() == 1) { | ||
| Object o = params.getArguments().get(0); | ||
| String r = o instanceof JsonElement ? ((JsonElement) o).getAsString() : o.toString(); | ||
| return IOUtils.toString(getClass().getResourceAsStream(r)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could change this to deal with random URLs instead of just a path that we then load from the classpath via the classloader (this getResourceAsStream). I think we could use Apache Commons IOs IOUtils here to fetch the content from a random URL. The URL then could be a standard jar:file:... URL or whatever. We just would need to make sure to pass the complete URL to the command as an arg, not just the file path somehow.
| // something went wrong | ||
| } | ||
| } | ||
| reference = ProjectBasedCatalogSource.getDefaultStereotypePath(url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would vote for turning all the URLs into a spring-boot-ls://resource URL and then use that URL when reading the content of the resource (see below where we handle the command). I think we don't need to distinguish between URLs that are based on the default catalog files from inside of the language server and URLs that point to "random" JARs from the classpath - and therefore don't convert the JAR URLs into something JDT specific. We could just pass the URL to the client, then transport the URL back to the fetch content command, and read all the content from the URL - whether it is a file URL or a JAR URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the end, this is what the JMolecules catalog is doing as well when we pass in all the URLs to all the catalog files for a project. It just reads the content from all those URLs.
Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>
Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>
Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>
Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>
ac29324 to
31b5102
Compare
|
@martinlippert this works well now. Instead of the straight JAR url it is a spring-boot-ls url for now. |
Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>
martinlippert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good to me. Will squash and merge.
Fixes #1642