swift-parsing icon indicating copy to clipboard operation
swift-parsing copied to clipboard

Case key path support

Open stephencelis opened this issue 2 years ago • 2 comments

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.

stephencelis avatar Nov 09 '23 04:11 stephencelis

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.

ShivaHuang avatar Jun 12 '24 03:06 ShivaHuang

@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!

stephencelis avatar Jun 12 '24 15:06 stephencelis

Closing in favor of #370.

stephencelis avatar Jan 16 '25 23:01 stephencelis