microsoft / vscode-black-formatter

Formatting support for Python using the Black formatter
https://marketplace.visualstudio.com/items?itemName=ms-python.black-formatter
MIT License
144 stars 34 forks source link

Format with `"editor.formatOnSaveMode": "modificationsIfAvailable"` introduces newlines #521

Closed mattgodbolt closed 1 week ago

mattgodbolt commented 1 week ago

Diagnostic Data

Behaviour

Expected Behavior

The save behaviour should not affect how the file is formatted.

Actual Behavior

With the setting "modificationsIfAvailable" in format on save mode, and the code:

class App:
    def __init__(self, *, name: str, description: str, log_name: str | None = None):
        self._name = name
        self._description = description
        self._log_name = log_name or name

making a change to the constructor line only will cause an extra line to be inserted between class App and def __init__ on save. A "black format" of the whole file does not introduce this line, only if the class App: is unmodified and def __init__ is modified, and then a save action takes place.

Reproduction Steps:

Set the setting as above; make and commit the file to source control without the newline. Make a trivial edit to the def __init__ line and save.

Logs:

Click here for detailed logs 2024-06-21 16:33:21.302 [info] /home/mgodbolt/dev/tradewinds/out/env/bin/python3 -m black --line-ranges 15-15 --line-ranges 18-18 --line-ranges 38-38 --line-ranges 42-43 --line-ranges 60-61 --line-ranges 124-124 --line-ranges 138-138 --line-ranges 140-142 --line-ranges 263-263 --line-ranges 277-277 --line-ranges 302-303 --line-ranges 304-304 --line-ranges 312-315 --stdin-filename /home/mgodbolt/dev/tradewinds/src/py/tradewinds/porthole/apps.py - 2024-06-21 16:33:21.303 [info] CWD formatter: /home/mgodbolt/dev/tradewinds 2024-06-21 16:33:21.392 [info] reformatted /home/mgodbolt/dev/tradewinds/src/py/tradewinds/porthole/apps.py All done! ✨ 🍰 ✨ 1 file reformatted.

Outcome When Attempting Debugging Steps:

Did running it from the command line work? No - command seems to want to read from stdin and so hung when I tried to run it as described above.

Extra Details

n/a

karthiknadig commented 1 week ago

@mattgodbolt This extension is a thin wrapper around black. From the CLI you can see that all we are doing is running black on your behalf. You can see if this is happening directly with black by running it like this using cat:

cat /home/mgodbolt/dev/tradewinds/src/py/tradewinds/porthole/apps.py | /home/mgodbolt/dev/tradewinds/out/env/bin/python3 -m black --line-ranges 15-15 --line-ranges 18-18 --line-ranges 38-38 --line-ranges 42-43 --line-ranges 60-61 --line-ranges 124-124 --line-ranges 138-138 --line-ranges 140-142 --line-ranges 263-263 --line-ranges 277-277 --line-ranges 302-303 --line-ranges 304-304 --line-ranges 312-315 --stdin-filename /home/mgodbolt/dev/tradewinds/src/py/tradewinds/porthole/apps.py

make sure the match the black version in environment with the one shipped with the extension.

mattgodbolt commented 1 week ago

Right, with that I can see I can repro the extra blank lines.

Does this mean it's a black issue?

When you say "make sure [to] match the black version" - how do I do that? Our python environment deliberately installs a black that our CI etc uses:

python -m black, 24.4.2 (compiled: no)

which is picked up from the virtual env we run python from. Is this the problem?

karthiknadig commented 1 week ago

Does this mean it's a black issue?

Correct.

When you say "make sure [to] match the black version" - how do I do that? Our python environment deliberately installs a black that our CI etc uses

The bundled version is 24.3.0, and when you file on black they will likely tell you to try the latest. Since you are using the latest from your environment you should be fine.

mattgodbolt commented 1 week ago

Ok thank you