Skip to content

Conversation

ori-sky
Copy link

@ori-sky ori-sky commented Jan 28, 2016

This patch replaces the two specialized instances of PathInfo for [String] and [Text] with an instance for [a] for all types a. The generalized instance has identical behaviour for [String] and [Text] to the removed instances.

For example, in combination with the web-routes-th package, the following data type is possible:

data Route = Search
           | ViewSubTree [Integer]
           | NewLeaf [Integer] String

This would match for the following URLs:

/search
/view-sub-tree/<Integer>/<Integer>/..etc..
/new-leaf/<Integer>/<Integer>/..etc../<String>

This patch replaces the two specialized instances of PathInfo for
[String] and [Text] with an instance for [a] for all types a. The
generalized instance has identical behaviour for [String] and [Text] to
the removed instances.

For example, in combination with the web-routes-th package, the
following data type is possible:

  data Route = Search
             | ViewSubTree [Integer]
             | NewLeaf [Integer] String

This would match for the following URLs:

  /search
  /view-sub-tree/<Integer>/<Integer>/..etc..
  /new-leaf/<Integer>/<Integer>/..etc../<String>
@stepcut
Copy link
Member

stepcut commented Jan 29, 2016

The generalized instance permits nested lists -- but nested lists are problematic because they are ambiguous. As implemented, the result is currently a runtime parse error:

*Web.Routes.PathInfo> fromPathInfo (Data.Text.Encoding.encodeUtf8  $ toPathInfo [[1,2,3::Int],[4,5,6::Int]]) :: Either String [[Int]]
*** Exception: Text.ParserCombinators.Parsec.Prim.many: combinator 'many' is applied to a parser that accepts an empty string.

If we think more abstractly about the problem, if we have the sequence [[1,2,3],[4,5,6]] and serialize that to /1/2/3/4/5/6 we have lost important structural information. [[1,2,3,4],[5,6]] also serializes to /1/2/3/4/5/6. This means we can not uniquely parse URL back to a unique value.

Accordingly, I am reluctant to pull this patch.

I should note that it could be argued that the existing instances are also bogus since they would cause similar problems with:

data Route = Foo [Text] [Text]

@ori-sky
Copy link
Author

ori-sky commented Jan 30, 2016

Ah, I understand the problem. Whilst I think the ability to parse and match on URLs of that kind of format is still a desirable thing, I can't really think of any way to solve the problem you've outlined.

I should note that I actually did notice a similar problem to the case you highlighted with [Text], with my implementation of NewLeaf [Integer] String, as the String cannot be anything that matches Integer like for example /new-leaf/1/2/53, which will parse to [1, 2, 53] for the [Integer] portion.

Might a plausible alternative to keep structural information be to represent lists as a list of comma-separated values or something similar? For example:

/new-leaf/1,2,53/hello

would equate to:

NewLeaf [1, 2, 53] "hello"

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.

2 participants