majintao0131 / yaml-cpp

Automatically exported from code.google.com/p/yaml-cpp
MIT License
0 stars 0 forks source link

Unsupported merges #41

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Current version of otherwise great yaml-cpp does not seem to support merges
in a way described for example here:
http://en.wikipedia.org/wiki/YAML#Data_merge_and_references

I don't have a deep knowledge of yaml-cpp internals (I am just its user),
but maybe this is somehow linked with the cloning problem - not yet
sufficiently solved.

I have tested the simple form of a merge in yaml-cpp 0.2.0 on RedHat Linux
3.4.6-9 using gcc 3.4.6 (20060404) - it doesn't work, but at least it
doesn't hurt the parser. With that gcc I am unable to compile 0.2.1 because
of new conversion operators in nodeimpl.h (it reported an error on lines
with "node.Read<T>" and the thing that caused that error was very probably
the templated read).

Do you plan to support merges in some future version?

Original issue reported on code.google.com by pole...@gmail.com on 8 Sep 2009 at 10:38

GoogleCodeExporter commented 9 years ago
To be even more precise - I just found the definition of merge right on 
yaml.org in
the following document: http://yaml.org/type/merge.html

Such a functionality is very useful if a YAML file is used as a configuration 
file,
because it allows creation of configuration groups and either overriding some 
values
or adding another values in different keys being aliases of a group.

Original comment by pole...@gmail.com on 8 Sep 2009 at 11:11

GoogleCodeExporter commented 9 years ago
Another clarification:

"With that gcc I am unable to compile 0.2.1 because of new conversion operators 
in
nodeimpl.h ..." - I thought (templated) comparison operators, of course. Sorry 
for
the mistake.

Maybe I will raise another issue regarding this problem later, but for now 
0.2.0 is
enough for me and there's nothing so important for me in 0.2.1 (and brand new 
0.2.2)
that I can't live without ;-)

Original comment by pole...@gmail.com on 8 Sep 2009 at 11:39

GoogleCodeExporter commented 9 years ago
Two things:

