dense-analysis / ale

Check syntax in Vim/Neovim asynchronously and fix files, with Language Server Protocol (LSP) support
BSD 2-Clause "Simplified" License
13.51k stars 1.43k forks source link

Fix: Ruff output option #4632

Closed adam-blackwater closed 10 months ago

adam-blackwater commented 11 months ago

Hi. I noticed that one of the hardcoded options for the python linter Ruff is out of date and was causing an "unexpected argument" error in the output when the linter first starts.

I have updated the argument so that it matches the newest documentation. Instead of --format text the option is now --output-format text.

Output before fix is shown below:

(finished - exit code 2) ['/usr/bin/zsh', '-c', 'cd ''/home/adam'' && ''ruff'' --format text --stdin-filename ''/home/adam/test.py'' - < ''/tmp/vlnWVAc/1/test.py'''] 

<<<OUTPUT STARTS>>>
error: unexpected argument '--format' found

  tip: to pass '--format' as a value, use '-- --format'

Usage: ruff check <FILES|--fix|--no-fix|--unsafe-fixes|--no-unsafe-fixes|--show-source|--no-show-source|--show-fixes|--no-show-fixes|--diff|--watch|--fix-only|--no-fix-only|--ignore-noqa|--output-format <OUTPUT_FORMAT>|--output-file <OUTPUT_FILE>|--target-version <TARGET_VERSION>|--preview|--no-preview|--config <CONFIG>|--select <RULE_CODE>|--ignore <RULE_CODE>|--extend-select <RULE_CODE>|--extend-ignore <RULE_CODE>|--per-file-ignores <PER_FILE_IGNORES>|--extend-per-file-ignores <EXTEND_PER_FILE_IGNORES>|--exclude <FILE_PATTERN>|--extend-exclude <FILE_PATTERN>|--fixable <RULE_CODE>|--unfixable <RULE_CODE>|--extend-fixable <RULE_CODE>|--extend-unfixable <RULE_CODE>|--respect-gitignore|--no-respect-gitignore|--force-exclude|--no-force-exclude|--line-length <LINE_LENGTH>|--dummy-variable-rgx <DUMMY_VARIABLE_RGX>|--no-cache|--isolated|--cache-dir <CACHE_DIR>|--stdin-filename <STDIN_FILENAME>|--exit-zero|--exit-non-zero-on-fix|--statistics|--add-noqa|--show-files|--show-settings|--ecosystem-ci>

For more information, try '--help'.
<<<OUTPUT ENDS>>>

Output after fix is shown below:

(finished - exit code 0) ['/usr/bin/zsh', '-c', 'cd ''/home/adam'' && ''ruff'' --output-format text --stdin-filename ''/home/adam/test.py'' - < ''/tmp/vHGHOMk/1/test.py''']

<<<NO OUTPUT RETURNED>>>

I hope this helps :)

wmvanvliet commented 11 months ago

This does indeed fix the latest ruff. Do we want to keep supporting the older versions too? The ones that still have --format?

Hritik14 commented 11 months ago

ruff is a new evolving tool, we should stick to the latest version.

wmvanvliet commented 11 months ago

ruff is a new evolving tool, we should stick to the latest version.

While indeed young and evolving, ruff is already very popular (it being so much faster than flake8) and used as the default linter in many projects. This is the current version of it on various platforms: https://repology.org/project/ruff-python-linter/versions

If ALE smooths over version discrepancies for other linting tools, than it is probably worth doing this for ruff as well. If ALE doesn't, then it shouldn't for ruff either.

dtrifiro commented 11 months ago

Can confirm this fixes #4633

doc-sheet commented 11 months ago

ruff released 4 patch versions in those 2 weeks. I think there is no point in keeping compatibility with "alpha" 0.0.x

bodograumann commented 11 months ago

Anything still blocking this from being merged?

hoIIer commented 10 months ago

Can we merge this?

wmvanvliet commented 10 months ago

I think the issue is whether we want to keep support for earlier versions of Ruff (i.e. the if-statement on version number). Perhaps @w0rp can make a quick executive decision here? The code itself is good and ready to merge.

bodograumann commented 10 months ago

I don't see why this would be a blocker to merge this PR as it stands right now. We can always remove support for older versions later.

w0rp commented 10 months ago

ruff released 4 patch versions in those 2 weeks. I think there is no point in keeping compatibility with "alpha" 0.0.x

People get locked into all kinds of weird versions and I like to not break things when it's easy not to break things.

matthewarmand commented 3 months ago

Crossposting this question here since more folks involved in the project are on this PR. ALE hasn't had a new version in about 18 months, are there plans for a new release to be made to get this fix out (as well as anything else done in that time)? https://github.com/dense-analysis/ale/issues/4633#issuecomment-2191821583