Skip to content

Allow CLI flags for lsp #99

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

Merged
merged 2 commits into from
Jun 21, 2022
Merged

Conversation

xeger
Copy link
Contributor

@xeger xeger commented Jun 18, 2022

This lets me invoke the language server with plugins registered, to ensure that formatting works the way I want it to in my project. Rebased on top of #100 so that I could test them in concert. Works like a charm with vscode-syntax-tree!

P.S. this PR could stand to have a test added to make sure LSP always parses opts. LMK if the approach is sound and I'll whip up a test.

#
if arguments.first&.start_with?("--plugins=")
plugins = arguments.shift[/^--plugins=(.*)$/, 1]
plugins.split(",").each { |plugin| require "syntax_tree/#{plugin}" }
Copy link
Contributor Author

@xeger xeger Jun 19, 2022

Choose a reason for hiding this comment

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

Doing a self review, it strikes me that it's not obvious how one would register a third-party plugin (that isn't part of the syntax_tree gem).

I suppose another gem could expose a lib/syntax_tree/foo source, which might work in a Bundler setting (where the gem is pre-activated and its lib is added to the load paths).

When using system gems, I'm not sure this "delegated lib subdir" idea would work, as I vaguely recall some magic with require'ing a gem name and auto-activating it which might foil "deep require" of nonstandard-named lib subdirs... it's been too long since I've been without Bundler!

Perhaps the CLI needs an -r option to ensure gems are activated .. or perhaps an executable config file that can do whatever and configure syntax_tree is a better fit? 🤔

Anyhow: my comment applies to all uses of this option, lsp or no; not particularly relevant to this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I tried to be explicit about this in the README (https://github.com/ruby-syntax-tree/syntax_tree#configuration). It should be fine to add a syntax_tree directory under a path that is already in the require path. There are a lot of gems that do this (for example any Rails plugin that adds generators).

If folks need other gems activated that they for some reason can't require in their own plugins, I'm going to say they should use RUBYOPT instead. This isn't meant to be a whole replacement for the require system, just a small convenience option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hadn't considered RUBYOPT that ought to be sufficient for anyone's needs.

xeger added 2 commits June 19, 2022 15:06

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
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.

None yet

2 participants