Skip to content

Commit

Permalink
ruff.applyFormat now formats an entire notebook document (#11493)
Browse files Browse the repository at this point in the history
## Summary

Previously, `ruff.applyFormat`, seen in VS Code as the command `Ruff:
Format Document`, would only format the currently active notebook cell
inside a notebook document. This PR makes `ruff.applyFormat` format the
entire notebook document at once, operating on each code cell in order.

## Test Plan

1. Open a notebook document that has multiple unformatted code cells.
2. Run `Ruff: Format Document` through the Command Palette
(`Ctrl/Cmd+Shift+P` by default)
3. Observe that all code cells in the notebook have been formatted.
  • Loading branch information
snowsignal committed May 22, 2024
1 parent f0046ab commit 3cb2e67
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 14 deletions.
10 changes: 4 additions & 6 deletions crates/ruff_server/src/server/api/requests/execute_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,10 @@ impl super::SyncRequestHandler for ExecuteCommand {
.with_failure_code(ErrorCode::InternalError)?;
}
Command::Format => {
let response = super::format::format_document(&snapshot)?;
if let Some(edits) = response {
edit_tracker
.set_edits_for_document(uri, version, edits)
.with_failure_code(ErrorCode::InternalError)?;
}
let fixes = super::format::format_full_document(&snapshot)?;
edit_tracker
.set_fixes_for_document(fixes, version)
.with_failure_code(ErrorCode::InternalError)?;
}
Command::OrganizeImports => {
let fixes = super::code_action_resolve::organize_imports_edit(
Expand Down
68 changes: 60 additions & 8 deletions crates/ruff_server/src/server/api/requests/format.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
use crate::edit::{Replacement, ToRangeExt};
use crate::fix::Fixes;
use crate::server::api::LSPResult;
use crate::server::{client::Notifier, Result};
use crate::session::DocumentSnapshot;
use crate::{PositionEncoding, TextDocument};
use lsp_types::{self as types, request as req};
use ruff_python_ast::PySourceType;
use ruff_source_file::LineIndex;
use ruff_workspace::FormatterSettings;
use types::TextEdit;

pub(crate) struct Format;
Expand All @@ -23,25 +27,73 @@ impl super::BackgroundDocumentRequestHandler for Format {
}
}

/// Formats either a full text document or each individual cell in a single notebook document.
pub(super) fn format_full_document(snapshot: &DocumentSnapshot) -> Result<Fixes> {
let mut fixes = Fixes::default();

if let Some(notebook) = snapshot.query().as_notebook() {
for (url, text_document) in notebook
.urls()
.map(|url| (url.clone(), notebook.cell_document_by_uri(url).unwrap()))
{
if let Some(changes) = format_text_document(
text_document,
snapshot.query().source_type(),
snapshot.query().settings().formatter(),
snapshot.encoding(),
true,
)? {
fixes.insert(url, changes);
}
}
} else {
if let Some(changes) = format_text_document(
snapshot.query().as_single_document().unwrap(),
snapshot.query().source_type(),
snapshot.query().settings().formatter(),
snapshot.encoding(),
false,
)? {
fixes.insert(snapshot.query().make_key().into_url(), changes);
}
}

Ok(fixes)
}

/// Formats either a full text document or an specific notebook cell. If the query within the snapshot is a notebook document
/// with no selected cell, this will throw an error.
pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::FormatResponse> {
let doc = snapshot
let text_document = snapshot
.query()
.as_single_document()
.expect("format should only be called on text documents or notebook cells");
let source = doc.contents();
let mut formatted = crate::format::format(
doc,
format_text_document(
text_document,
snapshot.query().source_type(),
snapshot.query().settings().formatter(),
snapshot.encoding(),
snapshot.query().as_notebook().is_some(),
)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
}

fn format_text_document(
text_document: &TextDocument,
source_type: PySourceType,
formatter_settings: &FormatterSettings,
encoding: PositionEncoding,
is_notebook: bool,
) -> Result<super::FormatResponse> {
let source = text_document.contents();
let mut formatted = crate::format::format(text_document, source_type, formatter_settings)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
// fast path - if the code is the same, return early
if formatted == source {
return Ok(None);
}

// special case - avoid adding a newline to a notebook cell if it didn't already exist
if snapshot.query().as_notebook().is_some() {
if is_notebook {
let mut trimmed = formatted.as_str();
if !source.ends_with("\r\n") {
trimmed = trimmed.trim_end_matches("\r\n");
Expand All @@ -57,7 +109,7 @@ pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::Form

let formatted_index: LineIndex = LineIndex::from_source_text(&formatted);

let unformatted_index = doc.index();
let unformatted_index = text_document.index();

let Replacement {
source_range,
Expand All @@ -70,7 +122,7 @@ pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::Form
);

Ok(Some(vec![TextEdit {
range: source_range.to_range(source, unformatted_index, snapshot.encoding()),
range: source_range.to_range(source, unformatted_index, encoding),
new_text: formatted[formatted_range].to_owned(),
}]))
}

0 comments on commit 3cb2e67

Please sign in to comment.