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

Add an option to redirect offence reports to stderr #9043

Merged
merged 1 commit into from Nov 20, 2020

Conversation

knu
Copy link
Contributor

@knu knu commented Nov 13, 2020

This adds --stderr to make it easier to use --stdin in shell scripts, editor integration, CI workflow, etc.

In response to feedbacks, I altered this PR from changing the behavior of --stdin to adding a new option.

Here's the original description. This makes editor integration much easier to implement by eliminating the need to process the output.

In the short term, existing editor plugins that expect ==================== will be broken, but it should be relatively easy to fix.

For example, vscode-ruby-rubocop can check if stderr output contains (or ends with) ==================== to see if it needs to delete warnings from stdout output. (to support any version of rubocop)

Eventually, they just need to simply use the stdout output.


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 13, 2020

I get the rationale, but won't this break the existing editor plugins?

@dvandersluis
Copy link
Member

Yeah my concern is editors that you can’t just update by pulling a repo are going to be broken until they can be updated (like rubymine).

@knu
Copy link
Contributor Author

knu commented Nov 14, 2020

An easiest fix is to add 2>&1 to the command line, and we can inform that or submit PRs to the developers of those known plugins.

https://docs.rubocop.org/rubocop/integration_with_other_tools.html

@knu
Copy link
Contributor Author

knu commented Nov 14, 2020

Would it better to keep the behavior and add a new option, like --stderr for example?

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 14, 2020

I think so. Might also be a good idea to invite some of the plugin authors to share their perspective on this.

@knu knu changed the title Redirect offence reports to stderr when autocorrecting stdin Add an option to redirect offence reports to stderr Nov 15, 2020
@knu
Copy link
Contributor Author

knu commented Nov 15, 2020

OK, I changed so. A new option --stderr redirects all output except for the autocorrected source to stderr.

@tas50
Copy link
Contributor

tas50 commented Nov 15, 2020

Thanks for changing this to be an option. That'll make things a lot easier for those of us that parse rubocop output in editor plugins / other tools

@knu knu force-pushed the improve_autocorrect_stdin branch 3 times, most recently from 0bbe428 to 3d6038f Compare November 15, 2020 07:48
@bbatsov
Copy link
Collaborator

bbatsov commented Nov 17, 2020

I think you should add some section to the docs explaining the use-case for this option, otherwise I doubt people will make use of it.

This makes it easier to use --stdin in shell scripts, editor
integration, CI workflow, etc. by eliminating the need to process the
output.
@knu
Copy link
Contributor Author

knu commented Nov 19, 2020

I updated the description of this option mentioning it is useful when combined with --auto-correct and --stdin.

@bbatsov bbatsov merged commit a3817c8 into rubocop:master Nov 20, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented Nov 20, 2020

Thanks!

@knu knu deleted the improve_autocorrect_stdin branch November 20, 2020 12:03
knu added a commit to knu/standard that referenced this pull request Feb 12, 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.

None yet

4 participants