Skip to content

Use new lines option in Black 23.11 to format range #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions pylsp_black/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,29 +64,34 @@ def pylsp_settings():


def format_document(client_config, document, range=None):
text = document.source
config = load_config(document.path, client_config)
# Black lines indices are "1-based and inclusive on both ends"
lines = [(range["start"]["line"] + 1, range["end"]["line"])] if range else ()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Black says "indices are 1-based and inclusive on both ends" (https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#line-ranges), range uses a 0-based index.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also black says lines is a list of tuple but uses a tuple as default argument in https://github.com/psf/black/pull/4020/files.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Black says "indices are 1-based and inclusive on both ends" (https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#line-ranges), range uses a 0-based index.

Can you add this as a comment in the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added.


try:
formatted_text = format_text(text=text, config=config, lines=lines)
except black.NothingChanged:
# raised when the file is already formatted correctly
return []

if range:
formatted_lines = formatted_text.splitlines(True)

start = range["start"]["line"]
end = range["end"]["line"]
text = "".join(document.lines[start:end])
end = range["end"]["line"] + (len(formatted_lines) - len(document.lines))

formatted_text = "".join(formatted_lines[start:end])
else:
text = document.source
range = {
"start": {"line": 0, "character": 0},
"end": {"line": len(document.lines), "character": 0},
}

config = load_config(document.path, client_config)

try:
formatted_text = format_text(text=text, config=config)
except black.NothingChanged:
# raised when the file is already formatted correctly
return []

return [{"range": range, "newText": formatted_text}]


def format_text(*, text, config):
def format_text(*, text, config, lines):
mode = black.FileMode(
target_versions=config["target_version"],
line_length=config["line_length"],
Expand All @@ -107,7 +112,7 @@ def format_text(*, text, config):

# Will raise black.NothingChanged, we want to bubble that exception up
formatted_text = black.format_file_contents(
text, fast=config["fast"], mode=mode
text, fast=config["fast"], mode=mode, lines=lines
)

# Restore eols if necessary.
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ classifiers =
packages = find:
install_requires =
python-lsp-server>=1.4.0
black>=22.3.0
black>=23.11.0
tomli; python_version<'3.11'
python_requires = >= 3.8

Expand Down
15 changes: 14 additions & 1 deletion tests/fixtures/formatted.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,15 @@
a = "hello"
b = 42
b = [
"a",
"very",
"very",
"very",
"very",
"very",
"very",
"very",
"very",
"long",
"line",
]
c = 42
3 changes: 2 additions & 1 deletion tests/fixtures/unformatted.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
a = 'hello'
b = 42
b = ["a", "very", "very", "very", "very", "very", "very", "very", "very", "long", "line"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that long but long enough for black :) I added this line in the middle of the script to make sure we can format a selection in the middle of the text, that results in more lines in the final text.

c = 42
17 changes: 15 additions & 2 deletions tests/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def test_pylsp_format_document(config, unformatted_document, formatted_document)
{
"range": {
"start": {"line": 0, "character": 0},
"end": {"line": 2, "character": 0},
"end": {"line": 3, "character": 0},
},
"newText": formatted_document.source,
}
Expand Down Expand Up @@ -204,7 +204,20 @@ def test_pylsp_format_document_with_config(config, config_document):

@pytest.mark.parametrize(
("start", "end", "expected"),
[(0, 0, 'a = "hello"\n'), (1, 1, "b = 42\n"), (0, 1, 'a = "hello"\nb = 42\n')],
[
(0, 0, 'a = "hello"\n'),
(
1,
1,
'b = [\n "a",\n "very",\n "very",\n "very",\n "very",\n "very",\n "very",\n "very",\n "very",\n "long",\n "line",\n]\n', # noqa: E501
),
(2, 2, "c = 42\n"),
(
0,
2,
'a = "hello"\nb = [\n "a",\n "very",\n "very",\n "very",\n "very",\n "very",\n "very",\n "very",\n "very",\n "long",\n "line",\n]\nc = 42\n', # noqa: E501
),
],
)
def test_pylsp_format_range(config, unformatted_document, start, end, expected):
range = {
Expand Down