Allow defining black-formatter.interpreter per .vscode folder in multi-root workspace

[lukaspiatkowski]

Currently, the black-formatter.interpreter setting is defined as "window" scope here:

vscode-black-formatter/package.json

Line 119 in e0f7830

"scope": "window",

Because of this in a multi-root workspace setting it is impossible to define interpreters independently per each project. Consider this project layout:

root
├── proj1
│   └── venv
├── proj2
│   └── venv
├── my-workspace.code-workspace
└── venv

And this my-workspace.code-workspace setting:

{
  "folders": [
    {
      "path": "."
    },
    {
      "path": "proj1"
    },
    {
      "path": "proj2"
    }
  ]
}

Now one might want to use
In root -> black-formatter.interpreter == root/venv/bin/python
in proj1 -> black-formatter.interpreter == root/venv/bin/python (Note that the default python interpreter still has to be proj1/venv/bin/python for pylance to work)
in proj2 -> black-formatter.interpreter == root/proj2/venv/bin/python

Right now because black-formatter.interpreter can only be set in .code-workspace there is no way to express this configuration.

A workaround: use black-formatter.path which is declared with scope "resource"

vscode-black-formatter/package.json

Line 86 in e0f7830

"scope": "resource",

This way one might configure:
root/.vscode/settings.json:

{
    "black-formatter.path": ["venv/bin/black"],
}

proj1/.vscode/settings.json:

{
    "black-formatter.path": ["../venv/bin/black"],
}

proj2/.vscode/settings.json:

{
    "black-formatter.path": ["venv/bin/black"],
}

But this doesn't make use of the speedups this extension is supposed to give with not using black binary directly.

[karthiknadig]

@lukaspiatkowski Try with this build: https://github.com/microsoft/vscode-black-formatter/suites/12382500511/artifacts/657959412

[lukaspiatkowski]

@karthiknadig are you sure that is the right link? It is the same you've sent me for the other PR :-) Anyway, I've verified this issue is not fixed by this build.

[karthiknadig]

Sorry, this is the build that allow interpreter setting at resource level:
https://github.com/microsoft/vscode-black-formatter/suites/12533708159/artifacts/669253166

[lukaspiatkowski]

That one doesn't work as well, although the config is not greyed out in that version.

[karthiknadig]

@lukaspiatkowski Can you share the logs, the settings passed in should have the pythons selected for each root? We start only one server even in multi root case. But for each root we launch a sub-server if it uses different python.

Are you using python extension to select the python for the various roots?

[lukaspiatkowski]

Sure, here is the log: https://gist.github.com/lukaspiatkowski/f6a009aabf7c8f29c80402034d51aed4
I've created a special repo just for testing this: https://github.com/lukaspiatkowski/black_formater_test_repo

[karthiknadig]

@lukaspiatkowski I am not sure why it had to be that complicated, here is all that you really need:

{
  "folders": [
    {
      "path": "."
    },
    {
      "path": "proj1"
    },
    {
      "path": "proj2"
    }
  ],
  "settings": {
    "black-formatter.interpreter": ["${workspaceFolder}/.venv/Scripts/python"]
  }
}

These are the settings passed:

2023-05-01 17:28:50.736 [info] Settings used to run Server:
[
    {
        "cwd": "c:\\GIT\\demo\\black_formater_test_repo",
        "workspace": "file:///c%3A/GIT/demo/black_formater_test_repo",
        "args": [],
        "path": [],
        "interpreter": [
            "c:\\GIT\\demo\\black_formater_test_repo/.venv/Scripts/python"
        ],
        "importStrategy": "useBundled",
        "showNotifications": "off"
    },
    {
        "cwd": "c:\\GIT\\demo\\black_formater_test_repo\\proj1",
        "workspace": "file:///c%3A/GIT/demo/black_formater_test_repo/proj1",
        "args": [],
        "path": [],
        "interpreter": [
            "c:\\GIT\\demo\\black_formater_test_repo\\proj1/.venv/Scripts/python"
        ],
        "importStrategy": "useBundled",
        "showNotifications": "off"
    },
    {
        "cwd": "c:\\GIT\\demo\\black_formater_test_repo\\proj2",
        "workspace": "file:///c%3A/GIT/demo/black_formater_test_repo/proj2",
        "args": [],
        "path": [],
        "interpreter": [
            "c:\\GIT\\demo\\black_formater_test_repo\\proj2/.venv/Scripts/python"
        ],
        "importStrategy": "useBundled",
        "showNotifications": "off"
    }
]

