code review 6847074: net/http: mention ParseForm and Form in docs for (*Requ... (issue 6847074)

[minu...@gmail.com]

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golan...@googlegroups.com (cc: golan...@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go


Description:
net/http: mention ParseForm and Form in docs for (*Request).FormValue

Please review this at http://codereview.appspot.com/6847074/

Affected files:
M src/pkg/net/http/request.go


Index: src/pkg/net/http/request.go
===================================================================
--- a/src/pkg/net/http/request.go
+++ b/src/pkg/net/http/request.go
@@ -728,6 +728,8 @@
// FormValue returns the first value for the named component of the query.
// POST and PUT body parameters take precedence over URL query string
values.
// FormValue calls ParseMultipartForm and ParseForm if necessary.
+// If you want multiple values of the same key, invoke r.ParseForm() and
+// use r.Form directly.
func (r *Request) FormValue(key string) string {
if r.Form == nil {
r.ParseMultipartForm(defaultMaxMemory)

[Brad Fitzpatrick]

Avoid the word "you".  Notice that the current docs never use it: http://tip.golang.org/pkg/net/http/

[minu...@gmail.com]

Hello golan...@googlegroups.com, brad...@golang.org (cc:
golan...@googlegroups.com),

Please take another look.


http://codereview.appspot.com/6847074/

[brad...@golang.org]

https://codereview.appspot.com/6847074/diff/7001/src/pkg/net/http/request.go
File src/pkg/net/http/request.go (right):

https://codereview.appspot.com/6847074/diff/7001/src/pkg/net/http/request.go#newcode731
src/pkg/net/http/request.go:731: // If multiple values of the same key
is needed, invoke r.ParseForm() and
s/is/are/ and drop the parens on ParseForm. Actually, I'd drop
everything from "and ..." and move that to the docs of ParseForm where
it's currently missing.

So this is just:

// If multiple values of the same key are needed, use ParseForm.

Or, dropping the passive voice:

// To access multiple values of the same key use ParseForm.

I'm not the wordsmith, though. That should be closer, though. Rob or
Russ might suggest further changes.

https://codereview.appspot.com/6847074/

[minu...@gmail.com]

PTAL.

https://codereview.appspot.com/6847074/

[brad...@golang.org]

https://codereview.appspot.com/6847074/diff/11001/src/pkg/net/http/request.go
File src/pkg/net/http/request.go (right):

https://codereview.appspot.com/6847074/diff/11001/src/pkg/net/http/request.go#newcode653
src/pkg/net/http/request.go:653: // After calling this, use Form field
of r to access multiple values of the
Also mention PostForm. I would just tell them what the function does
and not explain how to solve every problem.

They can learn read the definition of r.Form and see that it's
multi-valued, so need to say "to access multiple values". Just describe
what is populated in what cases.

https://codereview.appspot.com/6847074/diff/11001/src/pkg/net/http/request.go#newcode658
src/pkg/net/http/request.go:658: func (r *Request) ParseForm() (err
error) {
can you fix this return value stutter while you're at it? Just "error",
not "(err error)".

https://codereview.appspot.com/6847074/