Feat formatting

[betaboon]

resolves #24

[betaboon]

@jhossbach i think this is done

[jhossbach]

Overall looks nice, thanks alot! I added a few comments in my review, and here is some additional stuff to keep track:

• Add format option to the README.md
• Default for format? Maybe add "I" as default and disable isort per default? (@ccordoba12 what is your opinion on pylsp-ruff disabling isort and using ruff's isort capabilities per default?)

[betaboon]

* [ ]  Default for `format`? Maybe add `"I"` as default and disable isort per default?

i thought so too. up for discussion i guess.

just adding it to the settings in pylsp_settings would have some implications tho:

• it would prevent the user from deactivating it
• it would get overwritten when the user provides format via config.

if we opt to disable pylsp_isort and autofix I001 or Iwe should inject that directly in run_ruff_format.

[betaboon]

i just pushed changes containing the README-mention of format.

and also the import-formatting.

[jhossbach]

Setting document.source = results[0]["newText"] is not possible, so changed the arguments of some methods to use the document source instead of parsing the whole document. Otherwise this works very nicely now, I am merging this PR.

[betaboon]

@jhossbach thanks for merging.
can we get another release? I'd like to package this for nixos :)

[jhossbach]

I have a bit of cleanup that I want to do before creating a new release as well as #28.
On a sidenote; Should we set the settings.format to overwrite the current list if provided? Otherwise I will always be present as fixable and there is no way to disable it nicely:

python-lsp-ruff/pylsp_ruff/plugin.py

Lines 366 to 368 in 09518aa

	fixable_codes = ["I"]
	if settings.format:
	fixable_codes.extend(settings.format)

should be

 fixable_codes = ["I"] 
 if settings.format: 
     fixable_codes = settings.format

[betaboon]

regarding isort:
tbh i dont see a good reason for users wanting to disable import-sorting through ruff.
i would argue: let's leave it like this and change it once someone asks for it.

regarding release:
do you got an ETA for that?
I'm asking because nixos is getting close to feature-freeze for 23.05 and i would really like to get this in :)

[yaegassy]

Hello, I think the "isort" plugin for pylsp uses formatting, so I assume that's why we're trying to have the same behavior, but just to be sure, I'll mention it as information.

The author of the ruff command provides a language server called ruff-lsp. In ruff-lsp, the "fixAll" and etc... feature is provided as a CodeAction, not as a Formatting.

It was once added as a formatting feature, but has been reverted.

• Revert "Handle for LSP textDocument/formatting request (#64)" astral-sh/ruff-lsp#73

[betaboon]

@yaegassy we're providing fixes and fixAll through codeactions as of #22.

the thought behind this PR is that the user can run fixing during formatting if they chose to do so.
in order to replace pyls-isort we're defaulting to only fix I during formatting.

is that what you're asking about?
or am i missing something here?

[yaegassy]

is that what you're asking about?
or am i missing something here?

@betaboon I have no particular opinion.

[jhossbach]

regarding isort: tbh i dont see a good reason for users wanting to disable import-sorting through ruff. i would argue: let's leave it like this and change it once someone asks for it.

Not necessarily disabling the import-sorting through ruff, but one issue I see is that someone that installed python-lsp-ruff will not have import sorting via format out-of-the-box, one would need to add e.g. extendSelect = {"I"} to the pylsp configuration (the format option just adds the code to the fixable argument to ruff which is ignored if the code itself is not checked for).
How about adding all codes in the format setting to be added as --select=<format codes> --fixable=<format codes> (or just the --select method)? That way formatting would always format those (and only those) codes that are given in the format option.

Regarding the ETA: I can wrap it up today, depending on how long it will take us to make formatting work out-of-the-box in a nice way, either way I want this to play nice before pushing a new release.

[betaboon]

Not necessarily disabling the import-sorting through ruff, but one issue I see is that someone that installed python-lsp-ruff will not have import sorting via format out-of-the-box, one would need to add e.g. extendSelect = {"I"} to the pylsp configuration (the format option just adds the code to the fixable argument to ruff which is ignored if the code itself is not checked for). How about adding all codes in the format setting to be added as --select=<format codes> --fixable=<format codes> (or just the --select method)? That way formatting would always format those (and only those) codes that are given in the format option.

i would actually find that behavior confusing.
my reasoning for that:

• there are three ways to configure ruff: pyproject.toml, ruff.toml, python-lsp-ruff
• thus the user will have a configuration in place what they want ruff to look for
• if the user wants ruff to care about import-sorting they will already have I in their configuration.

[jhossbach]

... That sounds reasonable enough to me. To make it obvious how to always have import-sorting I will provide an additional example config in the README, but otherwise I am fine with that

[betaboon]

To make it obvious how to always have import-sorting I will provide an additional example config in the README, but otherwise I am fine with that

good call.

[jhossbach]

Done :)