Exclude Regex is not working for formatOnSave

[kirankumarmanku]

Hi,
It's very easy to repro this. Say, you have this setting in your for autopep8.args:

 "autopep8.args": [
    "--exclude='*.py'",
  ]

Ideally, this shouldn't let autopep8 format on save any Python files. However, when I save a Python file, it's not respecting this setting and ending up formatting the file.

This is the log trace:

2023-08-06 11:37:59.009 [info] [Trace - 11:37:59 AM] Sending request 'textDocument/formatting - (1)'.
2023-08-06 11:37:59.009 [info] Params: {
    "textDocument": {
        "uri": "file:///Users/kirankumarmanku/test.py"
    },
    "options": {
        "tabSize": 4,
        "insertSpaces": true
    }
}


2023-08-06 11:37:59.024 [info] [Trace - 11:37:59 AM] Received notification 'window/logMessage'.
2023-08-06 11:37:59.024 [info] Params: {
    "type": 4,
    "message": "/opt/homebrew/opt/python@3.9/bin/python3.9 -m autopep8 --exclude='*.py' -"
}

I debugged this and found that we are not passing the path to autopep8 command at this line:

vscode-autopep8/bundled/tool/lsp_server.py

Line 97 in df3e688

result = _run_tool_on_document(document, use_stdin=True)

by setting use_stdin=True.

This is a bug and should be fixed since autopep8 supports excluding specific files and we should pass complete path while doing autoformat.

Is there any work around for this issue?

[karthiknadig]

@kirankumarmanku This is a limitation of autopep8 itself. We need to be able to format an unsaved file buffer. That means that we need to pass the contents of the file using stdin to autopep8. This is need to support format on save. I don't see a way to pass in the path to the file for reference in autopep8. Do let me know if there is such an option.

autopep8 --help
usage: autopep8 [-h] [--version] [-v] [-d] [-i] [--global-config filename]
                [--ignore-local-config] [-r] [-j n] [-p n] [-a]
                [--experimental] [--exclude globs] [--list-fixes]
                [--ignore errors] [--select errors] [--max-line-length n] 
                [--line-range line line] [--hang-closing] [--exit-code]   
                [files [files ...]]

Automatically formats Python code to conform to the PEP 8 style guide.    

positional arguments:
  files                 files to format or '-' for standard in

optional arguments:
  -h, --help            show this help message and exit
  --version             show program's version number and exit
  -v, --verbose         print verbose messages; multiple -v result in more
                        verbose messages
  -d, --diff            print the diff for the fixed source
  -i, --in-place        make changes to files in place
  --global-config filename
                        path to a global pep8 config file; if this file does
                        not exist then this is ignored (default:
                        C:\Users\kanadig\.pep8)
  --ignore-local-config
                        don't look for and apply local config files; if not
                        passed, defaults are updated with any config files in
                        the project's root directory
  -r, --recursive       run recursively over directories; must be used with
                        --in-place or --diff
  -j n, --jobs n        number of parallel jobs; match CPU count if value is
                        less than 1
  -p n, --pep8-passes n
                        maximum number of additional pep8 passes (default:
                        infinite)
  -a, --aggressive      enable non-whitespace changes; multiple -a result in
                        more aggressive changes
  --experimental        enable experimental fixes
  --exclude globs       exclude file/directory names that match these comma-
                        separated globs
  --list-fixes          list codes for fixes; used by --ignore and --select
  --ignore errors       do not fix these errors/warnings (default:
                        E226,E24,W50,W690)
  --select errors       fix only these errors/warnings (e.g. E4,W)
  --max-line-length n   set maximum allowed line length (default: 79)
  --line-range line line, --range line line
                        only fix errors found within this inclusive range of
                        line numbers (e.g. 1 99); line numbers are indexed at
                        1
  --hang-closing        hang-closing option passed to pycodestyle
  --exit-code           change to behavior of exit code. default behavior of
                        return value, 0 is no differences, 1 is error exit.
                        return 2 when add this option. 2 is exists
                        differences.

For example black has this CLI argument:

black --help
Usage: black [OPTIONS] SRC ...

  The uncompromising code formatter.

Options:
  --stdin-filename TEXT           The name of the file when passing it through
                                  stdin. Useful to make sure Black will
                                  respect --force-exclude option on some
                                  editors that rely on using stdin.

[kirankumarmanku]

Is it possible to work around the limitation of autopep8? Basically, since we are doing this only for single file, we can run the command with passing the file name and if we don't get any matching files or empty response from autopep8, then we ignore format on save and return. Otherwise, we can execute formatOnSave logic.

[kirankumarmanku]

Or, better yet, we can save the file and send the file name as an argument to autopep8? Autopep8 will take care of formatting the file and save it if the file has changes or ignore the file if it it matches exclude regex.

[karthiknadig]

@kirankumarmanku With VS Code save takes on several steps, when a user triggers save, there are the things that can happen based on code actions on save configuration. Following things are run one after the other, on the unsaved file buffer: Import organization (think isort), Quick fixes from linter (think pylint/flake8/ruff), Formatting or source changes (think autopep8/docformatter). Each tool is sent the content of the file, and we get a response back with updated content that is sent to the next tool.

If we send the path to autopep8 and it forces a save to disk, then the content that editor and other tools are expecting vs the one that got saved will be inconsistent. We want to prevent doing multiple passes for a single save as this can severely affect performance on save.

[kirankumarmanku]

@karthiknadig , I see. Is it possible to defer running autopep8 until last? I mean, we can run all other tools, save the file and trigger autopep8 run as the last step.

[karthiknadig]

Is it possible to defer running autopep8 until last?

Unfortunately, no.

If you need this for some scenario, you could disable format on save, and use task.json to set this up.

[kirankumarmanku]

@karthiknadig, I didn't understand how we can use task.json to disable format on save for specific files. Is there any example?

[kirankumarmanku]

@karthiknadig ,
I can create a quick PR by checking this function used in autopep8 if we have --exclude option in the settings file and return early without formatting. What do you think?

[karthiknadig]

I didn't understand how we can use task.json to disable format on save for specific files. Is there any example?

I meant disable the setting manually and use task.json to run autopep8 in command line.

I can create a quick PR by checking this function used in autopep8 if we have --exclude option in the settings file and return early without formatting. What do you think?

we are open to review PRs solving this.

[kirankumarmanku]

@karthiknadig ,
I created a branch and I am trying to push it to your repo to create a PR but github is not allowing me to. What are the guidelines to create a PR? I couldn't find information in contributing guidelines either.

[kirankumarmanku]

@karthiknadig ,
I have created a PR from my fork: #142
Please review and let me know if this solves the problem.

[kirankumarmanku]

@karthiknadig, when can I expect this update to go live in extension?

[karthiknadig]

If you want to test this immediately you can use the build here: https://github.com/microsoft/vscode-autopep8/suites/14963376853/artifacts/853474846

Insiders build will be available for this usually by next day morning Pacific time. The fix will get into stable after about a month in insiders.