1. I actually didn't realize that merge was officially a part of YAML. I read 
it on
the wikipedia page, but the spec doesn't have any reference to a merge. Given 
that it
does seem to be officially supported, I may add it in, but I'm not 100% sure 
what
status the yaml.org/type/* files should have. I do understand that this is very
useful, though (the so-called "prototype" pattern, according to Steve Yegge).

2. What do you mean by you're unable to compile it? Does the library (by 
itself) not
compile on gcc 3.4.6? Or is it some extension code you've written that doesn't
compile? If it's the latter, what does the code look like? In either case, what 
is
the error message?

Original comment by jbe...@gmail.com on 9 Sep 2009 at 1:47

GoogleCodeExporter commented 9 years ago

Original comment by jbe...@gmail.com on 9 Sep 2009 at 1:47

GoogleCodeExporter commented 9 years ago
1. Thank you for your positive opinion. Because of it this issue hopefully 
remains
open until you implement the merge.

2. Of course the library itself cannot be compiled. I made a mistake to run gcc 
-v,
because it is of some other version than g++32 actually used for compilation. 
See
http://code.google.com/p/yaml-cpp/issues/detail?id=42 for details. But I must 
say,
that with the following g++ (not some older g++32) everything is OK, even 
#pragma once:

$ g++ -v
Reading specs from /usr/lib/gcc/x86_64-redhat-linux/3.4.6/specs
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info --enable-shared --enable-threads=posix 
--disable-checking
--with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions
--enable-java-awt=gtk --host=x86_64-redhat-linux
Thread model: posix
gcc version 3.4.6 20060404 (Red Hat 3.4.6-3)

Original comment by pole...@gmail.com on 9 Sep 2009 at 7:59

GoogleCodeExporter commented 9 years ago
Yeah, I'll definitely leave this open until I implement it.

As for the gcc version, I do most of my testing with gcc 4.0.1 or 4.1.2. It's 
good to
hear it works with *something* in the 3.4.* range. It's very likely a bug in 
3.2.3
that's been fixed - why are you using 3.2.3?

Original comment by jbe...@gmail.com on 9 Sep 2009 at 4:52

GoogleCodeExporter commented 9 years ago
As a comment I added to the Issue 42 says, the reason for using such 
prehistoric GCC
was Oracle 10g compatibility. Nowadays we are using Oracle 11g and are happy 
with GCC
3.4.6 ;-)

Original comment by pole...@gmail.com on 1 Oct 2009 at 10:47

GoogleCodeExporter commented 9 years ago
I'm missing merges too, is there any update on the subject?
Can you estimate when it will be implemented (if you have such plans)?
In case I'll have time to contribute, can you estimate the amount of work 
involved?
Thanks.

Original comment by oranagra...@gmail.com on 21 Mar 2010 at 1:19

GoogleCodeExporter commented 9 years ago
The problem with merges is that I think I'd like to restructure the 
architecture of 
Nodes a bit. It wouldn't be that hard to "glue" merges onto the current 
structure, but 
it would be a big wart, and I want to do it the "right way".

As-is, there are a number of other things I'm working on now, so it probably 
will be 
at least several months before I get to this.

If you're interested in contributing, I'd be happy to see your patch, but I 
probably 
wouldn't immediately apply it (though I probably *would* take some hints from 
your 
code).

Original comment by jbe...@gmail.com on 21 Mar 2010 at 4:01

GoogleCodeExporter commented 9 years ago
Technically, the merge specification come from the types repository for YAML 
1.1,
while yaml-cpp is based on YAML 1.2.  I can understand how it would be a nice 
feature
to have, but there is a fundamental issue regarding equality of scalar values 
that
needs to be resolved first.  After all, you can't do a merge unless you can 
decide
whether the strings "17" and "0x11" (when expressed as plain scalars) are equal.

Without implementing a (extensible) mechanism for resolving scalars to canonical
form, I'm not sure how this can be implemented inside the yaml-cpp library and, 
IMO,
the client code should only get this functionality if it specifically requests 
it.

Original comment by rtweeks21 on 26 Mar 2010 at 11:44

GoogleCodeExporter commented 9 years ago
Richard, you're right, but we actually do that kind of resolution in yaml-cpp 
already - 
it's just merging raises more questions about it. For example,

0x11: foo
17: bar

is a legal yaml-cpp node, but `node[17]` will only retrieve one (unspecified 
which 
one, and you'd have to iterate through to get the whole thing, or ask for 
`node["0x11"]`). But the corresponding

0x11: foo
>>:
  17: bar

will also be a legal node, but in this case `node[17]` will retrieve `foo` (and 
to get 
'bar', I'm really not sure what you'd get if you iterate through the node).

I do completely agree that you only get this functionality if you specifically 
request it.

Original comment by jbe...@gmail.com on 28 Mar 2010 at 1:18

GoogleCodeExporter commented 9 years ago
Hi, I need merges too, so I did a small fix to support this. It works fine for 
me, but I do not know project's architecture, so afraid if some side affects 
can occur.
The change is to insert the lines below into FindValueForKey template (between 
for-loop and return 0):

        const Node *pValueMerge = FindValueForKey(std::string("<<"));
        if(pValueMerge)
        {
            return pValueMerge->FindValueForKey(key);
        }

Could you please comment?

Original comment by barma...@gmail.com on 31 Jan 2011 at 7:12

GoogleCodeExporter commented 9 years ago
@barma:

That would work for merges with a single dictionary, but the spec allows

>>: [*dict1, *dict2]

to merge multiple dictionaries.

Original comment by jbe...@gmail.com on 31 Jan 2011 at 5:52

GoogleCodeExporter commented 9 years ago
Any updates to this?  I have a overridable configuration situation which would 
be handled cleanly if there was this merge support in yaml-cpp.

Original comment by nev...@gmail.com on 12 Nov 2012 at 2:31

GoogleCodeExporter commented 9 years ago
Sorry, no updates to this. It's pretty low priority, to be honest, especially 
given all of the difficulties in making it "correct" (detailed above).

Original comment by jbe...@gmail.com on 12 Nov 2012 at 5:20

GoogleCodeExporter commented 9 years ago
Any updates this year?

Original comment by ben.far...@gmail.com on 7 Mar 2014 at 5:26

GoogleCodeExporter commented 9 years ago
Nope :(

Original comment by jbe...@gmail.com on 7 Mar 2014 at 5:32

GoogleCodeExporter commented 9 years ago
I have implemented this feature in my fork of the 0.5.1 code. It's gotten 
slightly behind the core here, so maybe it's not a straight patch, and the 
coding style is inconsistent, but it's still better than nothing.

The revision which implements this feature can be viewed here: 

https://github.com/WrinklyNinja/yaml-cpp/commit/c55b40b5ea42e337bb1c3b1870d1508f
f3a99966

I hope that helps!

Original comment by Oliver.H...@gmail.com on 23 Oct 2014 at 2:29

GoogleCodeExporter commented 9 years ago
I applied the patch to my copy of 0.5.1, and it worked as expected (at least 
for the use case that I tried). Thanks!

Original comment by sho...@gmail.com on 3 Nov 2014 at 7:17

GoogleCodeExporter commented 9 years ago
This issue moved to github: https://github.com/jbeder/yaml-cpp/issues/41

Original comment by jbe...@gmail.com on 30 Mar 2015 at 1:17