Visual Studio Code format on save doesn't work with --diff

[Krischtopp]

I would like Visual Studio Code to format the file with Darker on save, but it does nothing with Darker when I'm using the --diff argument. I'm using "editor.formatOnSave": true in settings.json and it works fine with Black.

If you choose to make this work, there is also the "editor.formatOnSaveMode": "modifications" setting. I would find it quite nice if it was possible to use this in Visual Studio Code instead of the --diff argument.

[akaihola]

Thanks @Krischtopp for your bug report!

Do you want Darker to modify the files on the disk? If that's the case, you can't use the --diff argument. As darker --help states:

  --diff                Don't write the files back, just output a diff for
                        each file on stdout. Highlight syntax on screen if the
                        `pygments` package is available.

Or is Visual Studio Code supposed to read the diff from the output of Darker (or Black) and apply the changes itself? In that case the reason may be the slightly different output format between Darker and Black – and for that there's already pull request #84 which I'm waiting for someone to test and review.

If you're willing to help by testing and reviewing PRs, please accept my invitation to be a collaborator on this project, and I'll add you as a reviewer for #84.

[akaihola]

@Carreau do you have an insight about this issue?

[Carreau]

No I'm not sure I went back to vim :-) if I have time I'll look into it. One of the difficulty is darker need access to the repo to get the changes, and not all vs code hooks allow that maybe ?

[akaihola]

What about @virtuald, IIRC you were trying out Darker with Visual Studio Code as well? Any insight on this issue?

[akaihola]

Oh, and @DylanYoung tried to get Darker working with VSCode as well. Does the problem @Krischtopp describes sound familiar to you?

[Krischtopp]

@akaihola Thank you very much for your invite.
Unfortunately I don't know whether Visual Studio Code wants a formatter to write files or output the formatted code.

I tried some more things. Beforehand I emptied .editorconfig, setup.cfg or any other config files, that might influence darker's/black's behavior.

• "editor.formatOnSave": true doesn't work with darker, with or without --diff.
• The Format Document command in Visual Studio Code works fine with darker, with or without --diff.
• I also installed the Make --diff output identical to Black #84 branch with pip install git+https://github.com/akaihola/darker.git@git-diff-format-parity and it didn't change the above behavior.

[akaihola]

Thanks @Krischtopp for your investigation! If I understand correctly, your conclusion is that Visual Studio Code somehow expects different behavior from the external formatter (in this case Darker) depending on whether formatting happens as the result of the explicit Format Document command or automaticaly when saving a document. Is that correct?

Have you verified that VS Code actually calls Darker on save? Or could it be that it uses some kind of an internal formatter instead?

One thing to note is that currently Darker always expects to read the source code to be reformatted from the disk. It can't e.g. accept Python code on the standard input and provide a reformatted version on its standard output. Could it be that VS Code calls Black in that mode?

Try e.g. this to see how the stdin/out mode of Black works:

echo "print(  1  )" | black -

[Krischtopp]

I had a look at the Visual Studio Code Python extension repository and how they call the black formatter: https://github.com/microsoft/vscode-python/blob/main/src/client/formatters/blackFormatter.ts#L43
Black is being called with --diff and --quiet. This explains why black and darker behave the same with or without --diff.

And I found something that looks like what happens on formatOnSave: https://github.com/microsoft/vscode-python/blob/main/src/client/providers/formatProvider.ts#L112
This leaves me rather confused, because Visual Studio Code just calls the Format Document command, nothing special going on there. I don't know why it behaves differently with darker.

[akaihola]

Thanks @Krischtopp for your continued investigation! As I'm not currenly a VS Code user, I'd like to leave figuring this out to you and other VS Code users (I mentioned some in previous comments).

Just an idea: maybe a good way to debug this is to modify Darker to dump its command line (or the parsed options in args in main()) to a file, try to run with those options manually and see where things start going wrong.

Also, keep in mind that Darker looks for defaults for its configuration options in pyproject.toml, and command line arguments can then be used to override those.

[akaihola]

For Darker 1.3.0, you can apply this patch:

