Skip to content

Conversation

@hseg
Copy link

@hseg hseg commented Jun 22, 2025

#99 attempted to fix multiline tuple patterns, but in the process broke some test. In fact, it turns out tuple patterns were underexercised in tests, so we were lucky to catch this (it also broke the handling of prefix operator definitions). Moreover, the logic in that PR was hard to understand in places.
This PR fixes this by replacing the tuple-splitting logic by a simple unfold splitting on delimiters.
Also, to silence GHC warnings, it introduces two prisms _TokenName and _NewLine to use instead of eg map (\(NewLine x ...) -> x) . filter isNewLine.

Closes: #101
Fixes: #99

hseg added 3 commits June 22, 2025 17:44
Instead of
map (\(NewLine x ...) -> x) . filter isNewLine
introduce prisms _TokenName and _NewLine and write
mapMaybe _NewLine
(also, reuse trimNewlines where relevant)
PR MarcWeber#99 attempted to fix the handling of multiple definitions by way of
tuple unpackings -- eg
(x,y,z) = ... and
(x,
 y) = undefined

Unfortunately, this was an edge case underexercised in the test suite,
so the bugs in MarcWeber#99 weren't caught -- we were lucky it also broke the
handling of operators, so 9.hs caught it.

Moreover, the comma splitting code was impenetrably obscure, rewrote it
as a simple unfold.

Closes: MarcWeber#101
Fixes: MarcWeber#99
- Handle inexhaustive pattern matches:
  - tail -> drop 1
  - map head . groupBy -> map D.L.NonEmpty.head . D.L.NonEmpty.groupBy
  - Replace unnecessarily partial conditions by function guards, list
    comprehensions, etc
- Removed unnecessary imports
@andreasabel
Copy link
Contributor

CI fails for 8.0 and 8.2 because of the Semigroup incompatibility.
I think these versions should simply be dropped from CI, rather than fixing the imports in the code.
To drop, remove them from tested-with in the .cabal file, and then run haskell-ci regenerate or just delete them from .github/workflows/haskell-ci.yml.

As recommended by @andreasabel, supporting those versions means needing
to bridge the "Semigroup as superclass of Monoid" proposal.
Unfortunately, pushing this commit stalled long enough that the CI logs
were wiped, so it is hard to assess the cost of supporting those old
versions.
@hseg
Copy link
Author

hseg commented Oct 22, 2025

In cleaning up my outstanding bugs list, I noticed I hadn't addressed this. Apologies. Unfortunately, enough time has elapsed that the CI logs have been wiped, so I can't assess how hard it'd be to bridge the Semigroup/Monoid change. Moreover, haskell-ci doesn't like some of the GHC versions configured (9.6.7, 9.8.4, 9.10.2, 9.12.2) so I couldn't do this automatically. Hopefully my manual patch is OK.

@andreasabel
Copy link
Contributor

Moreover, haskell-ci doesn't like some of the GHC versions configured (9.6.7, 9.8.4, 9.10.2, 9.12.2)

You need to install haskell-ci from its github repo, the released version on hackage is always way behind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Top level patterns change broke testsuite

2 participants