Skip to content
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

Range formatting support #373

Merged
merged 3 commits into from
Feb 5, 2024
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
65 changes: 62 additions & 3 deletions ruff_lsp/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
TEXT_DOCUMENT_DID_SAVE,
TEXT_DOCUMENT_FORMATTING,
TEXT_DOCUMENT_HOVER,
TEXT_DOCUMENT_RANGE_FORMATTING,
AnnotatedTextEdit,
ClientCapabilities,
CodeAction,
Expand All @@ -49,6 +50,8 @@
DidSaveNotebookDocumentParams,
DidSaveTextDocumentParams,
DocumentFormattingParams,
DocumentRangeFormattingParams,
DocumentRangeFormattingRegistrationOptions,
Hover,
HoverParams,
InitializeParams,
Expand All @@ -63,13 +66,16 @@
NotebookDocumentSyncOptionsNotebookSelectorType2CellsType,
OptionalVersionedTextDocumentIdentifier,
Position,
PositionEncodingKind,
Range,
TextDocumentEdit,
TextDocumentFilter_Type1,
TextEdit,
WorkspaceEdit,
)
from packaging.specifiers import SpecifierSet, Version
from pygls import server, uris, workspace
from pygls.workspace.position_codec import PositionCodec
from typing_extensions import Literal, Self, TypedDict, assert_never

from ruff_lsp import __version__, utils
Expand Down Expand Up @@ -140,6 +146,7 @@ class VersionModified(NamedTuple):
# Require at least Ruff v0.0.291 for formatting, but allow older versions for linting.
VERSION_REQUIREMENT_FORMATTER = SpecifierSet(">=0.0.291")
VERSION_REQUIREMENT_LINTER = SpecifierSet(">=0.0.189")
VERSION_REQUIREMENT_RANGE_FORMATTING = SpecifierSet(">=0.2.1")
# Version requirement for use of the `--output-format` option
VERSION_REQUIREMENT_OUTPUT_FORMAT = SpecifierSet(">=0.0.291")
# Version requirement after which Ruff avoids writing empty output for excluded files.
Expand Down Expand Up @@ -1209,14 +1216,47 @@ async def apply_format(arguments: tuple[TextDocument]):

@LSP_SERVER.feature(TEXT_DOCUMENT_FORMATTING)
async def format_document(params: DocumentFormattingParams) -> list[TextEdit] | None:
return await _format_document_impl(params, None)


@LSP_SERVER.feature(
TEXT_DOCUMENT_RANGE_FORMATTING,
DocumentRangeFormattingRegistrationOptions(
document_selector=[
TextDocumentFilter_Type1(language="python", scheme="file"),
TextDocumentFilter_Type1(language="python", scheme="untitled"),
],
ranges_support=False,
work_done_progress=False,
),
)
async def format_document_range(
params: DocumentRangeFormattingParams,
) -> list[TextEdit] | None:
return await _format_document_impl(
DocumentFormattingParams(
params.text_document, params.options, params.work_done_token
),
params.range,
)


async def _format_document_impl(
params: DocumentFormattingParams, range: Range | None
) -> list[TextEdit] | None:
# For a Jupyter Notebook, this request can only format a single cell as the
# request itself can only act on a text document. A cell in a Notebook is
# represented as a text document. The "Notebook: Format notebook" action calls
# this request for every cell.
document = Document.from_cell_or_text_uri(params.text_document.uri)

settings = _get_settings_by_document(document.path)

result = await _run_format_on_document(document, settings)
# We don't support range formatting of notebooks yet but VS Code
# doesn't seem to respect the document filter. For now, format the entire cell.
range = None if document.kind is DocumentKind.Cell else range
Comment on lines +1255 to +1257
Copy link
Member

Choose a reason for hiding this comment

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

This is unfortunate because as a user I wouldn't expect that "Format selection" in Jupyter notebook would format an entire cell. I'm not sure what the right solution is but I'd prefer to send a info / warning message to the user stating that range formatting isn't yet supported for Jupyter Notebooks instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree but I wasn't able to come up with something better. I'm worried that a warning would be rather annoying and force users to change their setting from modificationsIfAvailable to the entire document or they get annoyed by the warning on every save


