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

x/website, x/pkgsite: net/http.ServeMux.Handle example reads poorly if shown as a complete program #40103

Open
readpe opened this issue Jul 7, 2020 · 1 comment

Comments

@readpe
Copy link

@readpe readpe commented Jul 7, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14.4 darwin/amd64

Does this issue reproduce with the latest release?

In current package documentation at https://golang.org/pkg/net/http/#example_ServeMux_Handle

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOOS="darwin"

What did you expect to see?

Expected example to initiate http.ListenAndServe or equivalent with new ServeMux

package main

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

type apiHandler struct{}

func (apiHandler) ServeHTTP(http.ResponseWriter, *http.Request) {}

func main() {
	mux := http.NewServeMux()
	mux.Handle("/api/", apiHandler{})
	mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
		// The "/" pattern matches everything, so we need to check
		// that we're at the root here.
		if req.URL.Path != "/" {
			http.NotFound(w, req)
			return
		}
		fmt.Fprintf(w, "Welcome to the home page!")
	})
	log.Fatal(http.ListenAndServe(":8080", mux))
}

What did you see instead?

The example immediately exits without serving

package main

import (
	"fmt"
	"net/http"
)

type apiHandler struct{}

func (apiHandler) ServeHTTP(http.ResponseWriter, *http.Request) {}

func main() {
	mux := http.NewServeMux()
	mux.Handle("/api/", apiHandler{})
	mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
		// The "/" pattern matches everything, so we need to check
		// that we're at the root here.
		if req.URL.Path != "/" {
			http.NotFound(w, req)
			return
		}
		fmt.Fprintf(w, "Welcome to the home page!")
	})
}

I'd be happy to work on this for my first contribution if accepted

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Jul 11, 2020

Thanks for reporting. I agree the example, as displayed at https://golang.org/pkg/net/http/#example_ServeMux_Handle, can feel like it's missing some code to start the server.

However, the example as written in code (see here), and displayed on https://pkg.go.dev/net/http@go1.14.4#example-ServeMux.Handle at this time seems a lot more reasonable to omit starting up a server, since it's meant to show how to use the ServeMux.Handle method:

mux := http.NewServeMux()
mux.Handle("/api/", apiHandler{})
mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
	// The "/" pattern matches everything, so we need to check
	// that we're at the root here.
	if req.URL.Path != "/" {
		http.NotFound(w, req)
		return
	}
	fmt.Fprintf(w, "Welcome to the home page!")
})

So, we should decide whether to do given it's displayed differently on those websites, and take it into account if examples are displayed differently in the future.

Given that the example doesn't have an // Output: comment, it doesn't seem like it should be displayed as a complete program, so the current pkg.go.dev (and godoc.org, see https://godoc.org/net/http#example-ServeMux-Handle) version seems better.

/cc @julieqiu @jba @shaqque

@dmitshur dmitshur added this to the Backlog milestone Jul 11, 2020
@dmitshur dmitshur changed the title net/http: add http.ListenAndServe to ServeMux.Handle example godoc: net/http.ServeMux.Handle example reads poorly if shown as a complete program Jul 11, 2020
@dmitshur dmitshur changed the title godoc: net/http.ServeMux.Handle example reads poorly if shown as a complete program x/website, x/pkgsite: net/http.ServeMux.Handle example reads poorly if shown as a complete program Jul 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.