Fix bug on tags starting with "!!"

[adfhkjafsdk]

Fixes #1373 .

This adds support for SecondaryTag in EmitFromEvents::EmitProps, resolving tags starting with "!!".

Now the output of the example code is

some_string: !!str hello
some_int: 2

[SGSSGene]

@adfhkjafsdk Very cool! Could you add some tests?

[adfhkjafsdk]

@SGSSGene Thanks for your appreciation!

I added some tests for SetTag. I also noticed that we lack test for emitting SecondaryTag, added to test/integration/emitter_test.cpp.

After adding the test, it occurred to me that EmitFromEvents::EmitProps has problems handling named tag handles, like !a!foo (see SetLocalTagInNameHandle in emitter_test.cpp), and I also fixed it.

By the way, I'm not sure what the expected output is for codes like this:

    YAML::Node node;
    node = 42;
    node.SetTag("!a!");

    YAML::Node root;
    root["num"] = node;

    YAML::Emitter out;
    std::cout << out.c_str() << std::endl;

Currently I left it num: !<!a!> 42.

[SGSSGene]

@adfhkjafsdk Thank you so much. Good to see progress on these issues!

You are using 'tabs' instead of spaces in the source code. Please change it to spaces.

About the 'Named handles'.
If I read the specs correctly a tag !a! is not allowed, because it has a non-empty suffix (!a!b would be fine).
See 6.9.1 Node Tags specifically the Tag shorthands.
The example in Example 6.27 Invalid Tag Shorthands even shows this as invalid.
So I think the right thing to do is throw an exception for invalid tag.

Secondly, currently we do not really have support for named tags at all.
Strictly speaking even !a!b would be invalid if !a! wasn't specified before.
The currently master implementation just handles this case wrongly which might be very surprising for library users.
I think it would be much nice to also throw an exception with some thing like "!TODO: Unsupported named tag". And maybe in that code section a comment to this PR so the next person knows where we left off.

@jbeder: do you have any insights to the TAG support of yaml-cpp?

[adfhkjafsdk]

@SGSSGene Thank you for your hard work. I feel sorry about the poor indentation.

I totally agree with the idea of throwing an exception for invalid tag. I wonder why the old implementation just ignore the invalid tag ! when meeting it.

I know that !a!b is a Tag Shorthand, and it is invalid without previous TAG directive. But since we've allowed usage like out << LocalTag("a", "foo") << "bar"; (see LocalTagInNameHandle in test/integration/emitter_test.cpp), which outputs !a!foo bar, I think this should also be allowed through SetTag. Anyway, if we ban the two ways, there may be no way to emit tag shorthands. I think it may be better to let the emitter check whether the TAG directive exists.

Currently there's no throw in the emitter code and no Exception type for emitter exceptions. Maybe the author thought checking validity isn't what the emitter is supposed to do?

By the way I felt it inconvenient to store tag as a raw string (YAML::detail::node_data::m_tag), maybe in the future we should change it to more structured representations.

[SGSSGene]

@SGSSGene Thank you for your hard work. I feel sorry about the poor indentation.

To me the same happens, because of the difference between 2 vs 4 space indentation.

I totally agree with the idea of throwing an exception for invalid tag. I wonder why the old implementation just ignore the invalid tag ! when meeting it.

I believe this is something only @jbeder can tell us. I wouldn't be surprised if this just wasn't focus and is just open to do.

I know that !a!b is a Tag Shorthand, and it is invalid without previous TAG directive. But since we've allowed usage like out << LocalTag("a", "foo") << "bar"; (see LocalTagInNameHandle in test/integration/emitter_test.cpp), which outputs !a!foo bar, I think this should also be allowed through SetTag. Anyway, if we ban the two ways, there may be no way to emit tag shorthands. I think it may be better to let the emitter check whether the TAG directive exists.

I agree, there wouldn't be any way of emitting a shorthand tags (currently it is also not possible). I would argue
that fixing this should be done in another PR, keeping each PR complexity low and easy to overlook and review.

Currently there's no throw in the emitter code and no Exception type for emitter exceptions. Maybe the author thought checking validity isn't what the emitter is supposed to do?

Another good question, maybe @jbeder has some insights here?

By the way I felt it inconvenient to store tag as a raw string (YAML::detail::node_data::m_tag), maybe in the future we should change it to more structured representations.

This is something I would need to look into and can't comment on :-)