devonfw / IDEasy

Tool to automate the setup and updates of a development environment for any project (Successor of devonfw-ide).
Apache License 2.0
8 stars 23 forks source link

Merge XML files into workspace #43

Closed alfeilex closed 4 months ago

alfeilex commented 1 year ago

Copied from: https://github.com/devonfw/ide/issues/1277


As a devonfw-ide user, I want to manage XML configuration files for my workspace so that specific sections get merged into my current IDE configration.

I noticed that even though I have changed an XML configuration file in settings/intellij/workspace/update/.intellij/config/options/ and reran devon intellij the changed settings have not been merged into my workspace (workspaces/main/.intellij/config/options/). I had to delete the XML file in my workspace and restart intellij again via devon to make it work.

In general merging XML is very complex. However, our XML merger is implemented rather naive:

https://github.com/devonfw/ide/blob/10d3109754079a65573bdadb4782bf500983a0e3/configurator/src/main/java/com/devonfw/tools/ide/configurator/merge/XmlMerger.java#L28

In this story, we should first design a solution how to solve XML merging in general. For this we can get some inspirations and leanings from CobiGen. However, IMHO the CobiGen approach is too complex and we would have to maintain every new specific tag and attribute that IDEs could introduce in the future. Therefore, I rather think about a simple approach where we could somehow "annotate" the template XML file such that it will know which parts of the XML are actual "identifiers" and which are "values".

To outline the general problem lets have a look at the following example: XML Template from settings:

 <root>
   <list>
     <item>
       <id>key1</id>
       <value>value1</value>
     </item>
   </list>
   <plugins>
     <plugin>FooPlugin</plugin>
   </plugins>
 </root>

XML config in workspace:

 <root>
   <list>
     <item>
       <id>key1</id>
       <value>value2</value>
       <optional>Elementsomething</optionalElement>
     </item>
    </list>
   <plugins>
     <plugin>BarPlugin</plugin>
   </plugins>
 </root>

We might expect the merged result to be this:

 <root>
   <list>
     <item>
       <id>key1</id>
       <value>value1</value>
       <optional>Elementsomething</optionalElement>
     </item>
    </list>
   <plugins>
     <plugin>BarPlugin</plugin>
     <plugin>FooPlugin</plugin>
   </plugins>
 </root>

But how would our XmlMerger know that he has to merge existing item element with the same id being key1 and change its value from value2 to value1 while the plugin element will be added. We might have some heuristic saying that id is a special tag-name but looking at real examples from the mess that Intellij has created in its workspace config this will not work out. On the other hand, I do not want to go the CobiGen way and hard-code all the tag names and logic for Eclipse, Intellij, etc. into our XmlMerger. So my idea would be something like this for the template:

 <root xmlns:ide="https://github.com/devonfw/ide">
   <list>
     <item ide:kind="merge">
       <id ide:kind="id">key1</id>
       <value>value1</value>
     </item>
   </list>
   <plugins>
     <plugin ide:kind="merge-unique">FooPlugin</plugin>
   </plugins>
 </root>

Our XmlMerger would understand the semantic of the ide namespace and remove that namespace during the merging process. However, this way, it can understand that it will merge elements matching to /root/list/item and if present in the existing XML file to merge into that one that has an id element being identical to the one from the template (excluding the ide namespace) and otherwise to append the entire item. The option merge-unique means that an element is both the id and the value. So far this is just a draft that needs some more rethinking. Maybe we also need to configure things like ide:add="end" vs. ide:add="start" and maybe even ide:add="ascending" but then end should be the default, that can be omitted.

maybeec commented 1 year ago

You should have a look at https://github.com/maybeec/lexeme. With the XML merger you can specify rules the merger should follow.

hohwille commented 1 year ago

@maybeec thanks for your hint. We will definitely consider lexeme. However, my fear is that it is too complex for our users to configure: https://github.com/maybeec/lexeme/blob/master/src/test/resources/systemtests/schemas/beanmapping.xsd https://github.com/maybeec/lexeme/blob/master/src/test/resources/systemtests/mergeschemas/BeansMergeSchema.xml

And we can not pre-configure everything as we do not know what JetBrains, Eclipse, Microsoft, or millions of plugin vendors are doing or will do tomorrow.

For our use-case having the "mergeschema" inside the XML template itself and having not requirement for a schema at all would be what we actually need.

