Shopify / ruby-lsp

An opinionated language server for Ruby
https://shopify.github.io/ruby-lsp/
MIT License
1.33k stars 118 forks source link

Support VSCode formatOnSaveMode: 'modifications' #1973

Closed optikalefx closed 3 weeks ago

optikalefx commented 3 weeks ago

I have checked that this feature is not already implemented

Use case

When working in projects on teams that don't follow project-wide lint rules, those of us sane individuals who still wish to lint our code on-save are left with 2 options.

1) Lint the whole file 2) Don't lint at all

VSCode has a feature called "editor.formatOnSaveMode": "modifications"

Which uses the GIT working copy to only lint modifications instead of the whole file. The reason this is valueable comes down to PRs.

When reviewing someone's code who has linted an entire file, it's nearly impossible to see the actual business change that they made. They either need to submit lint as a sep PR first, or not lint at all, or comment on the changes they actually made.

If we support modifications then you get the best of both worlds. You get linting for your code, and your PR is still only what you changed.

Description

See Use case

Implementation

No response

vinistock commented 3 weeks ago

Thank you for the feature suggestion. This is a duplicate of https://github.com/Shopify/ruby-lsp/issues/203, so I'll close this one to keep discussions centralized in the original issue.

However, I do want to point something out here. It's not just the Ruby LSP that needs to support range formatting, but the linter/formatter itself has to support it.

For example, Syntax Tree has support for range formatting (I know because I added it myself 😄). But RuboCop doesn't. When you give a string for RuboCop to format, it always considers that source code to be a complete file and there's no API to say "format only this AST node" or "format this source code in a partial context".

This means that when we try to range format with RuboCop, you get a very odd experience. Take this example:

# frozen_string_literal: true

class Foo
  def bar
  end
end

Now imagine you selected only the method declaration for bar and then you ask RuboCop to range format. The result is going to be this:

# frozen_string_literal: true

class Foo
# frozen_string_literal: true
def bar
end
end

Because RuboCop is seeing the method declaration as the whole file, you get the magic string literal comment inserted and the indentation level is reduced as if the declaration was in the top level.

All that is to say, supporting range formatting will require work from other gem maintainers to add the necessary functionality and APIs so that the Ruby LSP can integrate with those.