#582: Use new XML merger with intellij configuration files #68
Conversation
Moved the intellij config files from setup to update
|
I am still not sure when exactly the OVERRIDE strategy is required |
|
Still needs some adjustments |
|
Should now be ready to be merged. However some files are using override as merge strategy. This works for now but is not ideal. As a possible workaround i replaced the merge:id"name()" with xpathexpressions wich seemed to be working. So when #498 is resolved and the override merge strategies get removed i would recommend to try and use xpathexpressions |
There was a problem hiding this comment.
@KianRolf thanks for this PR. Quite tricky to add all this merge attributes and namespaces.
Thanks also for testing this with IDEasy and finding the problems in the XML merger. 👍
I finally found the time to review - sorry for the long delay.
There are a lot of review comments I lefts.
Some of them are because of problems we already have before this PR so they are not your "fault" but I just noticed when I closely looked into them during the review.
Some problems would have been prevented by proper git mv usage so that should be a good learning for the future since git is fundamental for software development.
Please have a look and try to address the review comments.
| @@ -0,0 +1 @@ | |||
| $[PROJECT_NAME] - $[WORKSPACE] | |||
There was a problem hiding this comment.
You duplicated this file instead of moving it.
Either leave it in setup or move it to setup.
| @@ -0,0 +1,9 @@ | |||
| <?xml version="1.0" encoding="UTF-8" standalone="no"?> | |||
There was a problem hiding this comment.
Also this file gets duplicated instead of being moved.
IMHO we can also delete this file. All obsolete with .editorconfig.
See also PR #64
| @@ -0,0 +1,6 @@ | |||
| <?xml version="1.0" encoding="UTF-8" standalone="no"?> | |||
There was a problem hiding this comment.
Also this file gets duplicated instead of being moved.
IMHO we can also delete this file. All obsolete with .editorconfig.
See also PR #64
| @@ -0,0 +1,4 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
Also this file gets duplicated instead of being moved.
It is still needed so you can delete the one from setup in your feature branch.
| @@ -0,0 +1,8 @@ | |||
| <?xml version="1.0" encoding="UTF-8" standalone="no"?> | |||
There was a problem hiding this comment.
Also this file gets duplicated instead of being moved.
Currently it is unclear to me what this file is needed for.
@jan-vcapgemini Can we remove it?
IMHO this is also obsolete.
| <?xml version="1.0" encoding="UTF-8" standalone="no"?> | ||
| <application xmlns:merge="https://github.com/devonfw/IDEasy/merge" merge:strategy="override"> | ||
| <component name="ProjectJdkTable" merge:id="@name"> | ||
| <jdk version="2" merge:id="@version"> |
There was a problem hiding this comment.
This one will get really tricky since the design of this XML from IntelliJ is very much flawed.
If you have multiple JDKs configured, you will have multiple elements:
<jdk version="2>
<name value="MyJava1">
...
</jdk>
<jdk version="2>
<name value="MyJava2">
...
</jdk>So most probably merge:id="./name@value" would be correct for jdk.
I have never tested if our XML merger could handle this.
For this PR, we can ignore this comment but we might need to create a new IDEasy ticket out of this.
intellij/workspace/update/.intellij/config/options/path.macros.xml
Outdated
Show resolved
Hide resolved
intellij/workspace/update/.intellij/config/options/trusted-paths.xml
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,6 @@ | |||
| <?xml version="1.0" encoding="UTF-8" standalone="no"?> | |||
There was a problem hiding this comment.
where does this file come from?
Did you introduce it?
IMHO it should be removed here.
Co-authored-by: Jörg Hohwiller <[email protected]>
….xml Co-authored-by: Jörg Hohwiller <[email protected]>
…hs.xml Co-authored-by: Jörg Hohwiller <[email protected]>
…ps://github.com/KianRolf/ide-settings into adjust/Adjust-configuration-files-to-XML-merger
fixes devonfw/IDEasy#582
Moves intellij config files from setup to update and configures them for the new xml merger