code review 4244053: http: use Header type consistently

http: use Header type consistently

[rsc, 2011-03-07 03:27:42]

Hello bradfitzgo (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[dsymonds, 2011-03-07 03:30:32]

LGTM

[bradfitz, 2011-03-07 03:54:08]

LGTM

Much nicer.

On Sun, Mar 6, 2011 at 7:27 PM, <rsc@golang.org> wrote:

> Reviewers: bradfitzgo,
>
> Message:
> Hello bradfitzgo (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> http: use Header type consistently
>
> Please review this at http://codereview.appspot.com/4244053/
>
> Affected files:
>  M src/pkg/http/client.go
>  M src/pkg/http/readrequest_test.go
>  M src/pkg/http/request.go
>  M src/pkg/http/requestwrite_test.go
>  M src/pkg/http/response.go
>  M src/pkg/http/responsewrite_test.go
>
>
> Index: src/pkg/http/client.go
> ===================================================================
> --- a/src/pkg/http/client.go
> +++ b/src/pkg/http/client.go
> @@ -117,7 +117,7 @@
>        // Headers, leaving it uninitialized.  We guarantee to the
>        // Transport that this has been initialized, though.
>        if req.Header == nil {
> -               req.Header = Header(make(map[string][]string))
> +               req.Header = make(Header)
>        }
>
>        info := req.URL.RawUserinfo
> Index: src/pkg/http/readrequest_test.go
> ===================================================================
> --- a/src/pkg/http/readrequest_test.go
> +++ b/src/pkg/http/readrequest_test.go
> @@ -93,7 +93,7 @@
>                        Proto:         "HTTP/1.1",
>                        ProtoMajor:    1,
>                        ProtoMinor:    1,
> -                       Header:        map[string][]string{},
> +                       Header:        Header{},
>                        Close:         false,
>                        ContentLength: -1,
>                        Host:          "test",
> Index: src/pkg/http/request.go
> ===================================================================
> --- a/src/pkg/http/request.go
> +++ b/src/pkg/http/request.go
> @@ -247,7 +247,7 @@
>        // from Request, and introduce Request methods along the lines of
>        // Response.{GetHeader,AddHeader} and string constants for "Host",
>        // "User-Agent" and "Referer".
> -       err = writeSortedKeyValue(w, req.Header, reqExcludeHeader)
> +       err = writeSortedHeader(w, req.Header, reqExcludeHeader)
>        if err != nil {
>                return err
>        }
> Index: src/pkg/http/requestwrite_test.go
> ===================================================================
> --- a/src/pkg/http/requestwrite_test.go
> +++ b/src/pkg/http/requestwrite_test.go
> @@ -84,7 +84,7 @@
>                        },
>                        ProtoMajor:       1,
>                        ProtoMinor:       1,
> -                       Header:           map[string][]string{},
> +                       Header:           Header{},
>                        TransferEncoding: []string{"chunked"},
>                },
>
> @@ -112,7 +112,7 @@
>                        },
>                        ProtoMajor:       1,
>                        ProtoMinor:       1,
> -                       Header:           map[string][]string{},
> +                       Header:           Header{},
>                        Close:            true,
>                        TransferEncoding: []string{"chunked"},
>                },
> Index: src/pkg/http/response.go
> ===================================================================
> --- a/src/pkg/http/response.go
> +++ b/src/pkg/http/response.go
> @@ -67,10 +67,9 @@
>        // ReadResponse nor Response.Write ever closes a connection.
>        Close bool
>
> -       // Trailer maps trailer keys to values.  Like for Header, if the
> -       // response has multiple trailer lines with the same key, they will
> be
> -       // concatenated, delimited by commas.
> -       Trailer map[string][]string
> +       // Trailer maps trailer keys to values, in the same
> +       // format as the header.
> +       Trailer Header
>  }
>
>  // ReadResponse reads and returns an HTTP response from r.  The
> RequestMethod
> @@ -193,7 +192,7 @@
>        }
>
>        // Rest of header
> -       err = writeSortedKeyValue(w, resp.Header, respExcludeHeader)
> +       err = writeSortedHeader(w, resp.Header, respExcludeHeader)
>        if err != nil {
>                return err
>        }
> @@ -215,16 +214,16 @@
>        return nil
>  }
>
> -func writeSortedKeyValue(w io.Writer, kvm map[string][]string, exclude
> map[string]bool) os.Error {
> -       keys := make([]string, 0, len(kvm))
> -       for k := range kvm {
> +func writeSortedHeader(w io.Writer, h Header, exclude map[string]bool)
> os.Error {
> +       keys := make([]string, 0, len(h))
> +       for k := range h {
>                if !exclude[k] {
>                        keys = append(keys, k)
>                }
>        }
>        sort.SortStrings(keys)
>        for _, k := range keys {
> -               for _, v := range kvm[k] {
> +               for _, v := range h[k] {
>                        if _, err := fmt.Fprintf(w, "%s: %s\r\n", k, v); err
> != nil {
>                                return err
>                        }
> Index: src/pkg/http/responsewrite_test.go
> ===================================================================
> --- a/src/pkg/http/responsewrite_test.go
> +++ b/src/pkg/http/responsewrite_test.go
> @@ -22,7 +22,7 @@
>                        ProtoMajor:    1,
>                        ProtoMinor:    0,
>                        RequestMethod: "GET",
> -                       Header:        map[string][]string{},
> +                       Header:        Header{},
>                        Body:
>  nopCloser{bytes.NewBufferString("abcdef")},
>                        ContentLength: 6,
>                },
> @@ -38,7 +38,7 @@
>                        ProtoMajor:    1,
>                        ProtoMinor:    0,
>                        RequestMethod: "GET",
> -                       Header:        map[string][]string{},
> +                       Header:        Header{},
>                        Body:
>  nopCloser{bytes.NewBufferString("abcdef")},
>                        ContentLength: -1,
>                },
> @@ -53,7 +53,7 @@
>                        ProtoMajor:       1,
>                        ProtoMinor:       1,
>                        RequestMethod:    "GET",
> -                       Header:           map[string][]string{},
> +                       Header:           Header{},
>                        Body:
> nopCloser{bytes.NewBufferString("abcdef")},
>                        ContentLength:    6,
>                        TransferEncoding: []string{"chunked"},
>
>
>

[rsc, 2011-03-07 04:02:32]

*** Submitted as http://code.google.com/p/go/source/detail?r=4cf2dfa78382 ***

http: use Header type consistently

R=bradfitzgo, dsymonds
CC=golang-dev
http://codereview.appspot.com/4244053