Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Implement black formatting #143

Closed
jakebailey opened this issue Sep 25, 2018 · 12 comments
Closed

Implement black formatting #143

jakebailey opened this issue Sep 25, 2018 · 12 comments
Labels

Comments

@jakebailey
Copy link
Member

No description provided.

@bzd111
Copy link

bzd111 commented May 16, 2019

I have a question, if file have some empty line in the end , when I save the file,format with two empty lines.

Environment data

VS Code version: 1.33.1
OS and version: macOs High Sierra
Python version (& distribution if applicable, e.g. Anaconda): CPython 3.7.3
Type of virtual environment used (N/A | venv | virtualenv | conda | ...): venv
Relevant/affected Python packages and their versions: black 19.3b0

Actual behavior

Formatting using black leaves two empty lines at the end of the file, if the file has more than 1 empty line already.

Expected behavior

Formatting should leave just one line, and this is what happens when I run black manually from the terminal.

Steps to reproduce:

Open Python file
Insert 10 empty lines at the end
Activate "Format document" with black set as the formatter

@MikhailArkhipov
Copy link

LS does not format documents. Please open issue against extension. Thanks.

@MikhailArkhipov
Copy link

Since 90% of the code is actually managing UI - formatter selection and installation + packaging, there is very little to do in the LS. VS Code extension already implements formatting with formatter selection and we suggest PTVS to do the same something similar. The remaining code is basically save file to temp, call formatter, read the result back.

@pecigonzalo
Copy link

@MikhailArkhipov could you clarify? It is my understand that LSPs implement formatting, that being calling another tool or some other way.
I noticed you closed with the same comment as: #141 referencing PTVS but AFAIK non are implemented.

Is PTVS https://microsoft.github.io/PTVS/ ?

@MikhailArkhipov
Copy link

The point is that LS won't be doing formatting or running external tools. It is up to the host app to do it as there are many tools available and their selection and configuration is at the host app level anyway. On PTVS - it is not using this LS so it is unrelated.

@pecigonzalo
Copy link

pecigonzalo commented Apr 21, 2020

So this LS will not implement that? Would this be contradicting the idea that LSPs implement this functionality and then clients can just consume, providing reusable functionality to all clients?

There are also references like microsoft/vscode-python#3836 suggesting that it will be implemented here.

@jakebailey
Copy link
Member Author

That issue is about linting all files, which is not related to formatting.

Given the popularity of third-party formatters and linters, it would not be productive to reimplement them from scratch here. As for running them, most of the hard work is not running the formatters, it's managing their install, deciding what to do if they aren't installed, picking which one to run if there are many installed, handling the fact that some of these formatters do not support all of the required types of formatting in LSP (selection and line formatting), and providing a graphical UI with buttons to aid in this process. The VS Code extension and PTVS are much, much better suited to do all of these things than the LS is, and have already done that work.

@pecigonzalo
Copy link

@jakebailey Thanks for you reply. It is about linting, but its a similar case, given the linting tools fall on the same description you just gave. You have to install, decide what to do if not installed, etc.

I understand this is already implemented in the VS Code extension, but that does not really answer my question or this issue as AFAIK one of the goals of LS is to manage this linting and formatters as well, so instead of having an implementation on PTVS and one in VSCode and one in Atom, a single LS can manage for all.
At least that is also what I have seen in the LSP and other LS implementations as far as I recall.

Just to be sure, im not suggesting that we implement a new linter or formatter here, but rather that we interface with the existing ones, as its done in other LSs.

@MikhailArkhipov
Copy link

It depends on the language. If you take, say, C#, there are no other formatters and hence LS implements it. Case with Python is quite different. There are many formatters so implementation has to provide UI for installation, selection, way to set formatter options (like VS Tools | Options) as well as its and activation/running. This is bulk of the work. LS has very few UI facilities, mostly limited to Yes/No prompts. This also not particularly different from TypeScript where Prettier is not actually part of TS LS, and neither is ESLint. They are separate extensions. Technically someone could do the same - implement extension to VS Code that implements just formatting and calls specific formatter.

@jakebailey
Copy link
Member Author

Regardless of these challenges, our current focus is on trying to fix ongoing bugs and performance issues; the current situation with linters and formatters is functional and work to redo it isn't immediately planned.

@pecigonzalo
Copy link

pecigonzalo commented Apr 22, 2020

@MikhailArkhipov Probably C# is one of the few that does not have an external as it is most likely developed using Visual Studio.
Most other langs, provide formatting through a CLI, that being internal or external (Ruby, Golang, Python, Javascript, Typescript, etc).
I understand this can be done at the extension, but I repeat, isnt the point of LSs to avoid having to re-implement in 20 places? (the VSCode, Atom, Sublime, Vim, etc extensions)

@jakebailey
I understand the priority might not be to implement the formatter integration, but I believe this

LS does not format documents. Please open issue against extension. Thanks.

is not completely accurate, even when the formatting is done by third party tools, its part of the LSP to implement a formatting interface.

@mredaelli
Copy link

I understand this can be done at the extension, but I repeat, isnt the point of LSs to avoid having to re-implement in 20 places? (the VSCode, Atom, Sublime, Vim, etc extensions)

Absolutely second this.

Also, I don't see the big configuration problem: other than a flag "enableBlack", "enableFlake", "enableMypy",... nothing should be configurable in the LSP, because the configuration can (and should) be delegated to the various configuration files in the project (so everyone, independently of what tool they use in their editor, see the same errors and warnings).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants