Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prisms for working with prefixed and suffixed text #982

Merged
merged 7 commits into from
Nov 7, 2021

Conversation

Taneb
Copy link
Collaborator

@Taneb Taneb commented Sep 10, 2021

Closes #981

@RyanGlScott
Copy link
Collaborator

@Taneb, is this something you're planning to work on soon? I'm interested in landing this before the next lens release. If you don't have time, then no worries—I can pick it up (assuming your branch gives the appropriate permissions to lens contributors).

@Taneb
Copy link
Collaborator Author

Taneb commented Nov 1, 2021

I'd forgotten about it, being honest. My main issue was I couldn't figure out how to run the doctests locally, so I'd have had to do a lot of noisy trial and error to get things passing. Other than that I'm happy to work on it. I think the branch is on this repo so anyone with permission for ekmett/lens should be able to push to it

@RyanGlScott
Copy link
Collaborator

My apologies, I didn't realize you were blocked. Here is my workflow for running the doctests:

  1. Build the library with cabal build.
  2. Download cabal-docspec from https://github.com/phadej/cabal-extras/releases/download/cabal-docspec-0.0.0.20210111/cabal-docspec-0.0.0.20210111.xz
  3. Run cabal-docspec in the top-level lens checkout directory. (Make sure the same GHC version you used in step (1) is on your PATH.)

If that doesn't work, feel free to shout.

@RyanGlScott
Copy link
Collaborator

I pushed a commit which fixes the build errors on old GHCs. Or so I thought, at least. There's a separate issue on GHC 7.8.4:

[29 of 86] Compiling Control.Lens.Prism ( src/Control/Lens/Prism.hs, /__w/lens/lens/dist-newstyle/build/x86_64-linux/ghc-7.8.4/lens-5.1/build/Control/Lens/Prism.o )

src/Control/Lens/Prism.hs:397:31:
    Not in scope: ‘BS.stripPrefix’
    Perhaps you meant one of these:
      ‘TS.stripPrefix’ (imported from Data.Text),
      ‘TL.stripPrefix’ (imported from Data.Text.Lazy),
      ‘TS.stripSuffix’ (imported from Data.Text)

src/Control/Lens/Prism.hs:401:31:
    Not in scope: ‘BL.stripPrefix’
    Perhaps you meant one of these:
      ‘TL.stripPrefix’ (imported from Data.Text.Lazy),
      ‘TS.stripPrefix’ (imported from Data.Text),
      ‘TL.stripSuffix’ (imported from Data.Text.Lazy)

src/Control/Lens/Prism.hs:431:33:
    Not in scope: ‘BS.stripSuffix’
    Perhaps you meant one of these:
      ‘TS.stripSuffix’ (imported from Data.Text),
      ‘TL.stripSuffix’ (imported from Data.Text.Lazy),
      ‘List.stripSuffix’ (imported from Data.List.Lens)

src/Control/Lens/Prism.hs:435:33:
    Not in scope: ‘BL.stripSuffix’
    Perhaps you meant one of these:
      ‘TL.stripSuffix’ (imported from Data.Text.Lazy),
      ‘TS.stripSuffix’ (imported from Data.Text),
      ‘List.stripSuffix’ (imported from Data.List.Lens)

stripPrefix was introduced in bytestring-0.10.8.0, and unfortunately, GHC 7.8.4 comes bundled with an older version of bytestring. This doesn't seem easy to work around unless we backport the entire implementation of stripSuffix, which doesn't sound fun. Perhaps we should land #977 first and rebase on top of that.

Copy link
Collaborator

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI finally passes. Hooray!

src/Data/List/Lens.hs Outdated Show resolved Hide resolved
CHANGELOG.markdown Outdated Show resolved Hide resolved
@RyanGlScott
Copy link
Collaborator

I've pushed some commits which add more Haddocks, more changelog descriptions, and makes things more backwards-compatible. I think that takes care of the remaining review suggestions, so unless anyone notices something I've overlooked, I'll plan to merge this within a day or so.

@RyanGlScott RyanGlScott mentioned this pull request Nov 6, 2021
@RyanGlScott RyanGlScott merged commit 91ced56 into master Nov 7, 2021
@RyanGlScott RyanGlScott deleted the text-fixed-lenses branch November 7, 2021 11:29
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.

prefixed and suffixed prisms for Text
3 participants