Added property `createdWithError`

[Parabak]

The value is true if NodeSet was generated as a result of badly formed or unsupported XPath query. libxml2 in that case will return nil instead of a pointer. This bool property will provide a way how to distinguish to cases

• NodeSet is empty because there are no queried elements
• NodeSet is empty because of XPath syntax error

[cezheng]

This diff is unnecessarily big and hard to see what you actually changed, can you revert the indent changes?

[cezheng]

Also, I think it might be better to throw an error instead of adding a field which is not like swift convention

[Parabak]

ok, will try to make it a little bit more swifty. thanks for quick response.

[Parabak]

Hi, I've tried to make it in a more swifty way. What do you think?

[cezheng]

The code looks good to me, but after seeing the change in usage, the trys seem pretty annoying, so I'm actually not sure if making it swifty is a good idea any more.

I would like to see what users think about this change.

Would you mind explaining a bit more about the scenario where this error is useful to you? Thanks

[Parabak]

The main idea was to handle errors reported by built in libxml2 XPath validator. For example, in most cases xPath = "////*" will not work. However libxml2 treat it as "//*". So no error and it means that we can't just validate our xpath using for example https://codebeautify.org/Xpath-Tester. Smth that will pass libxml2 validation will be failed in another system and vice versa. In more complex cases like using nested functions it even easier to make a mistake in xPath. Imagine that xPath is not hardcoded in the codebase but coming from the feed(BE). Without these chunk of changes we can't see that smth went wrong. We received an empty NodeSet, what means that maybe xpath wrong or most likely no matches.

[Parabak]

If I still didn't convince you, than I may suggest to edit code the following way: we will have two methods to perform xpath - old one without throwing an error and a new one with it. for example: func try(xPath: String) throws -> NodeSet

[cezheng]

Yup, I like the idea to have another function and keeping the old functions as they are. That won't affect existing users of the lib.

[Parabak]

Will check what's wrong with with tests...

[cezheng]

This simply wraps libxml's error functions and will require users to reset error every time which is very much like manual memory management and error prone. Why not just keep the throw version of xpath as you previously implemented? They can coexist with the existing functions

[Parabak]

I think there is no extra memory management required, is it? I thought that it will give a liitle bit more flexibility in a following way: in case i as a user need to check if there is an error I can retrieve it. So in case were I really care about error I will call resetLastError, than do xpath and check lastError. It will be nil if everything is ok. Otherwise in other cases getting empty NodeSet is totally fine.

[cezheng]

I'm not saying it manual memory management but I'm saying it is similar to that.

In C, if you malloc something, then you forget to free, you have a memory leak.

Here, if you execute an xpath and forget to reset the xml error, this will leave the system in an incorrect state, next time you execute an valid xpath, you can still get the error which is not right.

[Parabak]

OK, will come back soon and implement in raising error way