lambdaisland/deep-diff2

Output could use ordered collections to keep it consistently ordered with input

esuomi opened this issue · 4 comments

Eg. https://github.com/clj-commons/ordered provides ordered collections, and the idea really is that there's no reshuffling of especially KV pairs in maps to make it even easier to spot differences between what was provided and what the diff was.

This is an interesting idea, and I agree that it would make the output easier to interpret. I haven't noticed deep-diff output being out of order, possibly because I mostly use deep-diff2 through Kaocha, so I don't see the original order. Is there a particular situation where this came up in?

I don't how optimized ordered collection libraries are or how big of dependencies they are, so I wonder if the best way is to make it configurable, require the user to provide the dependency, or both.

The thing is that this will only help if people are themselves using ordered collections in their code, since otherwise the order we read them in is not deterministic. For small maps clojure maintains source order (PersistentArrayMap). Once you get to bigger sizes it automatically switches to PersistentHashMap, and you can no longer depend on any ordering.

@alysbrooks No particular situation, this was literally just a Friday thought :)

I wonder if the sorting could be applied after diffing? After all it's more of a view thing than anything.

I think there are two ideas here:

  • Add support for flatland.ordered collections. This should be done in a bring-your-own-dependency fashion—we don't want to pull in this library if people aren't using it.
  • Sort. This can be probably be done by the library user but isn't straightforward since the Deletion and Insertion objects don't implement Comparable.

I think we'd welcome a PR for either.

The main reason to not accept support for flatland.ordered is that it could be a maintenance burden for a feature very few people use. On the other hand, that library appears to be very stable. We'd definitely want tests for it.