I am aware that for most of our average users it will be too complex independent if the merge-metadata is in an external file or inside the template but our users are able to do copy&paste of working solutions and therefore it makes a big difference.

maybeec commented 1 year ago

There is a default algorithm merging XML, while you can add-on the configuration with specific merge schemata for specific namespaces. Thus, it's your choice whether you want to have a very specific merge result or simply "trying to merge properly". From the issue description, I thought you might have use cases at least for eclipse and intellij, where you really want to provide better merge support, which could be pre-configured as part of the EasyIDE. It's not meant to be configurable by end-users I feel

hohwille commented 1 year ago

We sill somehow use lexeme but maybe we will not use it as library but rather fork our own variant of it :)

hohwille commented 8 months ago

To shed some more light on the problem space and the requirements of this story:

Let us assume we have this XML template (without any merge config):

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23">
    <profile kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

The actual XML in the workspace may currently be:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="replace"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

Now how should the resulting XML after the merge look like? Option 1:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

Option 2:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23">
    <profile kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

Option 3:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23">
    <profile kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

Option 4:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

Option 5:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="replace"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

There may be more options but it should already get obvious that this is a very complex problem. The question now is how can you "configure" what result you would love to have as a user without IDEeasy hardcoding a specific algorithm that would only support one of these options. Requirements on this:

My idea was to have the configuration inside the XML template itself via a custom IDEasy namespace. The configuration could be "inherited" from the previous same element so in the example only the first setting element would need to be annotated. If following setting elements on the same XPath (profiles/profile/setting) do not have dedicated IDEasy merge attributes, those get "inherited" from the previous one. Otherwise the configuration is overridden and also used for the next elements on the same XPath. The approach of LEXME is to have the configuration in a separate config file and to map XML catalaogs to such configs. Even in case that we might not want to do this, we should definitely have a look at LEXME and see how the configuration is designed and also on the code so we can reuse knowledge instead of starting from scratch and running in pitfalls and problems that LEXME has already solved.

salimbouch commented 8 months ago

I'm thinking the following:

we use additional IDEasy namespace attributes for identification and conflict handling, for example if the id of the element differs from "id", then we can specify with ide:identifier="some-attribute-or-more" which attribute(s) should be used as id. For conflict handling we use ide:conflict-strategy="workspace" or ide:conflict-strategy="template" and default being template when omitted.

Example:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles xmlns:ide="https://github.com/devonfw/ideasy" version="23">
    <profile ide:kind="merge" ide:identifier="name" ide:conflict-strategy="workspace" kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

here, all settings inside profile will have the conflict strategy "workspace", if a setting then changes it's strategy, the rest element on the same XPath will also change the strategy.

Another idea is to introduce ide:kind="replace", here the element in workspace will be replaced entirely and no subcontent will be treated.

I think this should be an easy way to configure the XML Template without too much effort, what do you think @hohwille ?

hohwille commented 8 months ago

For conflict handling we use ide:conflict-strategy="workspace" or ide:conflict-strategy="template" and default being template when omitted.

What do you mean with "conflict handling"? Is this rather the merge strategy?

Another idea is to introduce ide:kind="replace", here the element in workspace will be replaced entirely and no subcontent will be treated.

Sounds like the same thing merge-strategy="override" and kind="replace". What exactly should be the meaning of kind vs. conflict-strategy? Do we need both?

salimbouch commented 8 months ago

What do you mean with "conflict handling"?

Conflict handling means when merging, and the element is present in both the template and the workspace, which values to prefer, see here.

So basically if the template looks like this:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles xmlns:ide="https://github.com/devonfw/ideasy" version="23">
    <profile ide:kind="merge" ide:identifier="name" ide:conflict-strategy="template" kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

and the XML File in workspace looks like this:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="replace"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

then the result would be:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

And if the conflict strategy is set to workspace, then the result would be:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="replace"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

Sounds like the same thing merge-strategy="override" and kind="replace".

kind="replace" would mean exactly what you meant by merge-strategy="override", the element will be entirely overridden.

What exactly should be the meaning of kind vs. conflict-strategy? Do we need both?

We don't really need both, when there is an attribute conflict-strategy, then it implies that the element has to be merged and saying kind="merge" is unnecessary, however I thought of a scenario where we could use both:

Template (no config):

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23">
    <profile kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

File in Workspace:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="replace"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

If our goal is to have this as a result, my idea was to use kind="replace" and conflict-strategy="workspace"

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>
hohwille commented 8 months ago

