Skip to content

Correctly format files and ranges with line endings other than LF #28

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 3 commits into from
Mar 17, 2022
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
2 changes: 1 addition & 1 deletion .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: [3.6, 3.7, 3.8, 3.9, "3.10"]
python-version: [3.7, 3.8, 3.9, "3.10"]
steps:
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
Expand Down
23 changes: 21 additions & 2 deletions pylsp_black/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
import black
import toml
from pylsp import hookimpl
from pylsp._utils import get_eol_chars
Copy link
Collaborator

@haplo haplo Mar 17, 2022

Choose a reason for hiding this comment

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

I wish the module was pylsp.utils and not _utils, but not much we can do about that at this point.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right. We inherited that from the old Palantir project and now it's used everywhere in our fork.


logger = logging.getLogger(__name__)


GLOBAL_CONFIG: Optional[Path] = None
try:
if os.name == "nt":
Expand Down Expand Up @@ -68,8 +70,25 @@ def format_text(*, text, config):
string_normalization=not config["skip_string_normalization"],
)
try:
# will raise black.NothingChanged, we want to bubble that exception up
return black.format_file_contents(text, fast=config["fast"], mode=mode)
# Black's format_file_contents only works reliably when eols are '\n'. It gives
# an error for '\r' and produces wrong formatting for '\r\n'. So we replace
# those eols by '\n' before formatting and restore them afterwards.
replace_eols = False
eol_chars = get_eol_chars(text)
if eol_chars is not None and eol_chars != "\n":
replace_eols = True
text = text.replace(eol_chars, "\n")

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

# Restore eols if necessary.
if replace_eols:
formatted_text = formatted_text.replace("\n", eol_chars)

return formatted_text
except (
# raised when the file has syntax errors
ValueError,
Expand Down
4 changes: 2 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ classifiers =

[options]
packages = find:
install_requires = python-lsp-server; black>=19.3b0; toml
python_requires = >= 3.6
install_requires = python-lsp-server>=1.4.0; black>=19.3b0; toml
python_requires = >= 3.7

[options.entry_points]
pylsp = pylsp_black = pylsp_black.plugin
Expand Down
4 changes: 4 additions & 0 deletions tests/fixtures/formatted-crlf.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
if True:
print("foo")

print("bar") # noqa
4 changes: 4 additions & 0 deletions tests/fixtures/unformatted-crlf.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
if True:
print("foo")

print("bar") # noqa
32 changes: 32 additions & 0 deletions tests/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ def unformatted_pyi_document(workspace):
return Document(uri, workspace)


@pytest.fixture
def unformatted_crlf_document(workspace):
path = fixtures_dir / "unformatted-crlf.py"
uri = f"file:/{path}"
with open(path, "r", newline="") as f:
source = f.read()
return Document(uri, workspace, source=source)


@pytest.fixture
def formatted_document(workspace):
path = fixtures_dir / "formatted.txt"
Expand All @@ -53,6 +62,15 @@ def formatted_pyi_document(workspace):
return Document(uri, workspace)


@pytest.fixture
def formatted_crlf_document(workspace):
path = fixtures_dir / "formatted-crlf.py"
uri = f"file:/{path}"
with open(path, "r", newline="") as f:
source = f.read()
return Document(uri, workspace, source=source)


@pytest.fixture
def invalid_document(workspace):
path = fixtures_dir / "invalid.txt"
Expand Down Expand Up @@ -225,3 +243,17 @@ def test_entry_point():

module = entry_point.load()
assert isinstance(module, types.ModuleType)


def test_pylsp_format_crlf_document(unformatted_crlf_document, formatted_crlf_document):
result = pylsp_format_document(unformatted_crlf_document)

assert result == [
{
"range": {
"start": {"line": 0, "character": 0},
"end": {"line": 4, "character": 0},
},
"newText": formatted_crlf_document.source,
}
]