Always print something in the logs

[karthiknadig]

For another issue, this time with the extension iteslf

I wasn't getting any output to the console or any indication that anything is happening while trying to format certain files.

Had to hack in vscode-black-formatter/bundled/formatter/format_server.py to see what's going on
(both LSP_SERVER.show_message_log were added by me):

def _format(
    params: types.DocumentFormattingParams,
) -> Union[List[types.TextEdit], None]:
    """Runs formatter, processes the output, and returns text edits."""
    document = LSP_SERVER.workspace.get_document(params.text_document.uri)
    LSP_SERVER.show_message_log(f"Formatting... {document}")

    
    if utils.is_stdlib_file(document.path) or not is_python(document.source):
        LSP_SERVER.show_message_log(f"{utils.is_stdlib_file(document.path)=}, {not is_python(document.source)=}")
        # Don't format standard library python files. Or, invalid python code
        # or non-python code in case of notebooks
        return None

Got this output:

Formatting... file:///Users/bobronium/dev/py/pythings/tests/conftest.py
utils.is_stdlib_file(document.path)=False, not is_python(document.source)=True

is_python returns False when ast.parse(document.source) fails. But why this check would exist in the first place? Black already does it, and outputs:

black "/Users/bobronium/dev/py/pythings/tests/conftest.py"
error: cannot format /Users/bobronium/dev/py/pythings/tests/conftest.py: Cannot parse: 23:4:     item.add_marker("asyncio")

Oh no! 💥 💔 💥
1 file failed to reformat.

Not only it indicates the error, but also shows the exact line that couldn't be parsed.

My proposal:

1. Never exit without any output in any case. Always tell what's happening, e.g. "Detected standard library, exiting without reformatting...".
2. Drop is_python check and let black do its thing
3. Write something to the status bar when extension is called to indicate that wheels are turning

Originally posted by @Bobronium in #52 (comment)

[karthiknadig]

@Bobronium The is_python check is there to prevent black from raising syntax error on code that is currently being edited or non-python cells in a notebook (see #38). But I like your suggestion to write something in the logs for these scenarios. If you want to provide a PR, we are happy to take it for the logging part.

Write something to the status bar when extension is called to indicate that wheels are turning

This is definitely possible with the Progress APIs for LSP.
We just need to set workDoneProgress: True as the option when we register the Formatting feature @LSP_SERVER.feature
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#documentFormattingOptions

Then in the params handle the progress token and respond accordingly to those params.
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#clientInitiatedProgress

I will file a separate issue for this.

[Bobronium]

The is_python check is there to prevent black from raising syntax error on code that is currently being edited or non-python cells in a notebook (see #38).

I think it's too broad of a fix (which for me looks more like a regression) for a specific use-case.

When I'm trying to format file manually, I'd like to see error if black failed to do it. Actually, same goes with saving a file — I want to know tha the tool that is meant to reformat the file on save failed to do so.

I think we can make it configurable to address #38, but I don't think that silencing errors as a default behaviour is a good practice.

I'm happy to work on PR as well.

[karthiknadig]

The condition stdlib/python check is now split and adds specific logs. It also put the exact syntax error in logs.