Skip to content

Conversation

@kdheepak
Copy link
Contributor

@kdheepak kdheepak commented Jun 2, 2020

This PR updates julials to a working configuration. It appears adding using SymbolServer is important to prevent segfaults.

See julia-vscode/LanguageServer.jl#735 for more information.

@cmcaine
Copy link

cmcaine commented Jun 2, 2020

I think your commit is not clean: it has changes to jedi and dart, too. Might need a rebase :)

@cmcaine
Copy link

cmcaine commented Jun 2, 2020

Oh, sorry, it's the bot's commit that is not clean. I don't know what that's about, so I'll bow out.

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 2, 2020

I branched from the latest master, and double checked as well when I saw the bot's commit :) I should have posted here though. I'm not sure what is going on with the bot.

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 2, 2020

The bot updates the README based on the options in the package.json from respective vscode plugins. I didn't know that, that's pretty neat! It was looking at this file and that has changed in the last week or so since the README.md was last updated.

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 2, 2020

So this PR is ready to be reviewed.

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 8, 2020

@h-michael I have a question here.

The previous implementation of the configuration was installing the Julia LanguageServer and SymbolServer packages in the global Julia environment. This is not recommended by the LanguageServer developers.

Julia has good support for environments using built in package manager. After discussing with other Julia users, I've modified the PR above to add an environment that is specified by a JuliaProject.toml file. And I've modified the lsp client to start the Julia process and run the LanguageServer code using this specified environment file.

My question is, is this okay? Where should one put supplementary files to the lua files. Right now I put the JuliaProject.toml file in the lua/nvim_lsp folder. If you have a better suggestion let me know and I'll modify this PR.

@clason
Copy link
Member

clason commented Jun 8, 2020

My question is, is this okay? Where should one put supplementary files to the lua files. Right now I put the JuliaProject.toml file in the lua/nvim_lsp folder. If you have a better suggestion let me know and I'll modify this PR.

I think this should be made configurable, analogously to the cmd keyword for servers. (I for one would rather put it in .cache/nvim/lsp.)

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 8, 2020

Thanks for the comment @clason! I think making that configurable as part of a user setting is straightforward. But we still need to ship the JuliaProject.toml file as part of this repo, and I'm wondering where in this repo should we put this file.

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 8, 2020

Okay, I've updated it such that it uses ~/.cache/nvim/nvim_lsp/julials. I've also removed the JuliaProject.toml file. I think users can see the documentation and manually configure this environment however they want.

@clason
Copy link
Member

clason commented Jun 8, 2020

I was about to write something like that :) One small nit: I wouldn't hard code the path but refer to the XDG compliant variables (e.g., vim.fn.stdpath("cache")).

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 8, 2020

I've used this:

environment_directory = util.path.join(util.base_install_dir, "julials")

On my computer, other language servers seem to put files in there. So I think that should be fine. It expands to the ~/.cache/nvim/nvim_lsp/julials.

I've hardcoded it in the docstring though. Maybe I should change it there was well?

@clason
Copy link
Member

clason commented Jun 8, 2020

I've hardcoded it in the docstring though. Maybe I should change it there was well?

Well, the doc string should correspond to what actually happens :) Yes, better replace it with that, otherwise it will be confusing on Windows... (If you look at util.lua, it's indeed stdpath('cache')/nvim_lsp.)

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 8, 2020

Okay great! Thanks for all the comments! I've updated it so that docstring uses the same environment_directory as the install and cmd script. I've also tested it locally and it works as expected.

@clason
Copy link
Member

clason commented Jun 8, 2020

(but my user name isn't runner ;))

And just to make sure: if you put the server in a separate environment, it will still index and provide info for packages you use in the actual environment you're working on?

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 8, 2020

Yeah, the user name being runner in the documentation is what I wanted to avoid by hardcoding it. But I think that can be addressed by a comment in the README :)

Yes, it should! If you can try it and report any issues that would be great. I'm actually having issues in some packages where it is working fine within the package but isn't tracking dependencies correctly. I'm looking into it at the moment.

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 8, 2020

Okay, dependencies are being found correctly now as well! Thanks to @non-Jedi for all the help figuring this out.

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 9, 2020

With the exception of autocomplete, everything is working as expected for me across multiple projects. This should be good to do :) cc @h-michael

@kdheepak
Copy link
Contributor Author

kdheepak commented Jun 9, 2020

I've made the change you requested @h-michael and rebased on top of the latest master as well.

@h-michael h-michael merged commit b6f79e6 into neovim:master Jun 9, 2020
@h-michael
Copy link
Contributor

Ty :)

@kdheepak kdheepak deleted the kd/fix-julials branch June 9, 2020 04:59
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.

5 participants