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

vimPlugins.coc-nvim: 0.0.72 -> 0.0.73 (and change override) #64749

Merged
merged 2 commits into from Jul 18, 2019

Conversation

malob
Copy link
Member

@malob malob commented Jul 14, 2019

Motivation for this change

coc.nvim requires a build/index.js file that's only available in the release version. As such, auto-updating isn't possible with the current script since master doesn't contain this file.

The previous fix for this added the index.js file in for a given release, but this caused issues since when vimPlugins was auto-updated, the override wasn't always updated, resulting in the index.js file being out of sync with the rest of the plugin.

This override solves that problem by completely overriding the plugin with a release version.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@malob
Copy link
Member Author

malob commented Jul 14, 2019

cc: @teto, @rvolosatovs

Copy link
Member

@teto teto left a comment

Choose a reason for hiding this comment

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

I've noticed the runtime going out of sync with the javascript causing problems so I support this.
Now that the script is fully described in overrides.nix, maybe we should remove it from vim-plugin-names ?.
@timokau

# The previous fix for this added the index.js file in for a given release, but this caused
# issues since when vimPlugins was auto-updated, the override wasn't always updated, resulting in
# the index.js file being out of sync with the rest of the plugin. This override solves that
# problem by completely overriding the plugin with a release version.
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this much comment. One can use git blame to see a full explanation.
You could narrow it down to "only official releases contains the required index.js file"

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed.

@timokau
Copy link
Member

timokau commented Jul 15, 2019

I've noticed the runtime going out of sync with the javascript causing problems so I support this.
Now that the script is fully described in overrides.nix, maybe we should remove it from vim-plugin-names ?.
@timokau

I have no problem with that if it makes things easier. I don't (yet) use coc myself.

@malob
Copy link
Member Author

malob commented Jul 15, 2019

Alright, I've edited the comment at @teto's request, removed coc.nvim from the vim-plugin-names file, and updated the Vim plugins.

@teto teto merged commit b0f0c75 into NixOS:master Jul 18, 2019
@malob malob deleted the update-coc-nvim branch July 18, 2019 16:53
@gvolpe
Copy link
Member

gvolpe commented Sep 6, 2021

Hi @malob @teto, sorry for the desperate call in this PR but this is all I found on my current (related) issue and I was wondering if you could point me in the right direction? I don't understand all the NodeJS stuff and have always struggled understanding how coc.nvim is packaged in nixpkgs, does not seem easy to override.

I upgraded to the latest NixOS unstable and I am now getting this error:

UnhandledRejection: request error nvim_call_function - Vim:E117: Unknown function: coc#util#has_preview

This happens when I use coc-metals, an LSP client for Scala.

Click here to see the full logs
## versions

vim version: NVIM v0.5.0
node version: v12.20.1
coc.nvim version: 0.0.80-3086844413
coc.nvim directory: /nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim
term: alacritty 
platform: linux

## Log of coc.nvim

2021-09-06T17:16:17.891 WARN (pid:164446) [attach] - Plugin not ready when received "highlight" []
2021-09-06T17:16:18.353 INFO (pid:164446) [services] - registered service "languageserver.dhall"
2021-09-06T17:16:18.354 INFO (pid:164446) [services] - registered service "languageserver.haskell"
2021-09-06T17:16:18.356 INFO (pid:164446) [services] - registered service "languageserver.nix"
2021-09-06T17:16:18.408 INFO (pid:164446) [plugin] - coc.nvim 0.0.80-3086844413 initialized with node: v12.20.1 after 455ms
2021-09-06T17:16:19.159 INFO (pid:164446) [attach] - receive notification: highlight []
2021-09-06T17:16:22.801 INFO (pid:164446) [language-client-index] - Language server "metals" started with 165108
2021-09-06T17:16:31.576 ERROR (pid:164446) [node-client] - request error on "nvim_call_function" [ 'coc#util#has_preview', [] ] Vim:E117: Unknown function: coc#util#has_preview Error
  at ON.request (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:30:34613)
  at ON.call (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:33:3671)
  at /nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:8185
  at Generator.next (native)
  at /nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:856
  at new Promise (native)
  at r (/nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:604)
  at /nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:7801
  at Oe (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:37:1456)
  at tm (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:36:11231)
2021-09-06T17:16:31.583 ERROR (pid:164446) [server] - unhandledRejection  Promise {
<rejected> Error
    at ON.request (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:30:34613)
    at ON.call (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:33:3671)
    at /nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:8185
    at Generator.next (native)
    at /nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:856
    at new Promise (native)
    at r (/nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:604)
    at /nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:7801
    at Oe (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:37:1456)
    at tm (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:36:11231)
} Error
  at ON.request (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:30:34613)
  at ON.call (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:33:3671)
  at /nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:8185
  at Generator.next (native)
  at /nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:856
  at new Promise (native)
  at r (/nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:604)
  at /nix/store/zcbcx718l1dy09rz8lrvqd1cs1y64smn-vimplugin-coc-metals-1.0.7/share/vim-plugins/coc-metals/lib/index.js:28:7801
  at Oe (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:37:1456)
  at tm (/nix/store/xpkfvzymnhcra7w1zyg62mrllbkpz5qk-vimplugin-coc.nvim-2021-08-23/share/vim-plugins/coc.nvim/build/index.js:36:11231)
2021-09-06T17:17:40.875 INFO (pid:164446) [attach] - receive notification: doAutocmd [
1,
{
  regcontents: [ '' ],
  visual: false,
  inclusive: true,
  regname: '',
  operator: 'c',
  regtype: 'v'
},
3,
1003
]
2021-09-06T17:17:41.383 INFO (pid:164446) [completion-complete] - Results from: buffer
2021-09-06T17:17:41.590 INFO (pid:164446) [completion-complete] - Results from: around,buffer
2021-09-06T17:17:42.585 INFO (pid:164446) [attach] - receive notification: highlight []
2021-09-06T17:17:46.093 INFO (pid:164446) [attach] - receive notification: showInfo []

Maintainers have suggested upgrading to the latest coc.nvim where this is allegedly fixed, but I don't know how to.

Appreciate any hints, thanks! 🙏🏽

@malob
Copy link
Member Author

malob commented Sep 6, 2021

@gvolpe, I don't use Coc.nvim anymore since I upgraded to Nvim 0.5, so I'm not very familiar with how Coc related stuff is handled in nixpkgs anymore. I did just have a quick look though, and it looks like Coc.nvim itself is no longer using an override in nixpkgs. It looks like it's just updated automatically via being in the vim-plugins-names file:

neoclide/coc.nvim@release

So looks like updating should just require running the update.py script, and it looks like there's already a PR waiting to be merged that updates it: https://github.com/NixOS/nixpkgs/pull/136786/files#diff-b7a49d194ede1147f14a19a5be0875f0a7d9807db98d49602ec270e5ae44395cR714-R724

@gvolpe
Copy link
Member

gvolpe commented Sep 6, 2021

Ah thanks for your response @malob ! I've been postponing the migration to the native LSP stuff for a while, enough yaks on my shaving list already 😄

I'll make sure to check on that PR, thanks a lot, have a nice day!

@gvolpe
Copy link
Member

gvolpe commented Sep 6, 2021

In case anyone comes across the same issue, an overlay fixed it for me: gvolpe/nix-config@badd9a4

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

Successfully merging this pull request may close these issues.

None yet

4 participants