2023-05-01 17:28:50.737 [info] Global settings:
{
    "cwd": "C:\\Users\\kanadig\\AppData\\Local\\Programs\\Microsoft VS Code Insiders",
    "workspace": "C:\\Users\\kanadig\\AppData\\Local\\Programs\\Microsoft VS Code Insiders",
    "args": [],
    "path": [],
    "interpreter": [],
    "importStrategy": "useBundled",
    "showNotifications": "off"
}

Here you can see correct environment being picked up:

2023-05-01 17:31:52.988 [info] c:\GIT\demo\black_formater_test_repo/.venv/Scripts/python -m black --stdin-filename c:\GIT\demo\black_formater_test_repo\main.py -
2023-05-01 17:32:31.369 [info] c:\GIT\demo\black_formater_test_repo\proj1/.venv/Scripts/python -m black --stdin-filename c:\GIT\demo\black_formater_test_repo\proj1\main.py -
2023-05-01 17:34:22.407 [info] c:\GIT\demo\black_formater_test_repo\proj2/.venv/Scripts/python -m black --stdin-filename c:\GIT\demo\black_formater_test_repo\proj2\main.py -

You should be able to do this with the relese version, and you don't need separate .vscode folders. Actually, you should not even have to set this manually if you select the python for each workspace from the multi-root selector:

Formatter extension will automatically use the right one for each repo:

[
    {
        "cwd": "c:\\GIT\\demo\\black_formater_test_repo",
        "workspace": "file:///c%3A/GIT/demo/black_formater_test_repo",
        "args": [],
        "path": [],
        "interpreter": [
            "c:\\GIT\\demo\\black_formater_test_repo\\.venv\\Scripts\\python.exe"
        ],
        "importStrategy": "useBundled",
        "showNotifications": "off"
    },
    {
        "cwd": "c:\\GIT\\demo\\black_formater_test_repo\\proj1",
        "workspace": "file:///c%3A/GIT/demo/black_formater_test_repo/proj1",
        "args": [],
        "path": [],
        "interpreter": [
            "c:\\GIT\\demo\\black_formater_test_repo\\proj1\\.venv\\Scripts\\python.exe"
        ],
        "importStrategy": "useBundled",
        "showNotifications": "off"
    },
    {
        "cwd": "c:\\GIT\\demo\\black_formater_test_repo\\proj2",
        "workspace": "file:///c%3A/GIT/demo/black_formater_test_repo/proj2",
        "args": [],
        "path": [],
        "interpreter": [
            "c:\\GIT\\demo\\black_formater_test_repo\\proj2\\.venv\\Scripts\\python.exe"
        ],
        "importStrategy": "useBundled",
        "showNotifications": "off"
    }
]

Nothing set for the interpreter in the code-workspace file:

{
  "folders": [
    {
      "path": "."
    },
    {
      "path": "proj1"
    },
    {
      "path": "proj2"
    }
  ]
}

[lukaspiatkowski]

Well, obviously that is not the repository I am working on daily, it is just a minimal example to reproduce the problem. If you want a more complex one then I've pushed a new commit to that test repository.

Here are full logs: https://gist.github.com/lukaspiatkowski/d4d531d71370d63abbdb5f1a35c1ee5e

What I would expect from settings:

• root using black==22.3.0 from its venv
• proj1 using black==22.3.0 from root venv
• proj2 using black==23.3.0 from bundled

Instead, what I get is:

• ✅ root using black==22.3.0 from its venv
• ❌ proj1 failing to find black, because it looks for it in its own venv where it doesn't exist
• ❌ proj2 using black==22.8.0 from its venv instead of from bundled

[karthiknadig]

There was a bug in the fall back mechanism, but if you have black install in all three envs it should work.

I am going to make a fix for the fallback mechanism, with the fix you should see this:

