Skip to content

cmd/present: embed static and template files #264

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

Closed
wants to merge 10 commits into from
Closed

cmd/present: embed static and template files #264

wants to merge 10 commits into from

Conversation

ayang64
Copy link
Member

@ayang64 ayang64 commented Jan 1, 2021

Prior to this commit, the present tool relied on static and template
files in ${GOPATH}/src/golang.org/x/tools/cmd/present.

As of Go 1.16 that directory is not populated when the present tool is
installed using the go get command.

This commit embeds those assets in the present tool binary using
embed.FS.

To acheive this, we moved both static and template data to a "base"
directory in the source. This way, embedding "base" will include both
templates and static assets. We wrap the resulting embedded filesystem
with fs.Sub() to remove the "base" prefix.

Closes: golang/go#43459

stamblerre and others added 10 commits December 11, 2020 19:22
Change-Id: I428602bb1fbf886f949ecf12fa4d18818c5051ba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/277312
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>
Reviewed-by: Peter Weinberger <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Also, remove the replace directive.

Change-Id: I49ef2af4e3d796fd40fa6574a10a73e76b442f76
Reviewed-on: https://go-review.googlesource.com/c/tools/+/277313
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
…and update go.sum fixes

These were missing titles, which was showing up empty for users in the
VS Code UI and leading people not to trust the fixes.

Also did a find references on SuggestedFix to confirm that we always
set the title in other cases.

Fixes golang/go#43234

Change-Id: I8d0f272c383a2e1a364aefcec6650988d18d4fb4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/278778
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
(cherry picked from commit 48e5bd1)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/278785
Change-Id: I3337e90054b38206fd46053b8151e8886f625be4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/278786
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Go Bot <[email protected]>
…h.0.6

ef3185b internal/lsp/cache: add a check for snapshot invariants
b841374 internal/lsp: fix autocomplete appends on imports
929a849 internal/lsp: restructure the way we report critical errors
fbbba25 gopls/doc: generate documentation for analyzers
84d76fe internal/lsp: fix unimported completions with -mod=readonly
0661ca7 gopls/internal/regtest: show line numbers if TestUseGoplsMod fails
13ff221 internal/lsp/cmd: include new name in rename help message
0f6027f gopls/doc/vim.md: add table of contents, improve neovim docs
34cd474 internal/lsp: use an enum for GC annotations settings
b1c9089 internal/lsp/source: do not panic in "var func" outgoing callhierarchy
2b0845d gopls/release: add a command to validate the gopls release process
57089f8 internal/lsp: remove dependencies using text edits when necessary
834755c internal/lsp: add tests to set configuration options
bdbb3c9 internal/lsp/cache: only reload the workspace on saved changes
3e0a2b7 gopls/internal/regtest: skip a some new builders where regtests time out
f6952e4 internal/lsp/cache: fix some package event tags
9cbb1ef internal/lsp/source: add the shadow analyzer
3fa0e8f gopls: disable TestTemplate on Android
f2e330f gopls/test: add type checking for debug server templates
1965356 internal/lsp/mod: fix misplaced code lens with individual requires
b57d1c5 internal/lsp: return an error if code action has no title
ae774e9 internal/lsp: don't show duplicate diagnostics for go.mod errors
5b06639 internal/lsp/source: only show "run file benchmarks" if available
11a5667 gopls/internal/regtest: test metadata validation only on save for go.mod
5b43ef9 go/analysis/passes/fieldalignment: filter comments from suggested fix
4b31ac3 gopls/internal/regtest: test that accepting fix removes diagnostics
008e477 internal/lsp, gopls: recover from go-diff panics
48e5bd1 internal/lsp: add titles to `go mod tidy` and update go.sum fixes
fa10ef0 internal/lsp/source: update codelenses example
6307297 go/analysis/passes/fieldalignment: support fields without name

Change-Id: I449fee8301240637662f60930fe8b20e133fdd81
This CL copies Heschi's structural changes to the options from CL 278433
and makes the necessary adjustments in the JSON and documentation
generation. Nested settings are grouped together and the "status" of a
given setting is also listed. Currently the only possible statuses are
"experimental" and "debug", but I will add "advanced" in a follow-up (to
indicate that a setting is only for advanced users).

The options "set" function still expects flattened settings to avoid
fundamentally changing people's current configurations, so VS Code Go
will just have to make sure to flatten the settings before sending them
to gopls (which should be easy enough).

No names of any settings are changed (Heschi's earlier CL adjusted the
experimental prefixes). As discussed offline, we've decided to prefix
any setting that we expect to delete with "experimental", and so we'll
leave existing setting names as they are.

Updates golang/go#43101

Change-Id: I55cf7ef09ce7b5b1f8af06fcadb4ba2a44ec9b17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280192
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
The possible keys for analyses and codelenses are too long to enumerate
in settings, and we'd need to create enums for all possible analyzer
and code lens names, which is probably not feasible. Instead, collect
the list of possible values from the analyzers and command settings
generation and add them to enum values.

Also, handle default values by setting them in the enum keys instead of
one big default value. Quite a few hacks to get this right, but maybe
there are other better alternatives we can consider in the future.

Fixes golang/go#42961

Change-Id: I5c096862b5e8fb89fe5d6639b4f46c06492e49c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280355
Trust: Rebecca Stambler <[email protected]>
Trust: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
…se-branch.0.6"

This reverts commit 481d425.

Reason for revert: Merged to the wrong branch

Change-Id: I1bed47cd0857c22af0238b6c6c8b87781c7ac49e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280707
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Peter Weinberger <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
Prior to this commit, the present tool relied on static and template
files in ${GOPATH}/src/golang.org/x/tools/cmd/present.

As of Go 1.16 that directory is not populated when the present tool is
installed using the go get command.

This commit embeds those assets in the present tool binary using
embed.FS.

To acheive this, we moved both static and template data to a "base"
directory in the source.  This way, embedding "base" will include both
templates and static assets.  We wrap the resulting embedded filesystem
with fs.Sub() to remove the "base" prefix.
@google-cla google-cla bot added the cla: yes label Jan 1, 2021
@gopherbot
Copy link
Contributor

This PR (HEAD: 8f408d7) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/281032 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Zik Aeroh:

Patch Set 2: Code-Review-1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/281032.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Zik Aeroh:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/281032.
After addressing review feedback, remember to publish your drafts!

@rogpeppe
Copy link
Contributor

@ayang64 Is this fixed somewhere else? Because the reason for rejecting it seems to have gone away now (https://golang.org/issue/40067 and https://golang.org/issue/43400 have been fixed) so it might seem reasonable to consider fixing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x/tools/cmd/present: doesn't work with go in git master (1.16)
4 participants