result = await _run_format_on_document(document, settings, range)
if result is None:
return None

Expand Down Expand Up @@ -1375,6 +1415,7 @@ def _fixed_source_to_edits(
fixed_source = "".join(fixed_source)

new_source = _match_line_endings(original_source, fixed_source)

if new_source == original_source:
return []

Expand Down Expand Up @@ -1855,14 +1896,19 @@ async def _run_check_on_document(


async def _run_format_on_document(
document: Document, settings: WorkspaceSettings
document: Document, settings: WorkspaceSettings, format_range: Range | None = None
) -> ExecutableResult | None:
"""Runs the Ruff `format` subcommand on the given document source."""
if settings.get("ignoreStandardLibrary", True) and document.is_stdlib_file():
log_warning(f"Skipping standard library file: {document.path}")
return None

executable = _find_ruff_binary(settings, VERSION_REQUIREMENT_FORMATTER)
version_requirement = (
VERSION_REQUIREMENT_FORMATTER
if format_range is None
else VERSION_REQUIREMENT_RANGE_FORMATTING
)
executable = _find_ruff_binary(settings, version_requirement)
argv: list[str] = [
"format",
"--force-exclude",
Expand All @@ -1871,6 +1917,19 @@ async def _run_format_on_document(
document.path,
]

if format_range:
codec = PositionCodec(PositionEncodingKind.Utf16)
format_range = codec.range_from_client_units(
document.source.splitlines(True), format_range
)

argv.extend(
[
"--range",
f"{format_range.start.line + 1}:{format_range.start.character + 1}-{format_range.end.line + 1}:{format_range.end.character + 1}", # noqa: E501
]
)

for arg in settings.get("format", {}).get("args", []):
if arg in UNSUPPORTED_FORMAT_ARGS:
log_to_output(f"Ignoring unsupported argument: {arg}")
Expand Down
52 changes: 50 additions & 2 deletions tests/test_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from ruff_lsp.server import (
VERSION_REQUIREMENT_FORMATTER,
VERSION_REQUIREMENT_RANGE_FORMATTING,
Document,
_fixed_source_to_edits,
_get_settings_by_document,
Expand Down Expand Up @@ -41,7 +42,7 @@ async def test_format(tmp_path, ruff_version: Version):
)

with handle_unsupported:
result = await _run_format_on_document(document, settings)
result = await _run_format_on_document(document, settings, None)
assert result is not None
assert result.exit_code == 0
[edit] = _fixed_source_to_edits(
Expand Down Expand Up @@ -74,6 +75,53 @@ async def test_format_code_with_syntax_error(tmp_path, ruff_version: Version):
)

with handle_unsupported:
result = await _run_format_on_document(document, settings)
result = await _run_format_on_document(document, settings, None)
assert result is not None
assert result.exit_code == 2


@pytest.mark.asyncio
async def test_format_range(tmp_path, ruff_version: Version):
original = """x = 1



print( "Formatted")

print ("Not formatted")
"""

expected = """print("Formatted")\n"""

test_file = tmp_path.joinpath("main.py")
test_file.write_text(original)
uri = utils.as_uri(str(test_file))

workspace = Workspace(str(tmp_path))
document = Document.from_text_document(workspace.get_text_document(uri))
settings = _get_settings_by_document(document.path)

handle_unsupported = (
pytest.raises(RuntimeError, match=f"Ruff .* required, but found {ruff_version}")
if not VERSION_REQUIREMENT_RANGE_FORMATTING.contains(ruff_version)
else nullcontext()
)

with handle_unsupported:
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
result = await _run_format_on_document(
document,
settings,
Range(
start=Position(line=1, character=0),
end=(Position(line=4, character=19)),
),
)
assert result is not None
assert result.exit_code == 0
[edit] = _fixed_source_to_edits(
original_source=document.source, fixed_source=result.stdout.decode("utf-8")
)
assert edit.new_text == expected
assert edit.range == Range(
start=Position(line=3, character=0), end=Position(line=5, character=0)
)
Loading