pointfreeco / swift-parsing

A library for turning nebulous data into well-structured data, with a focus on composition, performance, generality, and ergonomics.
https://www.pointfree.co/collections/parsing
MIT License
864 stars 77 forks source link

Case key path support #323

Open stephencelis opened 1 year ago

stephencelis commented 1 year ago

Adds support for case key paths. While parsers do not seem benefit from some features of case key paths (they can't be abbreviated), they can at least be used more performantly without the reflection associated with the old style of case paths.

ShivaHuang commented 5 months ago

Hi @stephencelis,

Would you complete this PR? Recently I wrote some code exactly the same with this one.

I found it helps to write more clear code.

Lets say we have the following route enum:

@CasePathable
enum Route: Equatable {
    case home
    case user(User.ID)
}

Before, we need to write this way:

OneOf {
    Route(.case(Route.home)) {
        Path { "home" }
    }
    Route(.case(Route.user)) {
        Path { "user"; UUID.parser() }
    }
}

and with the change in this PR, we can write:

OneOf(output: Route.self) {
    Route(.case(\.home)) {
        Path { "home" }
    }
    Route(.case(\.user)) {
        Path { "user"; UUID.parser() }
    }
}

and the auto-completion is working great with case key path, so I think it's worth to do the improvements.

stephencelis commented 5 months ago

@ShivaHuang Part of the reason we haven't pushed through this is that case key paths can't be inferred the way you want. The root Enum type is not known in the builder, and so you'd have to do this:

 OneOf(output: MyRoute.self) {
-    Route(.case(\.home)) {
+    Route(.case(\MyRoute.Cases.home)) {
         Path { "home" }
     }
-    Route(.case(\.user)) {
+    Route(.case(\MyRoute.Cases.user)) {
         Path { "user"; UUID.parser() }
     }
 }

This isn't to say we shouldn't eventually land this PR, but we haven't prioritized it since it isn't an ergonomic improvement, and in fact is a bit more verbose than it was previously due to the need of .Cases.

Edit: Ah, catching up you're relying on the Output change for OneOf. That works, but it's a bummer that it would be limited to OneOf builders, and may confuse folks. We'll keep thinking about it, though! Thanks for the reminder!