Skip to content

Add ruby major mode keybindings#159

Merged
stevenguh merged 3 commits intoVSpaceCode:masterfrom
mdudzinski:add-ruby-major-mode-keybindings
Jan 24, 2021
Merged

Add ruby major mode keybindings#159
stevenguh merged 3 commits intoVSpaceCode:masterfrom
mdudzinski:add-ruby-major-mode-keybindings

Conversation

@mdudzinski
Copy link
Copy Markdown
Contributor

@mdudzinski mdudzinski commented Jan 11, 2021

Just a basic set of major keybindings for Ruby language. Keybindings are based on the editor commands and require Ruby and/or Ruby Solargraph extensions to be installed.

Related issue #158

@stevenguh
Copy link
Copy Markdown
Member

stevenguh commented Jan 11, 2021

That's awesome! All of the commands are from vscode, and that's fine. Are there any specific commands from either one of the extension?

If the answer is yes, is one of the extension preferred than the other? Currently we can only prefer one extension when the bindings contain commands from a specific extension (e.g. when the command in the major mode contains require a specific extension). Btw, there's a plan to refactor the core which allows us to have a plug-in like structure for supporting different major modes of a language.

@mdudzinski
Copy link
Copy Markdown
Contributor Author

I believe both come with some specific commands but I haven't used them. And it's worth noting those extensions aren't mutually exclusive. The Ruby extensions provides many features like syntax highlighting, snippets, debugger, linting and basic intellisense. However, you can disable linting, codecompletion and intellisense and allow other extension (Solargraph!) to take care of it. I prefer that approach because solargraph is IDE-agnostic and I use it with my (space)vim configuration.
But other people may be just fine with only Ruby extension and its LSP implementation. Using just editor commands in the major is extension agnostic :)

It's a really good plan to support different major modes for a single language! I can imagine other languages can have more extensions that can provide different commands for the same actions.

@mdudzinski
Copy link
Copy Markdown
Contributor Author

Now I'm wondering - shouldn't the default editor commands be available to all languages? Either via major mode or a new keybinding. I was thinking about l from language but it's taken by layouts.
It would be up to users whether they have providers (via extensions) installed for the given language. And it would save package.json from being bloated of copy-pasted keybindings for every language.

@stevenguh
Copy link
Copy Markdown
Member

stevenguh commented Jan 12, 2021

I believe both come with some specific commands but I haven't used them. And it's worth noting those extensions aren't mutually exclusive. The Ruby extensions provides many features like syntax highlighting, snippets, debugger, linting and basic intellisense. However, you can disable linting, codecompletion and intellisense and allow other extension (Solargraph!) to take care of it. I prefer that approach because solargraph is IDE-agnostic and I use it with my (space)vim configuration.
But other people may be just fine with only Ruby extension and its LSP implementation. Using just editor commands in the major is extension agnostic :)

Can you distill some of the gems (knowledge) as documentation? Is there any chance you can update our documentation like VSpaceCode/vspacecode.github.io#24?

It's a really good plan to support different major modes for a single language! I can imagine other languages can have more extensions that can provide different commands for the same actions.

Right, the thinking is like a major mode plug-in (A bit like layer concept in spacemacs) building on top of the extension system of vscode. So we can have

  • vspacecode-ruby that bundles the Ruby extension and have command specific to it
  • vspacecode-solargraph that bundles the Solargraph and have command specific to it
  • vspacecode-ruby-solargraph that bundles both Ruby and Solargraph extension and have commands specific to both extensions

I am working toward that but right now we can't really offer alternative on the major modes.

Now I'm wondering - shouldn't the default editor commands be available to all languages? Either via major mode or a new keybinding. I was thinking about l from language but it's taken by layouts.

Right, that's why some of the commands in your bindings can somewhat be achieved with the existing bindings or via vim. For example:
Go to definition - g d in Vim
Peek references - <spc> s r
Rename symbol - <spc> s e

We can consider integrate more of the built-in commands into non-major mode bindings.

It would be up to users whether they have providers (via extensions) installed for the given language. And it would save package.json from being bloated of copy-pasted keybindings for every language.

