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

Support custom config files #212

Merged
merged 25 commits into from
Sep 6, 2023
Merged

Support custom config files #212

merged 25 commits into from
Sep 6, 2023

Conversation

evie-lau
Copy link
Member

@evie-lau evie-lau commented Sep 1, 2023 •
edited
Loading

Resolves #66

NOTICE: Will require IDE plugins to monitor liberty-plugin-config.xml to send to LCLS, alongside all .properties and .env files

Changes:

LibertyConfigFileManager.java

  • Add support for custom config files from liberty-plugin-config.xml
  • logic to screen env and properties file here, instead of on IDE client-side
  • Add method to normalize paths across OSes

LibertyWorkspaceService.java

  • process liberty-plugin-config.xml when it changes

Various files:

  • use the new LibertyConfigFileManager
  • LibertyLanguageServer now initializes the LibertyConfigFileManager

dshimo and others added 16 commits September 1, 2023 15:14
@dshimo @evie-lau
change config file logic (OpenLiberty#194)
b7ff73c 
* config logic swap
@cherylking @evie-lau
Fix determination of Liberty Workspace for non-default server.xml (Op…
e025c59 
…enLiberty#201)

* Fix determination of Liberty Workspace for non-default server.xml

* Clean up
@dshimo @evie-lau
xmlreader on config portion
6acab22
@evie-lau
Initial changes to watch and process liberty-plugin-config.xml
a7b4a60
@evie-lau
Process and store custom config files
f59e641
@dshimo @evie-lau
init test, refactor to use LCFM
3df2de1
@evie-lau
Initial tests. Small refactor
ad35bf3
@evie-lau
Fix uri string mismatch. Update key completion to use file-checking m…
1c303fe 
…ethods.
@evie-lau
Update comments and todos
146a267
@evie-lau
Add check for config file path, moved from client-side filewatch filt…
41c4c30 
…ering
@evie-lau
Fix paths for windows
93ff57a
@dshimo @evie-lau
init custom config files list
6807f9c
@evie-lau
Normalize paths. Test still fails on Windows because test file has un…
6c55625 
…ix path
@evie-lau
Run custom config file test in Unix only
763c653
@evie-lau
Modify tests to account for Windows
5d4de2b
@evie-lau
Cleanup comments and add copyright
8bfd08e
@evie-lau evie-lau marked this pull request as draft September 1, 2023 21:37
cherylking
cherylking requested changes Sep 5, 2023
View reviewed changes
Copy link
Member

@cherylking cherylking left a comment

Choose a reason for hiding this comment

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

I refrained from making comments on the tests for the most part. I will review those once you have finished your updates.

liberty-ls/src/main/java/io/openliberty/tools/langserver/LibertyConfigFileManager.java Outdated Show resolved Hide resolved
liberty-ls/src/main/java/io/openliberty/tools/langserver/LibertyLanguageServer.java Outdated Show resolved Hide resolved
liberty-ls/src/main/java/io/openliberty/tools/langserver/LibertyLanguageServer.java Show resolved Hide resolved
liberty-ls/src/main/java/io/openliberty/tools/langserver/LibertyWorkspaceService.java Show resolved Hide resolved
...main/java/io/openliberty/tools/langserver/diagnostic/LibertyPropertiesDiagnosticService.java Show resolved Hide resolved
liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/ParserFileHelperUtil.java Show resolved Hide resolved
liberty-ls/src/main/java/io/openliberty/tools/langserver/LibertyConfigFileManager.java Show resolved Hide resolved
liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/XmlReader.java Outdated Show resolved Hide resolved
liberty-ls/src/test/java/io/openliberty/tools/langserver/xml/LibertyConfigFileManagerTest.java Outdated Show resolved Hide resolved
liberty-ls/src/test/resources/xml/unix/liberty-plugin-config.xml Outdated Show resolved Hide resolved
dshimo and others added 4 commits September 5, 2023 12:14
@dshimo
oops
0ac0973
@evie-lau
Minor fixes
07d6414
@evie-lau @dshimo
Fix Windows pathing
5e8b0aa 
Co-authored-by: David Shi <dshi@utexas.edu>
@evie-lau @dshimo
Fix parse on file change.
95cc901 
Co-authored-by: David Shi <dshi@utexas.edu>
@evie-lau evie-lau marked this pull request as ready for review September 5, 2023 22:59
@cherylking
Copy link
Member

@dshimo @evie-lau There is still a build failure on Windows.

@evie-lau
Make sure build/test actions are separate. Suppress progress tracking…
1043bbe 
… for downloads

Make sure Windows paths are converted to use forward slash

Test different approach for unix/windows systems

Use URI.getPath for windows Paths.get

Try removing initial slash when checking windows path

Fix default file check with File.separator

Fix windows test string
This was referenced Sep 6, 2023
Closed
Closed
@evie-lau evie-lau merged commit a1b25f2 into OpenLiberty:4q2023 Sep 6, 2023
4 checks passed
@evie-lau evie-lau mentioned this pull request Sep 6, 2023
Merged
@dshimo dshimo mentioned this pull request Sep 7, 2023
Open
dshimo added a commit that referenced this pull request Oct 10, 2023
@evie-lau @dshimo
Support custom config files (#212)
c4640e9 
* Process liberty-plugin-config.xml to parse and store list of custom config files, upon startup and file changes to the XML

* Fix uri string mismatch across all methods and classes.

* Add check for config file path, moved from IDE client-side filewatch filtering

* Make sure Github build/test actions are separate. Suppress progress tracking for downloads

Co-authored-by: David Shi <dshi@utexas.edu>
@evie-lau evie-lau deleted the customConfig branch October 19, 2023 15:36
@scottkurz scottkurz mentioned this pull request Oct 23, 2023
Closed
@awisniew90 awisniew90 mentioned this pull request Mar 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dshimo dshimo dshimo left review comments

@cherylking cherylking cherylking 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.
3 participants
@evie-lau @cherylking @dshimo