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

Add config to exclude certain binaries from updating #3668

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

EVODelavega
Copy link
Contributor

The only way to skip (or set a specific version) for the binaries installed via :GoInstallBinaries or :GoUpdateBinaries so far is to pass in a specific version. This works, but is somewhat clunky. Projects I'm working often use a specific version of golangci-lint as part of the CI. Instead of having to create mappings, or keep track of which versions and run the correct :GoUpdateBinaries golangci-lint@v1.2.3, I figured it'd be much easier to let me manage the version of that binary myself, and have :GoUpdateBinaries skip installing/updating that binary outright.

Another gripe I have with passing in specific versions of a binary is that, in order to skip one of the binaries, I'd need to list all of the binaries I do want to update (or update all, and then revert one to a specific version). IMHO, adding an exclude list is a much simpler solution, hence the PR.

When adding the docs for this new config variable, I also noticed a small typo in the docs for g:go_debug_breakpoint_sign_text.

Signed-off-by: Elias Van Ootegem <elias@vega.xyz>
@bhcleek
Copy link
Collaborator

bhcleek commented Jun 5, 2024

Thank you for contributing again.

With this change, it looks like :GoUpdateBinaries cannot be used to explicitly install/update a binary that is in g:go_exclude_binaries. Given let g:go_exlude_binaries = ['dlv'], :GoUpdateBinaries dlv would refuse to update dlv, even though the user is explicitly requesting it.

I'm open to this change conceptually, but I believe users should be able to update a tool that's given as an explicit argument. WDYT?

@EVODelavega
Copy link
Contributor Author

EVODelavega commented Jun 7, 2024

Hi, and yes. As it stands, the go_exclude_binaries list does override any and all attempts to install/update binaries, even if explicitly specified. I do agree that args passed directly to the commands ought to take precedence over config, so I've revisited the changes made here a bit, to skip the filtering if binaries are explicitly specified.

Thanks for the feedback.

Signed-off-by: Elias Van Ootegem <elias@vega.xyz>
Signed-off-by: Elias Van Ootegem <elias@vega.xyz>
" specified.
for l:bin in go#config#GoExcludeBinaries()
call remove(l:bin, l:packages)
endfor
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l:packages needs to copy s:packages (use the copy or deepcopy) since entries are being removed so that s:packages is not altered.

Also, :GoInstallBinaries should probably not skip anything that's missing. I think that means that instead of this removal, the check around line 160 that currently reads

      if a:updateBinaries == 1
        echo "vim-go: Updating " . l:binary . ". Reinstalling ". importPath . " to folder " . go_bin_path

should probably be something like

      if a:updateBinaries == 1
        if index(go#config#GoExcludeBinaries(), l:bin) != -1
            echo "vim-go: Skipping out of date " . l:binary . ". "
            continue
        endif
        echo "vim-go: Updating " . l:binary . ". Reinstalling ". importPath . " to folder " . go_bin_path

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

Successfully merging this pull request may close these issues.

2 participants