-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: Add gopls.sh #829
base: main
Are you sure you want to change the base?
feat: Add gopls.sh #829
Conversation
Adds a `gopls.sh` script that provides a `asdf`-aware way to start a `gopls` LSP server. This script helps my Emacs load and run the right `gopls` for any given project. To my fellow Emacs users, I offer these configs as an example: ``` (defun find-stencil-root () "Finds the directory that contains stencil.lock" () (message "evaluating find-stencil-root") (locate-dominating-file "." "stencil.lock")) (defun stencil-go-setup () "A go-mode hook for Outreach stencil projects" () (let ((stencil-root (find-stencil-root))) (if stencil-root (progn (setq-local go-command (concat stencil-root ".bootstrap/shell/go.sh")) (setq-local gofmt-command (concat stencil-root ".bootstrap/shell/gofmt.sh")) (setq-local godoc-command (concat stencil-root ".bootstrap/shell/go.sh doc")) (setq-local go-test-command (concat stencil-root ".bootstrap/shell/go.sh test -ldflags '-X github.com/getoutreach/go-outreach/v2/pkg/app.Version=testing -X github.com/getoutreach/gobox/pkg/app.Version=testing' -tags=or_test")) (setq-local lsp-go-gopls-server-path (concat stencil-root ".bootstrap/shell/gopls.sh")) (setq-local lsp-go-env '((GOFLAGS . "-tags=or_test,or_e2e"))) (setq-local flycheck-go-test-executable (concat stencil-root ".bootstrap/shell/go.sh") ) (setq-local flycheck-go-vet-executable (concat stencil-root ".bootstrap/shell/go.sh") ) (setq-local flycheck-go-gofmt-executable (concat stencil-root ".bootstrap/shell/gofmt.sh") ) (setq-local flycheck-go-build-executable (concat stencil-root ".bootstrap/shell/go.sh") ) ) () ) ) ) (add-hook 'go-mode-hook 'stencil-go-setup) ``` (Note the `gopls.sh` references in the middle there.) Folks using a different editor might not benefit from this specific advice, but I'm hoping the `gopls.sh` script will be of some use to them, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a bunch of opinions on this which I will write down in the morning. Marking as "requested changes" so it doesn't accidentally get merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I'm generally not inclined to add this for a few reasons:
- At least at Outreach,
gopls
is installed/updated every time a new Go version is installed, via functionality inasdf
. - We shouldn't put the
gopls
version in the script itself, if we need to track that, it should go intoversions.yaml
and the script should use theget_tool_version
function (see other scripts for its use). - Per the script docs, I don't know if we want to add
gopls
to services'.tool-versions
- it would be yet another item for DT to track, particularly when there may be breaking changes, and those changes would only affect a small minority of developers who don't use VSCode (VSCode installsgopls
automatically, just for VSCode as I understand it).
For whatever reason, this doesn't seem to be working for me. I always have to install and reshim the first time I encounter a new Go in a
Yeah, fair point. I personally would be OK with |
FWIW this is what it looks like (details omitted) when I install a previously uninstalled Go version:
|
Adds a
gopls.sh
script that provides aasdf
-aware way to start agopls
LSP server. This script helps my Emacs load and run the rightgopls
for any given project.To my fellow Emacs users, I offer these configs as an example:
(Note the
gopls.sh
references in the middle there.)Folks using a different editor might not benefit from this specific
advice, but I'm hoping the
gopls.sh
script will be of some use tothem, too.