It seems we agree and now just discuss about naming.

I would suggest the following. Let us use merge instead of ide for our namespace prefix and use strategy as local attribute name:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles xmlns:merge="https://github.com/devonfw/IDEasy/merge" version="23">
    <profile merge:strategy="template" merge:id="@name" kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

I also simplified identifier to id and I changed the value to be an XPath (@name in XPath specifies the attribute name in the current node. For the future XPath also allows us to specify text() or even ../element-name/@attribute-name - however I hope we never come to the point where we need such complex XPath expressions but what I see is that we might need to combine/append multiple XPath expressions like @name+@type - I have not checked if + is also used in XPath and we need to find a different separator for appending XPaths).

hohwille commented 8 months ago

Still we need to specify what values we can have for merge:strategy and what they should mean. I see the value that I called override that would simply ignore the corresponding node in the XML to merge into without any further considerations of child-elements. That is an easy case to specify and implement. Is that what you wanted to express with template?

But how to we express if the child elements should not be overridden from the template but the merge algorithm should recursively apply? In such case IMHO the "default" and most natural behavior would be that for the matching element all attributes from the template (excluding those from the merge namespace) will be overridden but potential other attributes are kept as is. We also need to specify that we do not want any attributes from the XML to merge into and just take the attributes from the template. Then there is also the option to only add attributes from the template that are currently not in the XML to merge into. Do we want to make this configurable per attribute? Sounds over-engineered...

And should the decision to handle the child-elements be independent (in a separate attribute) to the decision how to merge the attributes of the element itself? Sounds like better to understand and more flexible to configure as otherwise you would have to build combinations into a single value...

Are you also aware that we are merging 3 files and not only 2? We potentially have two templates (setup and update) and one workspace file to merge into. If we solve the problem properly this should not be much more complicated. To simplify we could even only support a single template in update but then via merge attributes we must be able to define what to do on setup and what on update...

salimbouch commented 8 months ago

Is that what you wanted to express with template?

No with template I meant when recursively merging, and an element is both present in template and workspace, that we keep the values of the template, and workspace would mean we keep the values of the workspace element.

In such case IMHO the "default" and most natural behavior would be that for the matching element all attributes from the template (excluding those from the merge namespace) will be overridden but potential other attributes are kept as is.

why should the template have less priority than workspace? shouldn't it be the other way around?

Are you also aware that we are merging 3 files and not only 2?

Yes, but actually setup is only used when there is no file in workspace, can't we just copy setup into workspace when there is no workspace file and then merge update and workspace? This way we only have to merge 2 files and not 3, and we only use the merge namespace in update.

Let's make a list of things we found so far:

namespace:

xmlns:merge="https://github.com/devonfw/IDEasy/merge"

namespace attributes:

value: Xpath to the attribute(s) to be used as ID.

values:

override: ignores the corresponding node in the XML to merge into without any further considerations of childelements. recursive: (maybe we can call this something else) recursively merge child elements, if child element in not present in both documents then the element is appended, otherwise the matching elements will be merged (exact behavior depends on merge:attributes).

values:

default: (when this attribute is omitted): for matching elements attributes only present in workspace are kept as is, attributes only present in template are added, and those who are present in both documents are merged (exact behavior depends on merge:conflict). override: overrides all attributes of the element in workspace by the attributes of the template. add: only adds attributes present in template but not in workspace.

values:

template: (is the default when omitted), prefer values (attribute values and text) of the template file. workspace: prefer values of the workspace file.

hohwille commented 7 months ago

why should the template have less priority than workspace? shouldn't it be the other way around?

You are right and I was not clear with my words. I meant to say:

the matching element all attributes will be overridden from the template (excluding those from the merge namespace) but potential other attributes are kept as is.

Yes, but actually setup is only used when there is no file in workspace, can't we just copy setup into workspace when there is no workspace file and then merge update and workspace? This way we only have to merge 2 files and not 3, and we only use the merge namespace in update.

OK, you are right. The technical difference for setup files is that they are templates and variables get resolved while files from the workspace are taken as is. That behavior has to stay but you are right that we only merge 2 XML files so my point is actually invalid here.

As this topic is rather complex it is not good to continue via asynchronous chats in this issue. Let us plan a workshop after Easter with both of us are required participants and the rest of the developers from the team as optional participants.

hohwille commented 7 months ago

