Fix ShellCmd String arguments

[cunningdefenestrator]

Fixes #143.

Changelog:

• Rework ShellCmd's instances to support String arguments.
• Add some tests.
• Remove the IncoherentInstances pragma as it's deprecated.
• Bump the major version as CmdArg's method has changed.
• Bump the resolver to lts-20.04 (the latest as of this writing).
• Bump haskell-ci to 0.15.20221107

[cunningdefenestrator]

Looks likes this succeeds for everything but ghc 8.0.2--I'm not sure why because AFAICT the instance selection rules haven't changed. How should I proceed?

[andreasabel]

@cunningdefenestrator : Thanks for the PR.
I couldn't push to your branch (permission denied). Could you give me permission?
In the meanwhile, I backed this PR up to:

• Fix ShellCmd String arguments #223

[andreasabel]

Looks likes this succeeds for everything but ghc 8.0.2--I'm not sure why because AFAICT the instance selection rules haven't changed. How should I proceed?

GHC 8.0 can be considered a legacy GHC by now, so a pragmatic solution would be to drop GHC 8.0.
(One could also investigate whether/how the broken files could be fixed for GHC 8.0.)

You suggest a major-major version bump to 2.0.0---do you expect a lot of breakage?

[cunningdefenestrator]

@andreasabel I've sent an invite.

If we're being strict about SemVer this deserves a major version bump since I changed the signature of CmdArgs. In practice, I don't think too many people would use CmdArgs directly, but I'll defer to your judgment here.

I suspect that re-enabling IncoherentInstances as a top-level pragma might work for GHC 8.0, but given that it's deprecated I think dropping support would be better.

[andreasabel]

@andreasabel I've sent an invite.

Thanks! For now, it is more convenient for me to continue on #223, but I might come back to this. (UPDATE: can work here after I force-pushed.)

If we're being strict about SemVer this deserves a major version bump since I changed the signature of CmdArgs. In practice, I don't think too many people would use CmdArgs directly, but I'll defer to your judgment here.

Yes, definitely a major version bump. I it only that shelly (like most Haskell packages) follow the Haskell PVP whether the first two numbers are major, the third is minor, and the fourth is patch version. Thus, 1.12.0 would already be a major version bump.
If lots of breakage is expected, developers might opt for even a major-major version bump, like 2.0.0.

I suspect that re-enabling IncoherentInstances as a top-level pragma might work for GHC 8.0, but given that it's deprecated I think dropping support would be better.

I think if we drop 8.0 then a simple major bump to e.g. 1.12.0 would be sufficient for sure, but it might even be sufficient if we keep 8.0 and alert of the breakage, suggesting IncoherentInstances as a workaround.

[andreasabel]

The removal of toTextArg breaks my own application of shelly here: https://github.com/BNFC/bnfc/blob/b2f673649c52cc15d4922d02c8bc23f717249df1/testing/src/ParameterizedTests.hs#L445-L447

tpBnfc :: TestParameters -> FilePath -> Sh ()
tpBnfc params grammar = run_ "bnfc" args
  where args = ["-m" <> toTextArg tpMakefile] ++ tpBnfcOptions params ++ [toTextArg grammar]

This is easily fixed, though.
Serokell search https://hackage-search.serokell.io/?q=toTextArg just gives me one relevant hit: pdf-slave-1.3.2.0 with 3 occurrences. However, this bounds shelly < 1.7 so would not be affected.

[andreasabel]

@cunningdefenestrator : Shouldn't Shelly.Pipe then be changed in the same way?

Shelly.hs/src/Shelly/Pipe.hs

Lines 612 to 615 in d0b16a6

	-- | Converter for the variadic argument version of 'run' called 'cmd'.
	class ShellArg a where toTextArg :: a -> Text
	instance ShellArg Text where toTextArg = id
	instance ShellArg FilePath where toTextArg = toTextIgnore

Also, since this is a breaking change, could you draft a migration guide, e.g. on top of the ChangeLog.md file?

[andreasabel]

Remaining:

• question about Shelly.Pipe
• migration guide
• decide fate of GHC 8.0
• decide on version 1.12 vs. 2.0 (estimate breakage)

[cunningdefenestrator]

After familiarizing myself with the Haskell PVP, this doesn't justify a major.major bump, so I dropped the version back down to 1.12.0. I've also added a note about migrating existing instances to the changelog, and I dropped support for GHC 8.0.

I'll make the corresponding change to Shelly.Pipe next.

[cunningdefenestrator]

@andreasabel
Apologies for the long turnaround, but I've made the Shelly.Pipe changes.

[andreasabel]

Thanks!
I tested this PR on my local use of shelly and wrote a more concrete migration example based on the changes I made.
Also, since the changelog is for the user of the library (not for our own bookkeeping), I removed entries that are not relevant for the users.

[andreasabel]

Candidate: https://hackage.haskell.org/package/shelly-1.12.0/candidate

Maybe toTextArgs should get a @since 1.12.0 annotation: https://hackage.haskell.org/package/shelly-1.12.0/candidate/docs/Shelly.html#v:toTextArgs

[andreasabel]

I reset this branch to remove my last 3 commits pertaining to the release of 1.12.0.