mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-30 13:51:16 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Move diff rendering to ruff_db (#20006)

Browse source 
Summary
--

This is a preparatory PR in support of #19919. It moves our `Diff`
rendering code from `ruff_linter` to `ruff_db`, where we have direct
access to the `DiagnosticStylesheet` used by our other diagnostic
rendering code. As shown by the tests, this shouldn't cause any visible
changes. The colors aren't exactly the same, as I note in a TODO
comment, but I don't think there's any existing way to see those, even
in tests.

The `Diff` implementation is mostly unchanged. I just switched from a
Ruff-specific `SourceFile` to a `DiagnosticSource` (removing an
`expect_ruff_source_file` call) and updated the `LineStyle` struct and
other styling calls to use `fmt_styled` and our existing stylesheet.

In support of these changes, I added three styles to our stylesheet:
`insertion` and `deletion` for the corresponding diff operations, and
`underline`, which apparently we _can_ use, as I hoped on Discord. This
isn't supported in all terminals, though. It worked in ghostty but not
in st for me.

I moved the `calculate_print_width` function from the now-deleted
`diff.rs` to a method on `OneIndexed`, where it was available everywhere
we needed it. I'm not sure if that's desirable, or if my other changes
to the function are either (using `ilog10` instead of a loop). This does
make it `const` and slightly simplifies things in my opinion, but I'm
happy to revert it if preferred.

I also inlined a version of `show_nonprinting` from the
`ShowNonprinting` trait in `ruff_linter`:


f4be05a83b/crates/ruff_linter/src/text_helpers.rs (L3-L5)

This trait is now only used in `source_kind.rs`, so I'm not sure it's
worth having the trait or the macro-generated implementation (which is
only called once). This is obviously closely related to our unprintable
character handling in diagnostic rendering, but the usage seems
different enough not to try to combine them.


f4be05a83b/crates/ruff_db/src/diagnostic/render.rs (L990-L998)

We could also move the trait to another crate where we can use it in
`ruff_db` instead of inlining here, of course.

Finally, this PR makes `TextEmitter` a very thin wrapper around a
`DisplayDiagnosticsConfig`. It's still used in a few places, though,
unlike the other emitters we've replaced, so I figured it was worth
keeping around. It's a pretty nice API for setting all of the options on
the config and then passing that along to a `DisplayDiagnostics`.

Test Plan
--

Existing snapshot tests with diffs
This commit is contained in:
Brent Westbrook 2025-08-21 09:47:00 -04:00 • committed by GitHub
parent 14fe1228e7
commit 692be72f5a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 266 additions and 230 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/ty_server/src/server/api/requests/completion.rs
View file

@ -6,6 +6,7 @@ use lsp_types::{
CompletionItem, CompletionItemKind, CompletionParams, CompletionResponse, Documentation, Url,
};
use ruff_db::source::{line_index, source_text};
use ruff_source_file::OneIndexed;
use ty_ide::completion;
use ty_project::ProjectDatabase;
use ty_python_semantic::CompletionKind;
@ -59,7 +60,8 @@ impl BackgroundDocumentRequestHandler for CompletionRequestHandler {
return Ok(None);
}
let max_index_len = completions.len().saturating_sub(1).to_string().len();
// Safety: we just checked that completions is not empty.
let max_index_len = OneIndexed::new(completions.len()).unwrap().digits().get();
let items: Vec<CompletionItem> = completions
.into_iter()
.enumerate()