diff --git a/src/darker/__main__.py b/src/darker/__main__.py
index bc497a3..8ab0a2b 100644
--- a/src/darker/__main__.py
+++ b/src/darker/__main__.py
@@ -279,6 +279,7 @@ def main(argv: List[str] = None) -> int:
     """
     if argv is None:
         argv = sys.argv[1:]
+    print(Path.cwd(), "$", " ".join(sys.argv), file=open("/tmp/darker.log", "a"))
     args, config, config_nondefault = parse_command_line(argv)
     logging.basicConfig(level=args.log_level)
     if args.log_level == logging.INFO:

You can then tail -f /tmp/darker.log and look at how VSCode calls Darker when saving files or issuing the Format Document command.

[akaihola]

I have this in VSCode settings:

    "python.formatting.blackPath": "/home/kaiant/.local/bin/darker",
    "python.formatting.provider": "black",
    "python.formatting.blackArgs": [
        "--revision=master...",
        "--isort"
    ],

and when I save a file, I see this in /tmp/darker.log:

/home/akaihola/darker $ /home/akaihola/.local/bin/darker --revision=master... --isort --diff --quiet /home/akaihola/darker/src/darker/__main__.py.7b5da28ec78040aee3d8d0048e94a4a6.tmp

and modified lines do get reformatted in VSCode.

[Krischtopp]

Thank you for looking into this again! I updated darker to 1.3.0 and applied your patch.
I am running Visual Studio Code 1.60.2 and these are my settings:

{
    "workbench.colorTheme": "Default Light+",
    "emmet.includeLanguages": {
        "django-html": "html"
    },
    "workbench.tree.indent": 16,
    "workbench.editorAssociations": {
        "*.ipynb": "jupyter-notebook"
    },
    "notebook.cellToolbarLocation": {
        "default": "right",
        "jupyter-notebook": "left"
    },
    "python.formatting.blackPath": "/home/christoph/.pyenv/shims/darker",
    "python.formatting.provider": "black",
    "python.formatting.blackArgs": [
        "--revision=master...",
        "--isort"
    ],
    "editor.formatOnSave": true
}

When I save an edited file, nothing gets reformatted, and I see this in the logs (edit: wrapped lines for clarity by @akaihola):

/home/christoph/projects/darker $ /home/christoph/.pyenv/versions/3.7.10/bin/darker \
  --revision=master... \
  --isort \
  --diff \
  --quiet \
  /home/christoph/projects/darker/src/darker/__main__.py.d468e4cbc4fb7be3a4707495ec2941de.tmp

When I save an unedited file, reformatting works as expected, and I see this in the logs:

/home/christoph/projects/darker $ /home/christoph/.pyenv/versions/3.7.10/bin/darker \
  --revision=master... \
  --isort \
  --diff \
  --quiet \
  /home/christoph/projects/darker/src/darker/__main__.py

As a quick sanity check, I saved an edited JSON file and reformatting worked as expected.

[akaihola]

@Krischtopp, I believe this revealed the issue. I took the liberty of wrapping lines in your log entries above, and – d'uh, of course! – Darker refuses to process .tmp files, only looking at .py files.

Two possible way to fix this in Darker:

• add *.py.*.tmp to the pattern of included files
• only apply the filter to paths which point to directories

[Krischtopp]

I did some debugging and had a look at the code of Darker and Black.

darker/src/darker/git.py

Lines 142 to 143 in d269159

	def should_reformat_file(path: Path) -> bool:
	return path.exists() and path.suffix == ".py"

Darker filters for *.py files. I'm not sure whether this is necessary at all. gen_python_files already filters for filename extensions.

darker/src/darker/black_diff.py

Lines 138 to 148 in d269159

	gen_python_files(
	(root / path for path in paths),
	root,
	include=DEFAULT_INCLUDE_RE,
	exclude=black_config.get("exclude", DEFAULT_EXCLUDE_RE),
	extend_exclude=black_config.get("extend_exclude"),
	force_exclude=black_config.get("force_exclude"),
	report=Report(),
	gitignore=None,
	**kwargs,
	)

Darker applies gen_python_files to all files and therefore also filters *.tmp files.

https://github.com/psf/black/blob/7b153936587e17b9db992a2d8c8b6cfba3ef7209/src/black/__init__.py#L537-L551
Black applies gen_python_files only to dirs. I think the idea of Black here is that there could be Python code in any file and if the user wants Black to reformat a specific file, then the user wants Black to reformat the file regardless of its filename extension.

Darker doesn't behave like Black in this case, and only reformats the file if it has a matching filename extension. I don't know if there's a reason for that, but to me, it seems like Black's behavior is preferable.

One could probably add *.py.*.tmp to the include argument of gen_python_files to fix the issue with Visual Studio Code, but to me, it seems like that would just be a workaround to an underlying issue.

There should be one more issue here. The *.py.*.tmp file of Visual Studio Code is a new file which is not in Git, and would need to be diffed against the corresponding *.py file in Git. Otherwise, Darker would just reformat the whole file. I didn't test this, but I suspect this is the case.

[akaihola]

@Krischtopp, FYI, I've started to work on this issue. Thanks for your additional notes as well – especially diffing the .py.*.tmp against the corresponding .py is of course essential!