GHC 8.4 compatibility.

[quasicomputational]

This is a simple application of
https://github.com/hvr/head.hackage/blob/master/patches/doctest-0.13.0.patch,
by Ryan Scott.

It's not quite road-ready yet and the 8.4 builder is still allowed to fail.

[quasicomputational]

Turns out that the patches were broken on pre-8.0 GHCs. I've added a semigroups dependency, which is light on new GHCs but heavier on old; if this is unacceptable, some more CPP can be used instead.

[quasicomputational]

I think the -Wmissing-home-module stuff isn't actually GHC 8.4 related, but it would manifest on 8.2 with Cabal 2.0. Either way, that's blocking turning -Werror on for the 8.4 builder. It might actually be a hspec-discover problem rather than a doctest problem.

[sol]

I think the -Wmissing-home-module stuff isn't actually GHC 8.4 related, but it would manifest on 8.2 with Cabal 2.0. Either way, that's blocking turning -Werror on for the 8.4 builder. It might actually be a hspec-discover problem rather than a doctest problem.

The easiest way to fix this is to convert to hpack. Are you already on it? If not, I'll do it now.

[quasicomputational]

That works. I don't really understand how hspec-discover does its thing; it looks like doctest.cabal has the test-component-and-library-component-share-modules pattern going on, but that might be necessary?

[sol]

Regarding hspec-discover: https://hspec.github.io/hspec-discover.html

[quasicomputational]

OK! So hspec-discover is actually irrelevant here, it seems, and the shared test/library modules are only there to expose internals to tests, which is a bit unfortunate but I guess can't presently be helped.

I'll leave the hpackification to you, since you've kindly offered, and once I've cleaned up this last stubborn warning I will squash and I think this will be ready to merge, though I think dependencies aren't ready yet so it shouldn't be released.

[sol]

@quasicomputational I pushed the Hpack changes to master.

[sol]

BTW, thanks for looking into this + I think the approach you took of trying to keep dependencies minimal is the right thing to do for doctest.

[quasicomputational]

Rebased and Travis is just checking that it likes it with -Werror again, but that should be fine and it should be ready to merge.

RE dependencies: hashable is a definite snag, but it might be the only one. haskell-unordered-containers/hashable#149 is tracking that and it might only need a bounds bump on base.

[sol]

I released a new version of hspec that vendors async to get rid of hasable and it's transitive dependencies (see simonmar/async#73). I guess we can get rid of the --alow-newer now + it should also decrease the Travis build time as text takes a long time to build.

[sol]

An other thing I discovered, tests fail with --show-details=direct. Apparently the ANSI sequences in GHC's error messages. Not sure what exactly masks this on Travis, but unless we can confirm otherwise, I assume this impacts doctest users when they use QuickCheck properties.

If GHC has some flag to disable fancy error messages, then that's what we want to use.

[sol]

I still want to get rid of this dependency.

[sol]

We want the simplest possible code here to make it compile (it's an internal type, we don't need the Semigroup instance), something like:

instance Monoid Summary where
  mempty = Summary 0 0 0 0
#if MIN_VERSION_base(4,11,0)
instance Semigroup Summary where
  (Summary x1 x2 x3 x4) <> (Summary y1 y2 y3 y4) =
#else
  (Summary x1 x2 x3 x4) `mappend` (Summary y1 y2 y3 y4) =
#endif
    Summary (x1 + y1) (x2 + y2) (x3 + y3) (x4 + y4)

(untested!)

[quasicomputational]

Weren't fancy colours introduced in GHC 8.2? I don't know why that'd suddenly start being a problem now. The flag to tell GHC to disable it is -fdiagnostics-colour=never.

[quasicomputational]

The 8.4 builder is green (or should be, unless I just messed up a prettifying rebase), but it's still marked as allow-failure. I think, until 8.4's officially released, that's sane.

[sol]

Weren't fancy colours introduced in GHC 8.2?

I guess is broken with 8.2 too. It has been reported as #181.

[sol]

@quasicomputational Thanks a lot! Regarding the Semigroup instance, I still think this is simpler: 512093e

[sol]

On Hackage.

[sol]

@quasicomputational are you on Twitter btw?

[quasicomputational]

Thanks! I'm not a fan of CPP within statements like that, but it's up to you; it's an internal module, so there's no desire to expose a Semigroup instance on 8.0/8.2, either.

No, I'm not on Twitter, sorry.

[sol]

I'm not a fan of CPP within statements like that

Yes, understandable. For me it's a trade off: One solution is to have two CPP statements that have invariants between each other, the second solution is CPP within statements. I don't like neither of it, but given a choice I lean towards the later (for the reason that I don't want to deal with the invariants).