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

feat(julials): add JuliaActivateEnv to activate a julia environment #3318

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mehalter
Copy link
Contributor

This adds a command for the additional julials API for activating an environment. This is based on how extra functionality is added into the texlab configuration

@mehalter
Copy link
Contributor Author

Looks like this fails the CI/CD check regarding the use of dirname and it potentially being used in the root_dir. I'm just confirming that the use of dirname does not go against the directive listed in the CI/CD check :)

@@ -1,5 +1,34 @@
local util = require 'lspconfig.util'

local function activate_env()
local bufnr = vim.api.nvim_get_current_buf()
local julials_client = util.get_active_client_by_name(bufnr, 'julials')
Copy link
Member

@justinmk justinmk Sep 24, 2024

Choose a reason for hiding this comment

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

use vim.lsp.get_clients(). This util.get_active_client_by_name function should be removed @glepnir (at least in all of our configs, and deprecated if we're worried about breaking consumers).

Copy link
Member

Choose a reason for hiding this comment

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

In order to facilitate the transition of 0.9 users, which version will completely stop supporting 0.9x? 0.10.2 or 0.11. then i can clean up utils including using vim.fs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in latest commit revision

Copy link
Member

@justinmk justinmk Sep 24, 2024

Choose a reason for hiding this comment

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

which version will completely stop supporting 0.9x?

We can drop 0.9 support now, if we want. But that's separate from the first step, which is to all usages of vim.util (where possible) in the configs in this repo :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be a good idea to keep the 0.9 support for now simply because the debian packages are currently at 0.9 so it would be good to have such a core plugin support the versions that are widely available. Moving forward it could be a good idea to make some sort of development practice such as "will support the latest 2 Neovim stable releases at all time or something just so people have an idea what is valid and enable a bit more stability in configuration management, but this is a separate discussion

Copy link
Member

Choose a reason for hiding this comment

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

It might be a good idea to keep the 0.9 support

Irrelevant for this PR. vim.lsp.get_clients() exists in 0.9

Copy link
Member

Choose a reason for hiding this comment

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

Nope. it only in 0.10.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Irrelevant for this PR. vim.lsp.get_clients() exists in 0.9

I figured adding code that doesn't not match the current Neovim version support of the plugin would be irrelevant for this PR and a followup would make the blanket decision of which Neovim version to support. For now, I have removed the 0.9 support as requested

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the idea is to remove all usages of util in the configuration. For now, should I include 0.9 support by checking for the old, deprecated function name?

Copy link
Contributor Author

@mehalter mehalter Sep 24, 2024

Choose a reason for hiding this comment

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

For now it seems like this PR shouldn't change the required neovim version since that is out of scope of what I am doing here. I have removed all reliance of the lspconfig util and also support neovim 0.8+ since that is listed in the README requirements

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.

3 participants