Let's make a list of things we found so far: ...

Agreed so far but if we separate between the attributes and the children...

In a workshop we can discuss this and collect all potential options and values - then discuss if we need to implement and support all of them or only start with a subset. If we have an agreement and want to formalize this, we could do TDD and create lots of testdata first with template + file to merge + expected result XML as triplets for arbitrary scenarios. This will also be very valuable to test the implementation later whereas documentation or these chat comments may be outdated faster than we can imagine.

salimbouch commented 6 months ago

namespace:

value: Xpath to the attribute(s) to be used as ID.

salimbouch commented 6 months ago

merge:self="combine" Template:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23" merge:self="combine">
    <profile kind="CodeFormatterProfile" name="IDE" version="23" merge:id="@kind">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert" merge:id="@id"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

workspace:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24" attr="example">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="replace"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

this would result in:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23">
    <profile kind="CodeFormatterProfile" name="IDE" version="23" attr="example">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

merge:self="override" Template:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23" merge:self="override">
    <profile kind="CodeFormatterProfile" name="IDE" version="23" merge:id="@kind">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert" merge:id="@id"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

Workspace:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24" attr="example">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="replace"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

result:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23">
    <profile kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

merge:self="keep" Template:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23" merge:self="keep">
    <profile kind="CodeFormatterProfile" name="IDE" version="23" merge:id="@kind">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert" merge:id="@id"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

workspace:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24" attr="example">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="replace"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

this would result in:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="24">
    <profile kind="CodeFormatterProfile" name="IDE" version="24" attr="example">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="replace"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>
hohwille commented 6 months ago
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23" merge:self="combine">
    <profile kind="CodeFormatterProfile" name="IDE" version="23" merge:id="@kind">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert" merge:id="@id"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

ah, new interesting case:

And for the expected result:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23">
    <profile kind="CodeFormatterProfile" name="IDE" version="23" attr="example">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
    </profile>
</profiles>

Why would we choose version 23 over 24 here and what could a user configure to have the opposite version result? IMHO we would need to be able to define priorities between template and workspace. I am not yet sure whether it would be better to distinguish 2 forms of combine for this or if we want to have yet another merge:priority or merge:precedence attribute for this.

hohwille commented 6 months ago

For the second expected result, I would disagree:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<profiles version="23">
    <profile kind="CodeFormatterProfile" name="IDE" version="23">
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_ellipsis" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_enum_declarations" value="insert"/>
        <setting id="org.eclipse.jdt.core.formatter.align_type_members_on_columns" value="false"/>
    </profile>
</profiles>

IMHO override would override the entire XML block and therefore align_type_members_on_columns line should not be expected here. Same for keep with insert_space_after_comma_in_enum_declarations. But apart from that this 2 cases seem to be clear and easy to me.

salimbouch commented 5 months ago

In intellij's codestyle's xml file we have the following behavior:

<code_scheme name="Default" version="173">
  <JavaCodeStyleSettings>
    <option name="SPACE_AFTER_CLOSING_ANGLE_BRACKET_IN_TYPE_ARGUMENT" value="true" />
    <option name="ALIGN_MULTILINE_RECORDS" value="false" />
    <option name="ALIGN_TYPES_IN_MULTI_CATCH" value="false" />
    <option name="JD_ALIGN_PARAM_COMMENTS" value="false" />
    <option name="JD_ALIGN_EXCEPTION_COMMENTS" value="false" />
     <!-- other options !-->
  </JavaCodeStyleSettings>
  <codeStyleSettings language="Groovy">
    <indentOptions>
      <option name="INDENT_SIZE" value="2" />
    </indentOptions>
  </codeStyleSettings>
  <codeStyleSettings language="HTML">
    <indentOptions>
      <option name="INDENT_SIZE" value="2" />
    </indentOptions>
  </codeStyleSettings>
  <codeStyleSettings language="JAVA">
    <option name="RIGHT_MARGIN" value="160" />
    <option name="KEEP_LINE_BREAKS" value="false" />
    <option name="KEEP_FIRST_COLUMN_COMMENT" value="false" />
    <option name="KEEP_CONTROL_STATEMENT_IN_ONE_LINE" value="false" />
    <option name="KEEP_BLANK_LINES_IN_DECLARATIONS" value="1" />
    <option name="KEEP_BLANK_LINES_IN_CODE" value="1" />
    <!-- a lot of other options !-->
    <indentOptions>
      <option name="INDENT_SIZE" value="2" />
      <option name="CONTINUATION_INDENT_SIZE" value="4" />
      <option name="SMART_TABS" value="true" />
    </indentOptions>
  </codeStyleSettings>
  <!-- other languages !--> 
