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 non-throwing indexMaybe function (continues #146) #261

Merged
merged 11 commits into from
Aug 23, 2020

Conversation

cole-miller
Copy link
Contributor

This is a continuation of @FintanH's work in #146. I've rebased his commits onto the current master.

Data/ByteString.hs Outdated Show resolved Hide resolved
@FintanH
Copy link
Contributor

FintanH commented Aug 21, 2020

Thanks for bringing it to the finish line ❤️

@Bodigrim
Copy link
Contributor

W. r. t. 02c54aa - please retain (!?). It is used as an operator for safe lookups in many packages: containers, vector, massiv, ral, relude, just to name a few. It is actually indexMaybe which is not featured by anyone else (except text-short).

@cole-miller
Copy link
Contributor Author

I think I've implemented most of the outstanding comments from the previous review; the things I'm still unclear about are:

  • Is there a preferred alternative to this definition?
indexMaybe = (fmap w2c .) . B.indexMaybe
  • Should ($) be strict in all versions of the indexMaybe definition, or only in those for the "strict" modules? (This is probably a dumb question, sorry.)

@cole-miller cole-miller marked this pull request as ready for review August 21, 2020 23:14
@Bodigrim
Copy link
Contributor

Is there a preferred alternative to this definition?

Let it be as is, it is potentially marginally better for inlining.

Should ($) be strict in all versions [...]?

Yes, make it strict in all versions.

@Bodigrim Bodigrim added this to the 0.11.0.0 milestone Aug 21, 2020
@Bodigrim Bodigrim merged commit f35e9e5 into haskell:master Aug 23, 2020
@cole-miller
Copy link
Contributor Author

Thanks @Bodigrim @fumieval @sjakobi for your guidance, and @FintanH for doing most of the work :)

@cole-miller cole-miller deleted the feature/bytestring branch August 23, 2020 19:56
@Bodigrim Bodigrim linked an issue Aug 24, 2020 that may be closed by this pull request
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.

Safe indexing functions
5 participants