code review 2220045: Add option to gofmt to help build tools that need packa...

Add option to gofmt to help build tools that need package name and imports.

[albert.strasheim, 2010-09-16 13:18:24]

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

I'd like you to review this change.

[gri, 2010-09-16 17:07:30]

Thanks, but this code (as is) doesn't belong into gofmt.

(If it were simply exposing all of the three parser modes (parse package
clause, parse imports, parse all) and use the normal formatting I could
perhaps be talked into it. But I think that would defeat the purpose as
imports would show up in various forms.)

It is very tempting to put this into gofmt because it's just a few lines and
all the infrastructure is there, but it opens the door to all kinds of other
things that have nothing to do with formatting. We don't want gofmt to
become the Swiss Army Knife of tools. It is intentionally kept super-simple
(and arguably, some of the other debug flags shouldn't be there, either).

Thanks.
- gri



On Thu, Sep 16, 2010 at 6:18 AM, <fullung@gmail.com> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> Add option to gofmt to help build tools that need package name and
> imports.
>
> Please review this at http://codereview.appspot.com/2220045/
>
> Affected files:
>  M src/cmd/gofmt/gofmt.go
>
>
> Index: src/cmd/gofmt/gofmt.go
> ===================================================================
> --- a/src/cmd/gofmt/gofmt.go
> +++ b/src/cmd/gofmt/gofmt.go
> @@ -12,6 +12,7 @@
>        "go/parser"
>        "go/printer"
>        "go/scanner"
> +       "go/token"
>        "io/ioutil"
>        "os"
>        pathutil "path"
> @@ -26,6 +27,7 @@
>        rewriteRule = flag.String("r", "", "rewrite rule (e.g., 'α[β:len(α)]
> -> α[β:]')")
>
>        // debugging support
> +       build    = flag.Bool("build", false, "print code useful for build
> tools")
>        comments = flag.Bool("comments", true, "print comments")
>        trace    = flag.Bool("trace", false, "print parse trace")
>        printAST = flag.Bool("ast", false, "print AST (before rewrites)")
> @@ -66,6 +68,9 @@
>        if *trace {
>                parserMode |= parser.Trace
>        }
> +       if *build && !*write {
> +               parserMode |= parser.ImportsOnly
> +       }
>  }
>
>
> @@ -86,6 +91,22 @@
>  }
>
>
> +func extractImports(file *ast.File) <-chan *ast.ImportSpec {
> +       ch := make(chan *ast.ImportSpec)
> +       go func() {
> +               defer close(ch)
> +               for _, decl := range file.Decls {
> +                       if gd, ok := decl.(*ast.GenDecl); ok && gd.Tok ==
> token.IMPORT {
> +                               for _, spec := range gd.Specs {
> +                                       ch <- spec.(*ast.ImportSpec)
> +                               }
> +                       }
> +               }
> +       }()
> +       return ch
> +}
> +
> +
>  func processFile(f *os.File) os.Error {
>        src, err := ioutil.ReadAll(f)
>        if err != nil {
> @@ -98,6 +119,14 @@
>                return err
>        }
>
> +       if *build && !*write {
> +               fmt.Printf("package %s\n", file.Name.Name)
> +               for spec := range extractImports(file) {
> +                       fmt.Printf("import %s\n", string(spec.Path.Value))
> +               }
> +               return err
> +       }
> +
>        if *printAST {
>                ast.Print(file)
>        }
>
>
>

[rsc1, 2010-09-16 20:48:50]

Closing review.