Cleanup error checking in urdfdom

[clalancette]

This does two things:

1. Check the return value from parseMaterial, parseLink, and parseJoint. While these can throw a ParseError exception, most often they return false when they fail to parse.
2. Revamp the error checking around those calls so that the code is indented a lot less. It is much easier to read this way.

The first point, in particular, has the chance to have a bit of semantic change in that XML that was "accepted" before may be dropped now. However, I think it is worthwhile doing the correct thing here, though we may want to think about only putting this in Lunar and newer.

[sloretz]

Looks good to me.

Would you mind giving an example of some XML that was accepted before this change but not after? The only difference that stands out to me is the parseLink return value wasn't being checked before.

[sloretz]

@clalancette ping. I forgot about this one. Is it something that should be merged soon?

[clalancette]

@clalancette ping. I forgot about this one. Is it something that should be merged soon?

Sorry I didn't get back to this. Will try to come up with some example XML and add it to the PR.

[clalancette]

So, I just added a couple of tests showing things that would have successfully parsed before, but now fail.

However, looking at this, I am starting to get a bit more concerned about the backwards-compatibility impact of it. In particular, this will now reject "incorrect" URDF files that used to work (one good example is ones without a name for their material). These are supposed to be errors, but since they haven't been in the past, we may break and frustrate users with this going forwards. So I'm not quite sure what to do about it. @sloretz , @scpeters , opinions?

[sloretz]

Depends on what the master branch means. @scpeters what does master mean on this repo? Is it the next major release like default on the gz/ign mercurial repos?

Maybe it's worth doing a tick-tock with a warning first and an error later?

[scpeters]

version 1.0.0 was released from master, and I haven't made a release branch since we haven't made any breaking changes since then. I'll take a look at the changes

[scpeters]

Yeah, I think a tick-tock would be good here.

[scpeters]

This now has conflicts, possibly since I just merged #114

[clalancette]

Rebased now, though I haven't considered the tick-tock mechanism yet.