2023-05-01 23:13:34.419 [info] c:\GIT\demo\black_formater_test_repo\.venv\Scripts\python.exe -m black --version
2023-05-01 23:13:34.604 [info] CWD formatter: c:\GIT\demo\black_formater_test_repo
2023-05-01 23:13:36.017 [info] Version info for formatter running for c:\GIT\demo\black_formater_test_repo:
black, 22.3.0 (compiled: no)

2023-05-01 23:13:36.018 [info] SUPPORTED black>=22.3.0
FOUND black==22.3.0

2023-05-01 23:13:36.021 [info] c:\GIT\demo\black_formater_test_repo\proj1\.venv\Scripts\python.exe -m black --version
2023-05-01 23:13:36.024 [info] CWD formatter: c:\GIT\demo\black_formater_test_repo\proj1
2023-05-01 23:13:36.030 [info] Version info for formatter running for c:\GIT\demo\black_formater_test_repo\proj1:
black, 22.3.0 (compiled: no)

2023-05-01 23:13:36.032 [info] SUPPORTED black>=22.3.0
FOUND black==22.3.0

2023-05-01 23:13:36.035 [info] c:\GIT\demo\black_formater_test_repo\proj2\.venv\Scripts\python.exe -m black --version
2023-05-01 23:13:36.036 [info] CWD formatter: c:\GIT\demo\black_formater_test_repo\proj2
2023-05-01 23:13:36.818 [info] Version info for formatter running for c:\GIT\demo\black_formater_test_repo\proj2:
black, 23.1.0 (compiled: no)
Python (CPython) 3.11.3

2023-05-01 23:13:36.819 [info] SUPPORTED black>=22.3.0
FOUND black==23.1.0

[lukaspiatkowski]

We don't have black installed in all of our projects. In fact we have >10 projects defined on few deepnesses of folders and they all use black from the root venv (that is very convenient to make black consistent in the repository and have only the root venv up-to-date for commit hooks to work) except for one project that uses a newer version of Python and newer version of black.

That is why I've opened this issue, I am not sure which fallback mechanism you are referring to when you say it is broken as the are few things that work inconsistently here. Having all settings behave exactly like black-formatter.path would solve all the problems.

[karthiknadig]

There are two fallback mechanisms. One for single python scenario, and another for multi-python scenario. In multi-python scenario. Each root in the multi-root case gets its own server. All of these are co-ordinated my the main server. The sub-server runner sets up the sys.path based on which import strategy you use. There was a bug in the code that sets it up in the multi python scenario.

[lukaspiatkowski]

Ok, understood. But that fix doesn't really close this issue. Or is it that, based on what you said, you won't implement this feature request?

[karthiknadig]

This issue actually had two separate things,

1. The fallback mechanism in multi root case: fixed here Ensure black is always added as fallback #228
2. The scope issue with interpreter setting: fixed here Use workspace scope when reading interpreter setting #229

[lukaspiatkowski]

Thanks for being so very responsive! I am really impressed by how speedy you were addressing those issues.

For what it is worth, I've downloaded and tried out the vsix from #229 and it works very well. On the repo mentioned above, the logs are here: https://gist.github.com/lukaspiatkowski/9ad051073e889fbb7191c7323bfad513

What I would expect from settings:

• root using black==22.3.0 from its venv
• proj1 using black==22.3.0 from root venv
• proj2 using black==23.3.0 from bundled

Instead, what I get is:

• ✅ root using black==22.3.0 from its venv
• ✅ proj1 using black==22.3.0 from root venv
• ❌ proj2 using black==22.8.0 from its venv instead of from bundled

But to be honest, that last case was purely academical, as we don't need "black-formatter.importStrategy": "fromBundled" to be honored in .vscode.

[lukaspiatkowski]

Oh, BTW, should I open similar issues for other ms-python extensions (I am thinking here about isort and mypy) or have you commited those changes to a common repository and solved it for all of those extensions?

[karthiknadig]

Oh, BTW, should I open similar issues for other ms-python extensions (I am thinking here about isort and mypy) or have you commited those changes to a common repository and solved it for all of those extensions?

I will be propagating these fixes across to all extensions. No need to open issues on all the repos.