Yeah, that's what the planned work I mentioned above would help so that the package.json is more manageable, and users will install the major mode as an extension in vscode marketplace. As far as duplication in the major mode, it is probably okay for now. Ideally, the major mode should have commands specific to the extension (LSP). In the future, the refactoring should facilitate binding reuse, so major mode won't have to redeclare similar bindings.

@stevenguh stevenguh mentioned this pull request Jan 12, 2021
2 tasks
@marcoieni marcoieni mentioned this pull request Jan 12, 2021
20 tasks
@marcoieni
Copy link
Copy Markdown
Member

marcoieni commented Jan 12, 2021

Can you distill some of the gems (knowledge) as documentation? Is there any chance you can update our documentation like VSpaceCode/vspacecode.github.io#24?

Ok, so basically once we write in the docs that Ruby is required we can merge this, right?

Anyway, regarding solargraph..maybe if these actions works without solargraph there is no need to suggest to install it, right?

@stevenguh
Copy link
Copy Markdown
Member

Can you distill some of the gems (knowledge) as documentation? Is there any chance you can update our documentation like VSpaceCode/vspacecode.github.io#24?

Ok, so basically once we write in the docs that Ruby is required we can merge this, right?

I think so :) Sorry for a lot of extra context😅

Anyway, regarding solargraph..maybe if these actions works without solargraph there is no need to suggest to install it, right?

Either way would be fine. I thought it would nice to document it, but it may be hard to explain?

@marcoieni
Copy link
Copy Markdown
Member

Either way would be fine. I thought it would nice to document it, but it may be hard to explain?

As mdudzinski prefers :)

@mdudzinski
Copy link
Copy Markdown
Contributor Author

@stevenguh Sure I'll update the documentation.

I agree that it may be hard to explain about Solargraph. The nice thimg about the Ruby extension's LSP implementation is it doesn't require any external software (because the LSP server is written in TypeScript and bundled with the extension) where the Solargraph extension is just a client for the 3rd party server than one needs to install to have the Solargraph extension working.

@stevenguh
Copy link
Copy Markdown
Member

stevenguh commented Jan 14, 2021

Thanks. We will merge this once there's a documentation PR.

It's pretty cool that Ruby extension's LSP is in TypeScript. I am surprised that Solargraph extension doesn't install the LSP on launch automatically like C# extension (but then I don't know much about Ruby)

@mdudzinski mdudzinski force-pushed the add-ruby-major-mode-keybindings branch from 6d5c29b to ed2bc79 Compare January 23, 2021 20:15
@mdudzinski
Copy link
Copy Markdown
Contributor Author

I rebased the branch against master. PR for the documentation > VSpaceCode/vspacecode.github.io#28

I wanted to test keybindings with just Ruby extension (to be sure Solargraph is optional) but its LSP server didn't work for me. So I marked both extensions as required.

@marcoieni
Copy link
Copy Markdown
Member

From the docs it looks like the LSP has to be enabled. See the section "Using the Language Server" here.

@mdudzinski
Copy link
Copy Markdown
Contributor Author

@marcoieni Yes, I know it has to be enabled. It was and it just didn't boot and was stuck at Initializing Ruby language server.... May be something wrong on my end but anyway I need both extensions in order to make the ruby major mode keybindings work.

I can change the documentation patch (VSpaceCode/vspacecode.github.io#28) and make Ruby Solargraph recommended instead of required but I am not really sure if the current version of language-server-ruby supports all commands used in the bindings. And I can't test it.

@marcoieni
Copy link
Copy Markdown
Member

Ok, I think it's fine like this.
Maybe if in the future you notice that something changes you can update the docs if you have time :)
Steven if you like it too, you can merge this.

@mdudzinski
Copy link
Copy Markdown
Contributor Author

@marcoieni I know it isn't ideal but I'll update the docs in the future if I manage to make the LSP server from the Ruby extension work. Cheers!

@stevenguh stevenguh merged commit fe6c779 into VSpaceCode:master Jan 24, 2021
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