haskellari / tree-diff Goto Github PK

This would allow tree-diff to compile in an lts-12.x environment.

Currently the full context is shown when pretty printing diffs, e.g. the entire record. It would be useful to be able to display only the minimal relevant context in which the diff is understandable, e.g. only the record constructor and the changed fields.

Hey,

Any particular reason why this package used to be BSD licensed, but was changed to GPL?

It would be tremendously useful to split out the ToExpr type class together with Expr data type into a separate package.

It would allow libraries to define ToExpr instances with minimal set of transitive dependencies. tree-diff isn't exactly a lightweight package when it comes to dependencies

In other words libraries then would be able to define ToExpr instances by only depending on tree-diff-class, while test suites would depend on the full ctree-diff package

In primitive 0.8.0.0, if base >= 4.17 then Data.Primitive.ByteArray is just a reexport of Data.Array.Byte.ByteArray. This causes the instances in Data.TreeDiff.Class to be redundant.

Would it be possible to extract code that forces GPL2 license on the package into a separate package and release tree-diff under a more permissive license?

GPL2 license makes this package a bit less useful than it otherwise could be.

The Data.TreeDiff.List.diffBy function allows you to specify a comparision (a -> a -> Bool) function, and throws away the other element if it succeeds, yielding a Cpy a; seemingly very sensible.

But, if your comparision is not ==, it may be convenient to also retain the other parameter, giving a Cpy like Cpy (a,a) or more generally perhaps a monoid like Cpy (f a), then it would be a First or a list or whatever you wish ...

Is there interest in a change like this? I assume not, but I just want to at least note that this has been a useful change for me, and if others would find it a worthwhile api-breaking change I'd happily make a PR for it.

The library does just what I was looking for, but it is to complicated to use it in existing project for app config type, because it contains dozens of types spread across submodules and 3rd packages. To much work to make 1 test failure more readable. I would like to use ediffEq for Showable types. ediffEq doesn't look inside strings and it is not obvious what is different when long strings are compared.

Your package looks cool and I wanted to use it in tests for xlsx library but dependency restrictions are quite high. Are they strictly required? Probably you use some features which are missing from lower versions, right?

Motivation: commercialhaskell/stackage#7306

Problem

I have some tests using tasty-golden + ediffGolden with data structures containing HashMap values. These tests work locally, but fail on CI due to the CI server generating Expr values for the HashMap values with the items in a different order than was captured in the golden snapshot.

Analysis

ediffGolden assumes (?) that a == b implies toExpr a == toExpr b.

However, the ToExpr instance for HashMap (source) captures the raw ordering returned by HM.toList, which isn't reliable across platforms (or in the presence of collisions and insert reordering), so the above property doesn't hold.

(This also affects the ToExpr instance of HashSet.)

Workaround

I'm already using a Expr construction in the affected code, so for the time being, I'm just using this variation of toExpr for HashMap sorts the items:

toExpr_HM :: (ToExpr k, ToExpr v, Ord k, Ord v) => HM.HashMap k v -> Expr
toExpr_HM x = App "HM.fromList" [ toExpr $ sort $ HM.toList x ]

Encountered during stackage nightly build.

tree-diff> test (suite: tree-diff-test, args: --quickcheck-replay=579929)
                       
tests
  trifecta-pretty roundtrip:       OK (5.60s)
    +++ OK, passed 100 tests.
  parsec-ansi-wl-pprint roundtrip: FAIL (0.91s)
    *** Failed! Falsified (after 33 tests and 16 shrinks):
    Rec "" (fromList [("\"\n\"",Rec "" (fromList []))])
    `` {" " = `` {}}
    Rec "" (fromList [("\" \"",Rec "" (fromList []))])
    Just `` {" " = -`` {}, " " = +`` {}}
    Use --quickcheck-replay=579929 to reproduce.
    Use -p '/parsec-ansi-wl-pprint roundtrip/' to rerun this test only.
  diffBy example1:                 OK
    +++ OK, passed 1 test.
  diffBy model:                    OK (0.04s)
    +++ OK, passed 100 tests.
  Golden
    exFoo:                         OK
    MyInt1:                        OK
    MyInt2:                        OK
    MyInt3:                        OK
    Positional:                    OK
    HashSet:                       OK
      Golden file did not exist; created

1 out of 10 tests failed (6.55s)

produces:

src/Data/TreeDiff/Class.hs:382:30: error:
    Not in scope: type constructor or class ‘Semi.Option’
    Module ‘Data.Semigroup’ does not export ‘Option’.
    |
382 | instance ToExpr a => ToExpr (Semi.Option a) where
    |                              ^^^^^^^^^^^

src/Data/TreeDiff/Class.hs:383:13: error:
    Not in scope: data constructor ‘Semi.Option’
    Module ‘Data.Semigroup’ does not export ‘Option’.
    |
383 |     toExpr (Semi.Option x) = App "Option" [toExpr x]

ansi-terminal-1.0 is released on Hackage.

See also commercialhaskell/stackage#6976.

Based on my tests, this is really only a bounds change. I tested by building with Stack >= 2.9.3 and modified stack.yaml:

# resolver: lts-13.23
resolver: lts-20.21 # GHC 9.2.7
packages: [.]

extra-deps:
- ansi-terminal-1.0
- ansi-terminal-types-0.11.5

allow-newer: true
allow-newer-deps:
- ansi-wl-pprint

% cabal build  --with-compiler=ghc-8.2.2 -vnormal+markoutput     
-----BEGIN CABAL OUTPUT-----
Build profile: -w ghc-8.2.2 -O1
In order, the following will be built (use -v for more details):
...
 - tree-diff-0.1 (lib) (first run)
-----END CABAL OUTPUT-----
Downloading  base-orphans-0.8.1
-----BEGIN CABAL OUTPUT-----
Starting     bytestring-builder-0.10.8.2.0 (lib)
-----END CABAL OUTPUT-----
-----BEGIN CABAL OUTPUT-----
Starting     colour-2.3.5 (lib)
-----END CABAL OUTPUT-----

The Downloading is cabal's output.

Could you please relax the version constraint on base on Hackage to allow for building with GHC 8.4.1?