</code_scheme>

In the java codestyle settings, the options have a different id attribute (name) compared to their parent (language), that means we have to repeat merge:id="@name" for each option. My idea is to introduce a new merge attribute only for such usecases, where a lot of elements have different id compared to their parent, so it would look like this:

<code_scheme xmlns:merge="https://github.com/devonfw/IDEasy/merge" name="Default" version="173" merge:id="@language" merge:self="combine">
  <JavaCodeStyleSettings merge:ChildrenId="@name"> <!-- no attributes, matches through tag name !-->
    <option name="SPACE_AFTER_CLOSING_ANGLE_BRACKET_IN_TYPE_ARGUMENT" value="true" />
    <option name="ALIGN_MULTILINE_RECORDS" value="false" />
    <option name="ALIGN_TYPES_IN_MULTI_CATCH" value="false" />
    <option name="JD_ALIGN_PARAM_COMMENTS" value="false" />
    <!-- other options !-->
  </JavaCodeStyleSettings>
  <codeStyleSettings language="Groovy">
    <indentOptions>
      <option name="INDENT_SIZE" value="2" merge:id="@name"/>
    </indentOptions>
  </codeStyleSettings>
  <codeStyleSettings language="HTML">
    <indentOptions>
      <option name="INDENT_SIZE" value="2" merge:id="@name"/>
    </indentOptions>
  </codeStyleSettings>
  <codeStyleSettings language="JAVA" merge:childrenId="@name">
    <option name="RIGHT_MARGIN" value="160" />
    <option name="KEEP_LINE_BREAKS" value="false" />
    <option name="KEEP_FIRST_COLUMN_COMMENT" value="false" />
    <option name="KEEP_CONTROL_STATEMENT_IN_ONE_LINE" value="false" />
    <option name="KEEP_BLANK_LINES_IN_DECLARATIONS" value="1" />
    <option name="KEEP_BLANK_LINES_IN_CODE" value="1" />
    <option name="KEEP_BLANK_LINES_BETWEEN_PACKAGE_DECLARATION_AND_HEADER" value="1" />
    <option name="KEEP_BLANK_LINES_BEFORE_RBRACE" value="1" />
    <option name="BLANK_LINES_AROUND_FIELD" value="1" />
    <option name="BLANK_LINES_BEFORE_METHOD_BODY" value="1" />
    <option name="BLANK_LINES_AROUND_FIELD_IN_INTERFACE" value="1" />
    <option name="ALIGN_MULTILINE_PARAMETERS" value="false" />
    <option name="ALIGN_MULTILINE_RESOURCES" value="false" />
    <option name="ALIGN_MULTILINE_FOR" value="false" />
    <option name="SPACE_WITHIN_ARRAY_INITIALIZER_BRACES" value="true" />
    <option name="SPACE_BEFORE_ARRAY_INITIALIZER_LBRACE" value="true" />
    <option name="CALL_PARAMETERS_WRAP" value="1" />
    <option name="METHOD_PARAMETERS_WRAP" value="1" />
    <option name="RESOURCE_LIST_WRAP" value="5" />
    <option name="EXTENDS_LIST_WRAP" value="1" />
    <!--a lot of other options !-->
    <indentOptions>
      <option name="INDENT_SIZE" value="2" />
      <option name="CONTINUATION_INDENT_SIZE" value="4" />
      <option name="SMART_TABS" value="true" />
    </indentOptions>
  </codeStyleSettings>
</code_scheme>

In eclipse's codestyle settings we have the same behavior, but luckily the option elements have an attribute id instead of name.

I already implemented this. WDYT?

hohwille commented 5 months ago

In the java codestyle settings, the options have a different id attribute (name) compared to their parent (language), that means we have to repeat merge:id="@name" for each option.

IMHO the simplest approach could be to assume that an ID is typically defined per tagname. So we could change our design that when an merge:id is processed that then in some "context" we map the qualified tagname to the ID and therefore all following occurrences of an element with this qualified tagname in the same XML document will have this ID configuration unless it is overridden somewhere later in the XML again by an explicit merge:id attribute.