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

Add OptionsHandler to TreeMux & node.setHandler #19

Closed
wants to merge 1 commit into from

Conversation

derekperkins
Copy link
Contributor

No description provided.

@derekperkins
Copy link
Contributor Author

I squashed my commits from #18 and it closed the PR, not quite sure what happened. Here it is again as 1 commit. :)

@dimfeld
Copy link
Owner

dimfeld commented Dec 8, 2015

I think this will make it impossible to set a different OPTIONS handler on any node, and it will also cause a difference in behavior if t.OptionsHandler is set before or after a route is added. Instead of setting this in the tree nodes, how about we just handle this in the router code after it has found a node. I'm thinking something like this, at router.go:311

if !ok {
    if r.Method == "HEAD" && t.HeadCanUseGet {
        handler, ok = n.leafHandler["GET"]
    } else if r.Method == "OPTIONS" && t.OptionsHandler != nil {
        handler = t.OptionsHandler
        ok = true
    }

    if !ok {
        t.MethodNotAllowedHandler(w, r, n.leafHandler)
        return
   }
}

@derekperkins
Copy link
Contributor Author

With my branch, you'd be able set a custom OPTIONS route first, but you're right, that'd be unexpected behavior. I like your solution much better.

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.

None yet

2 participants