Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gorename command not printed at any logging level #2634

Closed
daniel-santos opened this issue Jan 31, 2023 · 5 comments · May be fixed by #2639
Closed

gorename command not printed at any logging level #2634

daniel-santos opened this issue Jan 31, 2023 · 5 comments · May be fixed by #2639
Assignees
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@daniel-santos
Copy link

When attempting to rename an identifier via F2 or right click --> Rename Symbol, I am am getting the very simple error:

Rename failed: gorename: can't find package containing /home/daniel/proj/mypath/src/go/proto.go

However, this bug report is about the fact that I have logging set to verbose and am not seeing the command run:

{
	"settings": {
		"go.logging.level": "verbose",
	},
}

Note: other settings omitted.

The correct behavior is to output the command run at an appropriate debug logging level (decided by the maintainer), but always when set to verbose.

@gopherbot gopherbot added this to the Untriaged milestone Jan 31, 2023
@hyangah
Copy link
Contributor

hyangah commented Feb 2, 2023

"go.logging.level" was added to debug much later as a hack to debug issues related to finding 'go' binary. :-(
So, I wouldn't be surprised we don't have any instrumentation or logging and the setting has no effect on gorename invocation debugging. We are not actively supporting renaming functionality that uses gorename, and in fact, we are working towards removing non-gopls features (including renaming using gorename).

Can you tell us more about why you couldn't use gopls?

@hyangah hyangah added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 2, 2023
daniel-santos added a commit to daniel-santos/vscode-go that referenced this issue Feb 2, 2023
Fixes golang#2634, or at least it might. I don't know how to
build this project, don't know typescript, etc.
@daniel-santos
Copy link
Author

@hyangah Hello and thank you for your response. Indeed, my project builds Go as a dependency because we're cross-compiling for a few different archs (we'll be running Go on tiny MIPS board!), we have a few hacks to Go as well as a couple of patches that we need to send upstream (MIPS-specific optimizations).

Anyway,

  1. This seems like a trivial change. I've put in a merge request, although I haven't built or tested it since I don't know how. Could I get somebody to do that please?
  2. It would seem that having this work for all tool invocations is not only reasonable, but could serve to entice other users to become contributors to the project.
  3. Even though this feature may have been added as a "hack", it would be a flaw to have it behave inconsistently -- i.e., work for some tool invocations, but not others.
  4. I hope you don't remove support for these features without having a few releases that warns the users that such support is being deprecated! Please don't suddenly yank the rug out from underneath your users! We could lock the version of the add-on, but in these moden-day "ecosystems", this can quickly cause problems, so a little advance warning can help SO much.

Finally, I've avoided having to learn what gopls is and now I'm being forced to. When I read "language server", my brain thinks "ok, so m$ or somebody wants me to use some service in the cloud and collect metrics or data from my work. That's great, another friendly 'let me help you' while they siphon my data." But I'm being a good programmer and reading up. It sounds freaking awesome!!

(Off-topic warning) I really hope there's support (or will be) for modeling via UML. It sounds like a powerful mechanism, even if m$ wrote it! I'm familiar with intermediate code representation as I've worked on gcc, adding optimizations to it. There's a lot that can be done with such IR.

@daniel-santos
Copy link
Author

@hyangah ping

@hyangah hyangah self-assigned this Mar 30, 2023
@hyangah hyangah modified the milestones: Untriaged, vscode-go/later Mar 30, 2023
@adonovan
Copy link
Member

Hi Daniel, I do encourage you to try using the rename feature built in to gopls. The old gorename tool has been unmaintained for a long time and has many bugs, whereas the LSP-integrated feature is actively maintained. We do try to minimize disruption as code evolves, and the old gorename tool has been documented as a "legacy mode" for some time now; see https://github.com/golang/vscode-go/wiki/tools#misc-tools-used-in-the-legacy-mode

Also, be assured that we take the privacy of your source code very seriously, and do not collect any source code or any information derived from it.

@hyangah
Copy link
Contributor

hyangah commented Nov 1, 2023

In the next vscode go (v0.40.0), we are removing all legacy language providers (we announced in #2799 and notified affected users in v0.39.0 released in June. One of the legacy language feature providers is gorename. So, this issue is obsolete.

@daniel-santos Gopls can handle code that is targeted to be cross-compiled (e.g. set GOOS/GOARCH and necessary env vars in the [gopls] [build.env] setting). If it doesn't work in your use case, please file a issue. We are happy to listen and help.

@hyangah hyangah closed this as completed Nov 1, 2023
@hyangah hyangah closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2023
@golang golang locked and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants