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

Docs: minor error with Routing #101

Closed
jasiozet opened this issue Jan 1, 2023 · 3 comments
Closed

Docs: minor error with Routing #101

jasiozet opened this issue Jan 1, 2023 · 3 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@jasiozet
Copy link

jasiozet commented Jan 1, 2023

Hello, I've noticed a small issue here:
https://github.com/pimbrouwers/Falco/blob/master/doc/routing.md

open Falco
open Falco.Routing
open Falco.HostBuilder

webHost [||] {
    endpoints [
        get "/hello/{name:alpha}" (fun ctx ->
            let route = Request.getRoute ctx
            let name = route.GetString "name" "" // <- This Doesn't compile for me
            let message = sprintf "Hello %s" name
            Response.ofPlainText message ctx)
    ]
}

However this is completely fine:

webHost [||] {
    endpoints [
        get "/hello/{name:alpha}" (fun ctx ->
            let route = Request.getRoute ctx
            let name = route.GetString ("name", "") // Change to tupled parameters made here
            let message = sprintf "Hello %s" name
            Response.ofPlainText message ctx)
    ]
}

The route binding example from here: https://www.falcoframework.com/docs/request.html#route-binding
Is completely fine and uses a simplified version, like this:

open Falco

// Assuming a route pattern of /{Name}
let manualRouteHandler : HttpHandler = fun ctx ->
    let r = Request.getRoute ctx
    let name = r.GetString "Name"
    Response.ofPlainText name ctx

let mapRouteHandler : HttpHandler =
    Request.mapRoute (fun r ->
        r.GetString "Name")
        Response.ofPlainText

Which would translate our first example to something like this:

webHost [||] {
    endpoints [
        get "/hello/{name:alpha}" (fun ctx ->
            let route = Request.getRoute ctx
            let name = route.GetString "name"
            let message = sprintf "Hello %s" name
            Response.ofPlainText message ctx)
    ]
}

That would be one of my suggestion the other is using a working tupled example in a more descriptive way:

webHost [||] {
    endpoints [
        get "/hello/{name:alpha}" (fun ctx ->
            let route = Request.getRoute ctx
            let name = route.GetString ("name", "FalcoEnthusiast") // Could be "defaultValue", "fallbackValue"
            let message = sprintf "Hello %s" name
            Response.ofPlainText message ctx)
    ]
}

But my question is will it ever be an empty string here? Maybe the fallback value is never needed in this example?

I would be very happy to make a PR for that, but it is also a trivial change so might be an overkill.
Have a great New Year Pim and everyone! :)

@pimbrouwers pimbrouwers self-assigned this Jan 2, 2023
@pimbrouwers pimbrouwers added the documentation Improvements or additions to documentation label Jan 2, 2023
pimbrouwers added a commit that referenced this issue Jan 2, 2023
@pimbrouwers
Copy link
Owner

Thanks for finding this. It's a relic of the old-style API. I have updated the routing.md document.

@jasiozet
Copy link
Author

jasiozet commented Jan 2, 2023

Thank you! :)

@pimbrouwers
Copy link
Owner

pimbrouwers commented Jan 2, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants