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

Support alternate config file/directory location #66

Open
cherylking opened this issue Jun 10, 2022 · 11 comments
Open

Support alternate config file/directory location #66

cherylking opened this issue Jun 10, 2022 · 11 comments
Assignees
Labels
enhancement New feature or request high priority LCLS Liberty Config Language Server LemMinX Liberty extension to Eclipse Lemminx

Comments

@cherylking
Copy link
Member

In the Maven/Gradle plugins, the default configDirectory can be overridden. The various config files (serverXmlFile, serverEnvFile, bootstrapPropertiesFile) can also be overridden. The liberty-plugin-cfg.xml file will contain the values used for those various config files/dir and can be used by our LS to determine if a file is indeed a config file that should have hover/completion/validation support.

This is not part of the MVP. This is included on slide 34 of the UFO.

@cherylking cherylking added the enhancement New feature or request label Jun 10, 2022
@cherylking cherylking added the LemMinX Liberty extension to Eclipse Lemminx label Sep 30, 2022
@cherylking
Copy link
Member Author

Although I said we could use the values from the liberty-plugin-cfg.xml file, that file will not exist before the Liberty server has been created. The user would get inconsistent behavior based on whether the target/build directory existed for the project.

To address that we would have to potentially parse the build file (pom.xml / build.gradle) to determine the plugin configuration. So far the language server has not needed to parse the build file.

@scottkurz
Copy link
Member

scottkurz commented Jan 24, 2023

I wonder too about the idea of querying an opened XML file for a <server> root element.

This does present the possibility that a user is getting assistance for an XML file that the LMP/LGP is not configured to use... i.e. a dead/stale file rather than a "live" one.

Perhaps once the plugin is run a warning could be generated for a case like this... even though the warning wouldn't appear initially?

Another downside though is performance... though parsing with a streaming API like STAX should be quick it still seems like there could be a case where it scales badly.

@cherylking
Copy link
Member Author

I have assigned David to lead this, with support from Evie. Some ideas we discussed today for starting on this:

  1. First implement using the liberty-plugin-cfg.xml to identify and support various additional config files.
  2. Look into ways to generate the liberty-plugin-cfg.xml without having to run a full build/install, or just some temp file in the .libertyls folder that would identify which source files are Liberty config files used in the build file (pom.xml or build.gradle).
  3. Parsing the build files in LCLS is absolute last resort.

@cherylking
Copy link
Member Author

We discussed this a little bit at today's design issues call. For idea 2 above, if we added a new goal/task to the plugins that simply processed the build file to generate the liberty-plugin-cfg.xml file with the various locations of the Liberty config files, then we would need a way to invoke that new goal/task. I proposed that the new goal/task could be invoked at the time that Liberty Tools is determining which projects to add to the Liberty Dashboard. That should also include any manual path for adding projects to the Liberty Dashboard.

@scottkurz
Copy link
Member

Seems like an alternative to a new goal/task would be to use an existing one like install-server or create and if there are concerns about collision with normal install, working through those, e.g. using a dummy location. Not to say a new goal/task might not be better...but just thinking that through.

@cherylking
Copy link
Member Author

Seems like an alternative to a new goal/task would be to use an existing one like install-server or create and if there are concerns about collision with normal install, working through those, e.g. using a dummy location. Not to say a new goal/task might not be better...but just thinking that through.

I wouldn't want to actually install Liberty or create a server at that time, as that could affect the behavior of running dev mode later. (I'm thinking of the case of skip install feature, where it detects if this is the first install or not). Also, I would be worried about performance. This is basically a setup step that we need to perform and would want it to return quickly.

@dshimo
Copy link
Contributor

dshimo commented Jul 27, 2023

I wonder too about the idea of querying an opened XML file for a root element.

@scottkurz
I think this idea has some promise in its simplicity. How about an idea to support all xml files while using that heuristic?

In terms of design, catching our desired files with a good heuristic would be more straightforward than managing a growing list of files to support. Then in terms of consequences, I think potentially missing a file and denying a user features where they expect it may be worse than enabling Liberty support for a file a user didn't expect. It would be worthwhile to determine how the plugin could break user experience though.

@evie-lau
Copy link
Member

evie-lau commented Aug 11, 2023

After discussing with David, supporting custom serverEnv/bootstrapProperties is a bit tricky. LCLS itself can't monitor the liberty-plugin-cfg.xml; the client side (IDE plugin) has to do that.

Based on LSP protocol, this has to be the flow:

  • We change the LT IDE Plugins to monitor liberty-plugin-cfg.xml (currently only monitors server.env/bootstrap.properties files)
  • LT IDE Plugin activates LCLS to parse custom server/bootstrap files from liberty-plugin-cfg.xml, and stores the list of files on the LS side
  • We have to change all the LT IDE Plugins to monitor ALL files in the workspace (or maybe just *.env, *.properties), to send those to LCLS (which will filter based on the previously generated list) to provide LS features

The last part, I'm not sure how much of a performance impact it would have.

@scottkurz
Copy link
Member

2023-08-23 DXDI update - We like the idea of relying on the liberty-plugin-cfg.xml to store alt. locations. We are OK with the fact that this implies only the default locations will get code assist initially (before a build). This leaves the door open too still for a future lightweight goal/task the plugins could call to only generate plugin cfg.

@dshimo
Copy link
Contributor

dshimo commented Sep 7, 2023

With #212, LCLS has the logic for scanning liberty-plugin-config.xml on init and workspace didChangeWatchedFiles, which includes creation, edits, and deletion. The behavior for pruning the list on deletion has not be added yet #213.

There is remaining work for the IDE clients to change their watch files. Here is a PR example for these changes in VS Code, but each IDE may be different.

@evie-lau
Copy link
Member

evie-lau commented Oct 17, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request high priority LCLS Liberty Config Language Server LemMinX Liberty extension to Eclipse Lemminx
Projects
Status: Target feature release
Development

No branches or pull requests

4 participants