Grokbase › Groups › Go › golang-nuts › April 2015
FAQ
Hi,

Currently the default build context is passed to the `gorename` command:

https://github.com/golang/tools/blob/master/cmd/gorename/main.go#L55

This is ok for most use cases, however if the source code has a tag
that is not satisfied by the default constraints, gorename refuses to
work at all. Say "// +build appengine". This also hit someone while
using vim-go (which uses gorename as a wrapper):
https://github.com/fatih/vim-go/issues/389

I want to introduce a new flag called `-tags` which would be a comma
separated tags that will be passed to the build contexts, "BuildTags"
field. It would be backwards comatible. The usage would be in the form
of:

gorename -offset file.go:#123 -to foo -tags "appengine"
gorename -offset file.go:#123 -to foo -tags "appengine,integration"

If it's ok I want o open a CL that introduces this change. We could of
course change the flag name to something like "-buildtags" or
"-build-tags".

Thanks in advance

Regards

--
Fatih Arslan

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Search Discussions

  • Alan Donovan at Apr 21, 2015 at 6:38 pm ⇧

    On 21 April 2015 at 09:40, Fatih Arslan wrote:

    Hi,

    Currently the default build context is passed to the `gorename` command:

    https://github.com/golang/tools/blob/master/cmd/gorename/main.go#L55

    This is ok for most use cases, however if the source code has a tag
    that is not satisfied by the default constraints, gorename refuses to
    work at all. Say "// +build appengine". This also hit someone while
    using vim-go (which uses gorename as a wrapper):
    https://github.com/fatih/vim-go/issues/389

    I want to introduce a new flag called `-tags` which would be a comma
    separated tags that will be passed to the build contexts, "BuildTags"
    field. It would be backwards comatible. The usage would be in the form
    of:

    gorename -offset file.go:#123 -to foo -tags "appengine"
    gorename -offset file.go:#123 -to foo -tags "appengine,integration"

    If it's ok I want o open a CL that introduces this change. We could of
    course change the flag name to something like "-buildtags" or
    "-build-tags".

    Since you're the second person this week to ask for this essentially
    feature, you're in luck:

    https://go-review.googlesource.com/#/c/9172/1

    However, I'm not sure how effective it will be for the refactoring tools
    (gorename and eg) because the Go type checker can only make sense of a
    single configuration at a time. If your entire package is subject to the
    "appengine" build tag, renaming should work just fine, but if some files
    require the tag and some require its negation, the best you can hope for is
    to use the tool on the majority of files and deal with the rest by hand.
    Unfortunately this is a fundamental design issue of the type checker.

    cheers
    alan

    --
    You received this message because you are subscribed to the Google Groups "golang-nuts" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Fatih Arslan at Apr 21, 2015 at 7:21 pm ⇧
    Thanks Alan for the clarification. Though there is something I'm not
    sure. You said if the entire package is subject to the `appengine`
    build tag, renaming should work fine. However I've just added a test
    case (removed the others to not clutter) to "TestRewrites" in
    `rename_tests.go`(https://github.com/golang/tools/blob/master/refactor/rename/rename_test.go#L419).

    Assuming that each case is self contained, because they have a package
    declaration, I would assume the following would not fail:

    http://play.golang.org/p/7PpPcf3S2-

    But it fails with the error:

    rename_test.go:478: -from "main::foo2" -to "foo": unexpected error: no
    object "foo2" declared in package "main"

    However if you pass the "Integration" to the buildTags in
    "fakeContext" (just comment out in the snippe above") it'll work
    without any problem and the tests pass. So according to tests I need
    to pass the build tags to the context.

    Do I miss something here?

    Thanks

    On Tue, Apr 21, 2015 at 9:38 PM, Alan Donovan wrote:
    On 21 April 2015 at 09:40, Fatih Arslan wrote:

    Hi,

    Currently the default build context is passed to the `gorename` command:

    https://github.com/golang/tools/blob/master/cmd/gorename/main.go#L55

    This is ok for most use cases, however if the source code has a tag
    that is not satisfied by the default constraints, gorename refuses to
    work at all. Say "// +build appengine". This also hit someone while
    using vim-go (which uses gorename as a wrapper):
    https://github.com/fatih/vim-go/issues/389

    I want to introduce a new flag called `-tags` which would be a comma
    separated tags that will be passed to the build contexts, "BuildTags"
    field. It would be backwards comatible. The usage would be in the form
    of:

    gorename -offset file.go:#123 -to foo -tags "appengine"
    gorename -offset file.go:#123 -to foo -tags "appengine,integration"

    If it's ok I want o open a CL that introduces this change. We could of
    course change the flag name to something like "-buildtags" or
    "-build-tags".


    Since you're the second person this week to ask for this essentially
    feature, you're in luck:

    https://go-review.googlesource.com/#/c/9172/1

    However, I'm not sure how effective it will be for the refactoring tools
    (gorename and eg) because the Go type checker can only make sense of a
    single configuration at a time. If your entire package is subject to the
    "appengine" build tag, renaming should work just fine, but if some files
    require the tag and some require its negation, the best you can hope for is
    to use the tool on the majority of files and deal with the rest by hand.
    Unfortunately this is a fundamental design issue of the type checker.

    cheers
    alan


    --
    Fatih Arslan

    --
    You received this message because you are subscribed to the Google Groups "golang-nuts" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Alan Donovan at Apr 21, 2015 at 7:36 pm ⇧

    On 21 April 2015 at 15:20, Fatih Arslan wrote:

    You said if the entire package is subject to the `appengine`
    build tag, renaming should work fine.

    Yes, but you must still specify the "appengine" build tag. Without it, the
    package would be empty.

    The problem I was alluding to is that if some files in the package



    However I've just added a test
    case (removed the others to not clutter) to "TestRewrites" in
    `rename_tests.go`(
    https://github.com/golang/tools/blob/master/refactor/rename/rename_test.go#L419
    ).

    Assuming that each case is self contained, because they have a package
    declaration, I would assume the following would not fail:

    http://play.golang.org/p/7PpPcf3S2-

    But it fails with the error:

    rename_test.go:478: -from "main::foo2" -to "foo": unexpected error: no
    object "foo2" declared in package "main"
    That's correct. The package is empty.


    However if you pass the "Integration" to the buildTags in
    "fakeContext" (just comment out in the snippe above") it'll work
    without any problem and the tests pass. So according to tests I need
    to pass the build tags to the context.
    Yes.


    Do I miss something here?
    >

    I don't think so.

    --
    You received this message because you are subscribed to the Google Groups "golang-nuts" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Fatih Arslan at Apr 21, 2015 at 7:47 pm ⇧
    The problem I was alluding to is that if some files in the package
    Ok I got what you mean now :)

    Thanks for including the functionality. It'll be useful for those
    whose need it. I'll add a the functionality to vim-go once it's been
    usable.

    Regards
    On Tue, Apr 21, 2015 at 10:36 PM, Alan Donovan wrote:
    On 21 April 2015 at 15:20, Fatih Arslan wrote:

    You said if the entire package is subject to the `appengine`
    build tag, renaming should work fine.

    Yes, but you must still specify the "appengine" build tag. Without it, the
    package would be empty.

    The problem I was alluding to is that if some files in the package


    However I've just added a test
    case (removed the others to not clutter) to "TestRewrites" in

    `rename_tests.go`(https://github.com/golang/tools/blob/master/refactor/rename/rename_test.go#L419).

    Assuming that each case is self contained, because they have a package
    declaration, I would assume the following would not fail:

    http://play.golang.org/p/7PpPcf3S2-

    But it fails with the error:

    rename_test.go:478: -from "main::foo2" -to "foo": unexpected error: no
    object "foo2" declared in package "main"

    That's correct. The package is empty.

    However if you pass the "Integration" to the buildTags in
    "fakeContext" (just comment out in the snippe above") it'll work
    without any problem and the tests pass. So according to tests I need
    to pass the build tags to the context.

    Yes.

    Do I miss something here?

    I don't think so.


    --
    Fatih Arslan

    --
    You received this message because you are subscribed to the Google Groups "golang-nuts" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-nuts @
categoriesgo
postedApr 21, '15 at 1:41p
activeApr 21, '15 at 7:47p
posts5
users2
websitegolang.org

2 users in discussion

Fatih Arslan: 3 posts Alan Donovan: 2 posts

Content

People

Support

Translate

site design / logo © 2022 Grokbase