code review 6680044: cmd/go: add list template function. (issue 6680044)

41 views
Skip to first unread message

rogp...@gmail.com

unread,
Oct 13, 2012, 8:11:50 AM10/13/12
to r...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: rsc,

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

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


Description:
cmd/go: add list template function.

It's common to use the go list command in shell scripts, but
currently it's awkward to print a string slice from the Package
type in a way that's easily parseable by the shell. For example:

go list -f '{{range .Deps}}{{.}}
{{end}}'

(and even that prints an unwanted new line at the end|).

As a possibility for making this easier, this CL adds a "list"
function to the format template, to print items separated by new lines.
For example:

go list -f '{{list .Deps}}'

This CL is mainly for discussion. There are plenty of other ways
of doing a similar thing, of course.

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

Affected files:
M src/cmd/go/list.go


Index: src/cmd/go/list.go
===================================================================
--- a/src/cmd/go/list.go
+++ b/src/cmd/go/list.go
@@ -9,6 +9,7 @@
"encoding/json"
"io"
"os"
+ "strings"
"text/template"
)

@@ -26,8 +27,10 @@

The -f flag specifies an alternate format for the list,
using the syntax of package template. The default output
-is equivalent to -f '{{.ImportPath}}'. The struct
-being passed to the template is:
+is equivalent to -f '{{.ImportPath}}'. One extra template function
+is available, "list", which takes a slice of strings and returns
+a single string with all the slice elements separated by new lines.
+The struct being passed to the template is:

type Package struct {
Dir string // directory containing package sources
@@ -113,7 +116,8 @@
out.Write(nl)
}
} else {
- tmpl, err := template.New("main").Parse(*listFmt)
+ list := func(x []string) string { return strings.Join(x, "\n") }
+ tmpl, err := template.New("main").Funcs(template.FuncMap{"list":
list}).Parse(*listFmt)
if err != nil {
fatalf("%s", err)
}


David Symonds

unread,
Oct 13, 2012, 8:23:37 AM10/13/12
to re...@codereview-hr.appspotmail.com, roger peppe, golan...@googlegroups.com, r...@golang.org

A join function would be better.

minu...@gmail.com

unread,
Oct 13, 2012, 8:27:00 AM10/13/12
to rogp...@gmail.com, r...@golang.org, dsym...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/6680044/diff/5001/src/cmd/go/list.go
File src/cmd/go/list.go (right):

https://codereview.appspot.com/6680044/diff/5001/src/cmd/go/list.go#newcode130
src/cmd/go/list.go:130: if out.Count() > 0 {
i still prefer to fix the extra \n problem at least.
(I don't oppose to the added list function)

i think the reason why cmd/go will add a \n unconditionally
is to avoid miss one at the end,
but the logic here can be changed to provide extra \n
only when really necessary.

https://codereview.appspot.com/6680044/diff/5001/src/cmd/go/list.go#newcode159
src/cmd/go/list.go:159: func (cw *CountingWriter) Write(p []byte) (n
int, err error) {
for example, we only need to look at p[len(p)-1],
and if it is '\n' set count = 0; otherwise set count=1;
no other code need to be changed to remove the
extra \n.

(if we choose to go this route, CountingWriter should be
renamed)

https://codereview.appspot.com/6680044/

Rémy Oudompheng

unread,
Oct 13, 2012, 8:34:31 AM10/13/12
to David Symonds, re...@codereview-hr.appspotmail.com, roger peppe, golan...@googlegroups.com, r...@golang.org
I also prefer join. Especially if I can write {{ .Deps | join ", " }}

2012/10/13, David Symonds <dsym...@golang.org>:

minux

unread,
Oct 13, 2012, 8:38:27 AM10/13/12
to Rémy Oudompheng, David Symonds, re...@codereview-hr.appspotmail.com, roger peppe, golan...@googlegroups.com, r...@golang.org

On Sat, Oct 13, 2012 at 8:34 PM, Rémy Oudompheng <remyoud...@gmail.com> wrote:
I also prefer join. Especially if I can write {{ .Deps | join ", " }}
+1. join without hardcoding join character to \n is more useful.
Reply all
Reply to author
Forward
0 new messages