Adding in a check for routes with just /

[ShaneSaww]

This should fix #79.
Looking for feedback!

[elithrar]

Thanks for submitting this. Will review tomorrow!

[ShaneSaww]

If it helps this is the code I used to test:

package main

import (
	"fmt"
	"log"
	"net/http"

	"github.com/gorilla/mux"
)

func main() {
	router := mux.NewRouter()
	adminRouter := router.PathPrefix("/admin").Subrouter()
	adminRouter.HandleFunc("", someHandler)
	adminRouter.HandleFunc("/", someHandler)

	log.Print("Listening on :5000")
	httpErr := http.ListenAndServe(":5000", router)
	if httpErr != nil {
		log.Fatal("ListenAndServe:", httpErr)
	}
}

func someHandler(w http.ResponseWriter, r *http.Request) {
	fmt.Printf("In the handler\n")
}

Running this with my fix should allow you to hit both
localhost:5000/admin
localhost:5000/admin/

[elithrar]

Can you add some additional test cases to this?

1. /foo/bar
2. /foo/bar/
3. /{category}
4. /{category}/

This will help prevent any regressions in future. mux has grown a lot over time & I want to make sure the testing story is as robust as it can be.

[ShaneSaww]

@elithrar Just added the extra test cases!

[elithrar]

LGTM - @kisielk?

[kisielk]

LGTM as well. Thanks!

[scisci]

The code added doesn't seem to make any sense:

if tpl == "/" && (len(tpl) == 0 || tpl[0] != '/') {

Wouldn't that always evaluate to false?

I would think you would want to say:
if len(tpl) > 0 && tpl[0] != '/' {

[ShaneSaww]

@scisci the reason that it makes sense (and it's kinda strange) is that if

tpl = "/" then tpl[0] == 47

which is why if tpl == "/" && (len(tpl) == 0 || tpl[0] != '/') { works.

With all that in mind you are correct that if len(tpl) > 0 && tpl[0] != '/' { would make more sense.

[scisci]

OK, guess I'm missing something :)

It looked to me like this code would always return false, so it wouldn't catch truly bad templates that don't start with a slash like router.HandleFunc("someNonEmptyRouteWithNoSlash", ...)

But I'm new to gorilla so maybe this is on purpose or maybe I'm wrong.

[kisielk]

I think I agree with @scisci ... looking at it again I can't seem to make this logic make sense in my head, unless I am missing something.

[ShaneSaww]

After playing with it in the playground it's broken.
https://play.golang.org/p/nQ3jNgUA-P
I can push up a fix for it later.