Improved error diagnostic output

[GoogleCodeExporter]

It would be great to have improved error trapping when a non-existent node is 
accessed.

For example:
  const int = parser["test"].as<int>();   // "test" does not exist
results in a nondescript error:
  yaml-cpp: error at line 0, column 0: bad conversion

It would be great to have something like this instead:
  Node "test" was not found.

Even better, if I had YAML:
  entry:
    tag1: a
    tag2: b
and then in C++:
  parser["entry"]["test"].as<int>();  // doesn't exist

It would be great to see an error message like
  Node "test" was not found as a child of "entry"

Thanks!

Original issue reported on code.google.com by jamessu...@gmail.com on 13 Mar 2014 at 6:41

• Merged into: #104

[GoogleCodeExporter]

Original comment by jbe...@gmail.com on 13 Mar 2014 at 6:50

• Changed state: Accepted
• Added labels: Component-New-API

[GoogleCodeExporter]

Along these lines, it would be helpful for a node to know its name.  That would 
facilitate improved error trapping as above.

Original comment by James.Su...@chemeng.utah.edu on 28 Mar 2014 at 1:07

[GoogleCodeExporter]

@James.Sutherland, please be more specific - it doesn't really make sense to 
talk about a node's "name".

Original comment by jbe...@gmail.com on 28 Mar 2014 at 1:31

[GoogleCodeExporter]

If I had something like this:
  Fruits:
    - apple
    - orange
    - pear
Then it would be great to have the ability to do:

  const YAML::Node fruits( parser["Fruits"] );
  BOOST_FOREACH( const YAML::Node& nd, fruits ){
    cout << nd.name() << endl;
  }

If this was available, then part of the error diagnostics that I mentioned 
above would be done.  The only remaining thing would be for each node to record 
a pointer to its parent node so that we could get exceptions with the 
information I suggested.

BTW, thanks for your responsiveness to questions on stackoverflow!  It is great!

Original comment by James.Su...@chemeng.utah.edu on 28 Mar 2014 at 1:44

[GoogleCodeExporter]

Thanks! :)

The thing is, I get what you're saying when there's a string key, but it can't 
really be implemented as you're writing, since, again, "name" doesn't really 
make sense. Aside from non-string keys (and what *do* you do about non-string 
keys?), you also have to decide what to do with anchors and aliases. E.g.,:

fruits:
  - apple
  - &foo orange
spheres:
  - *foo
  - basketball

What's the "parent" of "orange"? Remember, it's the *same* node in each 
instance, not a copy.

Original comment by jbe...@gmail.com on 28 Mar 2014 at 1:49

[GoogleCodeExporter]

For non-string keys, something like:
  node.set_name( boost::lexical_cast<std::string>( key ) );
should work, right?

I do see the potential problem with the parent issue in the example you 
describe with anchors.  A potential fix would be to record this on the user 
referencing interface rather than during parsing.  Then, if you had:

  string s1 = parser["fruits"]["orange"].as<string>();  // works
  string s2 = parser["fruits"]["grape" ].as<string>();  // throws an exception indicating that "'fruits' has no child named 'grape' "

This would circumvent the problem since the temporary node created by the [] 
operator could have a parent associated with it such that:

  Node n1 = parser["spheres"]["orange"];
  Node n2 = parser["fruits"]["orange"];
  assert( n1.parent().name() == "spheres" );
  assert( n2.parent().name() == "fruits" );

Make sense?

Original comment by James.Su...@chemeng.utah.edu on 28 Mar 2014 at 1:59

[GoogleCodeExporter]

Still not convinced. It adds a lot of complexity, lots of ambiguity (there are 
several ways to interpret things here), and above all, I can't think of a 
proper name for this method ("name" isn't accurate).

I'm still open to being convinced, but you need to really have thought through 
some of the details here. (E.g., lexical_cast probably isn't what you want 
here...)

Original comment by jbe...@gmail.com on 28 Mar 2014 at 3:40

[GoogleCodeExporter]

I have hacked in something that moves in the direction I am suggesting.  This 
isn't pretty since I am not terribly familiar with the library, but if you run 
a diff against the current release you will see what I have done.

Let me know what you think.

Original comment by James.Su...@chemeng.utah.edu on 5 Apr 2014 at 9:21

Attachments:

• yaml.zip

[GoogleCodeExporter]

Sorry - please use this version instead.  I neglected one final fix in the 
previous version.

Original comment by James.Su...@chemeng.utah.edu on 5 Apr 2014 at 9:33

Attachments:

• yaml.zip

[GoogleCodeExporter]

A zip file of the code is probably not the easiest thing to look at. If you do 
have a suggestion, please submit a patch, ideally against the tip of the repo, 
not the current release.

(Also, I noticed that you still called it "name". *Please* change that - it's 
essentially a non-starter, since it is neither standard YAML language nor a 
good description of its behavior.)

Original comment by jbe...@gmail.com on 5 Apr 2014 at 9:41

[GoogleCodeExporter]

Here is a patch to apply to release 0.5.1.  I was working off that copy, and 
didn't want to go to the effort of merging to the tip until/unless you felt 
that this was a reasonable approach to improving error trapping.

I changed "name" to "label" but wasn't really sure exactly what alternative you 
prefer.

Note that there is a key file added (include/node/detail/stringize.h) that 
handles error trapping for the supported key types as far as I could determine 
them.  I am not sure if you have a better way of handling this or not.

The basic idea here is that when a node is accessed via operator[], it leaves 
crumbs in the form of a "label" and a "parent" node so that one can trap errors 
with some sort of reasonable error message. 

Thanks for your willingness to consider this.  Even if you decide to not merge 
something based on this into the main development trunk, I would appreciate any 
suggestions for improvement so that I can maintain this locally.

Original comment by James.Su...@chemeng.utah.edu on 6 Apr 2014 at 12:35

Attachments:

• patch_0.5.1.diff

[GoogleCodeExporter]

Original comment by jbe...@gmail.com on 24 Jan 2015 at 10:32

• Changed state: Duplicate