Skip to content
New issue

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

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

Already on GitHub? # to your account

Setting go.showWelcome to false does not disable showing the welcome #3319

Closed
sfiggins opened this issue Apr 5, 2024 · 6 comments
Closed
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@sfiggins
Copy link

sfiggins commented Apr 5, 2024

After setting go.showWelcome to false in User or Workspace settings, the go welcome page is still shown.

Noted in version 0.39.1.

I believe that this is because getConfiguration returns a configuration object rather than a value, so the check on the setting always returns true.

I think this could work:

		if (!extensionInfo.isInCloudIDE && vscode.workspace.getConfiguration('go').get('showWelcome')) {
			showGoWelcomePage();
		}

Though it might be easier to add a test on this if it used getGoConfig to retrieve the configuration. The check could also be moved to shouldShowGoWelcome.

@gopherbot gopherbot added this to the Untriaged milestone Apr 5, 2024
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/577075 mentions this issue: extension/src/welcome: fix 'go.showWelcome' setting interpretation

@hyangah
Copy link
Contributor

hyangah commented Apr 6, 2024

Thanks @sfiggins!
Sent a change and added a test. Thanks for the good suggestion!

@hyangah hyangah modified the milestones: Untriaged, v0.41.3 Apr 6, 2024
@hyangah hyangah added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 6, 2024
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/580420 mentions this issue: extension/src/welcome: fix 'go.showWelcome' setting interpretation

@exodustx0
Copy link

It should be noted that the implemented fix didn't just fix and move the if-condition, it also changed its logic: previously, if isInCloudIDE, it would ignore go.showWelcome and not show, whereas now, if isInCloudIDE, it would ignore go.showWelcome and show.

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/581118 mentions this issue: extension/src/welcome: readd the fix for go.showWelcome handling

gopherbot pushed a commit that referenced this issue Apr 23, 2024
The original fix for #3319 also included
a minor refactoring attempt - centralize welcome page show logic
in shouldShowGoWelcomePage. But that unfortunately introduced
a new bug, i.e., failed to apply the Web-based IDE exclusion rule
correctly.

Reverted the previous change to remove the refactoring.
And, this change adds the specific fix to address the bug reported
in #3319.

Test coverage would be nice, but it is tricky without
major refactoring or complicating test logic to stub or
inject dependencies.

Renamed shouldShowGoWelcomePage to make it clear that
this is to decide whether we have news to show purely
based on the extension's version.

For #3319.

Change-Id: Ia4915e0201e73d136b3efcec7c0cf4f5a5e4559f
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/581118
kokoro-CI: kokoro <noreply+kokoro@google.com>
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
@hyangah
Copy link
Contributor

hyangah commented Apr 24, 2024

Thanks @exodustx0 That's obviously a bug. v0.41.4 released now should address the issue. Let us know if you are still seeing the issue.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants