renggli / dart-xml

Lightweight library for parsing, traversing, and transforming XML in Dart.
http://pub.dartlang.org/packages/xml
MIT License
223 stars 52 forks source link

Draft: feat: implement equality operators #153

Closed TheOneWithTheBraid closed 1 year ago

TheOneWithTheBraid commented 2 years ago

Those changes are particularly relevant when directly comparing unknown XML encoded data. The equality operator - as documented - is not meant to check whether the current representation of the data is equal, but just whether the data itself is equal. In order to reach there, e.g. empty text nodes or comments are ignored, attributes are ordered and namespaces are compared according to their URI, not their prefix.

Signed-off-by: TheOneWithTheBraid the-one@with-the-braid.cf

renggli commented 2 years ago

Thank you for your pull request.

I am worried with a default implementation of equality: There are a lot of very application specific decisions that might not be desired by everybody (removing of namespace declarations, trimming of whitespaces, ...). Also the provided operators are rather expensive, recursively traversing the whole DOM tree.

Not to provide a default (and possibly unsuitable) default comparator is intentional. The idea is that (in the rare case you need a custom comparison) you can build your own custom comparator. By default there is == and hashCode inherited from Object (the way like most other libraries do).

I agree that there could be some way to make such comparators easier to implement. Open for suggestions on that front.

TheOneWithTheBraid commented 2 years ago

I definitely see the point, that's why the PR is marked as draft.

Maybe we could think about a class like XmlComparator taking two XmlNodes and options on the configuration of equality of nodes in the particular use case?

Kind of:

final comparator = XmlComparator(node1, node2);
comparator.preserveAttributeOrder = true;
comparator.namespaceMode = NameSpaceMode.prefix; // or NameSpaceMode.uri
comparator.dataMode = <XmlNodeType>{ XmlNodeType.ELEMENT, XmlNodeType.TEXT};
TheOneWithTheBraid commented 2 years ago

But especially about the current behavior of XmlName's == operator, I am highly concerned. The same prefix does not at all indicate equality in this case. If the aim of the package is is to have equality checks implemented by the users, the operator should definitely be removed there as the behavior only covers a few se cases at the moment.

renggli commented 2 years ago

Good point about XmlName. I wasn't ware of it and I am not sure where this == implementation came from? Presumably this was added with Builder.optimizeNamespaces, which is the only user within the library aside from tests.

I imagine we could have something like Normalizer that is customisable and returns a Comparator<XmlNode> (like your example in https://github.com/renggli/dart-xml/pull/153#issuecomment-1240594206)?

renggli commented 1 year ago

Closing this as it was resolved with https://github.com/renggli/dart-xml/commit/366149f376ca9ce137c0a62a9f53e356477e383e.