magynhard/yaml_extend

merged order wrong?

Closed this issue · 10 comments

I thought this package would be a very nice addition to a mechanism I have in place to specify settings/configurations...but it seems the order in which merged files end up in the final result are giving not the results I would desire.

My settings syntax is as follows:

block:
- rule1:
   setting a
   setting b
   ....
- rule2:
  setting c 
  ...

The rules determine on what settings get applied. It is legal to list the same rule multiple times (that is why it is an array). If a setting is repeated, the 'last one wins' which feels most intuitive and follows what yaml does.

However in the following example:

# base.yaml
---
blockX:
- playground:
    macros:
      M_BASE: ja
      M_REPLACE: to_be_replaced
...
# settings.yaml
---
extends:
  - 'base.yaml'
blockX:
- playground:
    macros:
      M_DERIVE: ja
      M_REPLACE: replaced
...

Gives as result:

---
blockX:
- playground:
    macros:
      M_DERIVE: ja
      M_REPLACE: replaced
- playground:
    macros:
      M_BASE: ja
      M_REPLACE: to_be_replaced
...

In which it is unfortunate that the extended base comes last in the list, as a result when I process the file the macros result in:

M_DERIVE: ja
M_BASE: ja
M_REPLACE: to_be replaced ---> should haven bee 'replaced'

Would it be difficult to put the merged list in the order from base --> derived? (maybe optional)

BTW: this seems to have a similar cause as #5

Actually...the example you show on the main page, is not working as written. The outcome I get is:

---
data:
  name: Mr. Superman
  age: 134
  favorites:
  - Raspberrys
  - Bananas
  - Apples
  power: 2000

As you can see some of the items (especially the list of fruits) are in another order compared to the main page. Here they are in 'extensions last'

Probably, often this does not matter, but in my case the order of array entries means something.

Pull request #8 adds an option to yaml_extend to traverse the tree starting at the base class

@NicoLugil, thank you very much for your issue and pull request.

I would like to provide an option/fix for this requirement and will check your pull request soon.

Hi @NicoLugil,

i want to ensure to understand your problem exactly. Did you expect the following result in your example?

---
blockX:
- playground:
    macros:
      M_BASE: ja
      M_REPLACE: to_be_replaced
- playground:
    macros:
      M_DERIVE: ja
      M_REPLACE: replaced

Correct!

This is what my pull request gives if you use the option tree_traversal: :postorder

YAML.ext_load_file 'settings.yaml' gives:

{"blockX"=> [{"playground"=>{"macros"=>{"M_DERIVE"=>"ja", "M_REPLACE"=>"replaced"}}}, {"playground"=>{"macros"=>{"M_BASE"=>"ja", "M_REPLACE"=>"to_be_replaced"}}}]}

YAML.ext_load_file 'settings.yaml', tree_traversal: :postorder gives

{"blockX"=> [{"playground"=>{"macros"=>{"M_BASE"=>"ja", "M_REPLACE"=>"to_be_replaced"}}}, {"playground"=>{"macros"=>{"M_DERIVE"=>"ja", "M_REPLACE"=>"replaced"}}}]}

Thank you very much for your reply and PR.

As i figured out, the PR
nangelovaTeladoc#1
Does fix your issue and addresses the multi-file inheritance precedence order as well.

So i prefer using that solution and hope you don't feel bad when i have to reject your suggested solution.

If it works then that is fine.
However could you run the more elaborate test I provided with a0, a1, ... first to make sure it does give the correct result?

The one thing I did like about my solution is the use of rubytree.
It as a first step builds a tree.
In the next step it traverses and merges the tree in the desired order.
Rubytree has multiple built-in traversal schemes. If you want to keep that flexibility you might consider having another look at this solution.
It should also fix the multi file order issue.

I will definitely add more test cases, especially on the subject of the merging order.

And I'll reconsider your remarks.