Skip to content

Conversation

@alexfmpe
Copy link
Contributor

@alexfmpe alexfmpe commented May 15, 2024

Ran into a non-urls usecase where I wanted to use ([Text], ByteString) instead of ([Text], Map Text (Maybe Text)) and realized the groundwork for #989 allowed me to use ([Text], q) as long as I relaxed the type signatures so I pulled that bit into this separate, more conservative PR.

The tutorial describes the PageName version of pathComponentEncoder. Should that be updated with this PR or do we keep it simple?

I have:

  • Based work on latest develop branch
  • Followed the contribution guide
  • Looked for lint in my changes with hlint . (lint found code you did not write can be left alone)
  • Run the test suite: $(nix-build -A selftest --no-out-link)
  • Updated the changelog
  • (Optional) Run CI tests locally: nix-build release.nix -A build.x86_64-linux --no-out-link (or x86_64-darwin on macOS)

-- various cases when encoding a dependent sum of type `(R p)`.
data SegmentResult check parse a =
PathEnd (Encoder check parse a (Map Text (Maybe Text)))
data SegmentResult check parse a b =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Breaking change - migration while keeping old meaning is to use (Map Text (Maybe Text)) for b, but sometimes this can be generalized, as with indexOnlyRouteSegment.

Do we want to do a proper warn-first-break-later migration cycle at the cost of polluting the namespace with a SegmentResult' or so for a few releases?

@alexfmpe
Copy link
Contributor Author

alexfmpe commented May 15, 2024

Ended up also locally defining a variant of queryOnlyEncoder with the more general type.

I wonder if this should be more of a generalise-query-type thing.

In theory we could do the same for path, so instead of PageName everywhere, unless url-specific we'd instead have ([fragment], query) but I don't actually have a use-case for that so hard to tell what exactly to generalise to. What even is a generic path afterall?

At least allowing () locally as the query type seems pretty convenient and common

@alexfmpe alexfmpe force-pushed the generalise-segment-result branch from dc7a7a9 to 64db94f Compare November 14, 2024 23:53
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.

1 participant