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

Eliom_parameter.sum is broken on Eliom 4.1 #182

Closed
darioteixeira opened this issue Jul 6, 2015 · 7 comments
Closed

Eliom_parameter.sum is broken on Eliom 4.1 #182

darioteixeira opened this issue Jul 6, 2015 · 7 comments

Comments

@darioteixeira
Copy link
Contributor

It seems that Eliom_parameter.sum does not work with Eliom 4.1 (the server returns a 500 error and always complains about wrong parameters). I don't know exactly when this regression was introduced, but IIRC this feature worked fine on Eliom 3. Here is a minimal example that illustrates the problem:

open Eliom_content.Html5.F
open Eliom_parameter

let coucou_handler sum () =
    let msg = match sum with Inj1 s -> s | Inj2 i -> string_of_int i in
    Lwt.return
        (html
            (head (title (pcdata "Coucou")) [])
            (body [p [pcdata "You sent: "; pcdata msg]]))

let coucou_service =
    Eliom_registration.Html5.register_service
        ~path:["coucou"]
        ~get_params:Eliom_parameter.(sum (string "s") (int "i"))
        coucou_handler

let main_handler () () =
    Lwt.return
        (html
            (head (title (pcdata "Main")) [])
            (body
                [
                h1 [pcdata "Links for coucou:"];
                p [a coucou_service [pcdata "Inj1 (string)"] (Inj1 "testing")];
                p [a coucou_service [pcdata "Inj2 (int)"] (Inj2 123)];
                ]))

let main_service =
    Eliom_registration.Html5.register_service
        ~path:[]
        ~get_params:Eliom_parameter.unit
        main_handler
@Drup
Copy link
Member

Drup commented Jul 6, 2015

I can reproduce, but I really don't know what's going on.

@darioteixeira
Copy link
Contributor Author

PR #114 seems to be the most recent modification to the code that parses sum parameters. Perhaps @hhugo or @balat know what's going on?

@hhugo
Copy link
Member

hhugo commented Jul 6, 2015

looking quickly at the code, I wonder if

let v,l = (List.assoc_remove (pref^name^suff) params) in
should go inside the try-with

@vasilisp
Copy link
Contributor

vasilisp commented Jul 7, 2015

I tested @hhugo's hypothesis, and it seems to fix the issue. List.assoc_remove throws Not_found for one of the two cases, and we do not produce Errors_ (as should be the case), but rather the exception bubbles up.

@vasilisp vasilisp assigned vasilisp and unassigned vasilisp Jul 8, 2015
@vbmithr
Copy link
Member

vbmithr commented Jul 21, 2015

@Drup @balat Once this is fixed, we will release a new eliom version.

@balat
Copy link
Member

balat commented Jul 21, 2015

@hhugo Do you remember why you did this? 930fdbc

@balat
Copy link
Member

balat commented Jul 21, 2015

Ok I found. It's related to #45

balat added a commit that referenced this issue Jul 21, 2015
@balat balat closed this as completed in fb903d3 Jul 21, 2015
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

No branches or pull requests

6 participants