mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-09 18:02:19 +00:00
d0bcf56bd9
## Summary
As noted in a code TODO, our `Diff` rendering code previously didn't
have any
special handling for notebooks. This was particularly obvious when the
diffs
were rendered right next to the corresponding diagnostic because the
diagnostic
used cell-based line numbers, while the diff was still using line
numbers from
the concatenated source. This PR updates the diff rendering to handle
notebooks
too.
The main improvements shown in the example below are:
- Line numbers are now remapped to be relative to their cell
- Context lines from other cells are suppressed
```
error[unused-import][*]: `math` imported but unused
--> notebook.ipynb:cell 2:2:8
|
1 | # cell 2
2 | import math
| ^^^^
3 |
4 | print('hello world')
|
help: Remove unused import: `math`
ℹ Safe fix
1 1 | # cell 2
2 |-import math
3 2 |
4 3 | print('hello world')
```
I tried a few different approaches here before finally just splitting
the notebook into separate text ranges by cell and diffing each one
separately. It seems to work and passes all of our tests, but I don't
know if it's actually enforced anywhere that a single edit doesn't span
cells. Such an edit would silently be dropped right now since it would
fail the `contains_range` check. I also feel like I may have overlooked
an existing way to partition a file into cells like this.
## Test Plan
Existing notebook tests, plus a new one in `ruff_db`
69 lines
2.5 KiB
Rust
69 lines
2.5 KiB
Rust
use serde::{Deserialize, Serialize};
|
|
|
|
use ruff_source_file::{LineColumn, OneIndexed, SourceLocation};
|
|
|
|
/// Jupyter Notebook indexing table
|
|
///
|
|
/// When we lint a jupyter notebook, we have to translate the row/column based on
|
|
/// [`ruff_text_size::TextSize`] to jupyter notebook cell/row/column.
|
|
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
|
|
pub struct NotebookIndex {
|
|
/// Enter a row (1-based), get back the cell (1-based)
|
|
pub(super) row_to_cell: Vec<OneIndexed>,
|
|
/// Enter a row (1-based), get back the row in cell (1-based)
|
|
pub(super) row_to_row_in_cell: Vec<OneIndexed>,
|
|
}
|
|
|
|
impl NotebookIndex {
|
|
pub fn new(row_to_cell: Vec<OneIndexed>, row_to_row_in_cell: Vec<OneIndexed>) -> Self {
|
|
Self {
|
|
row_to_cell,
|
|
row_to_row_in_cell,
|
|
}
|
|
}
|
|
|
|
/// Returns the cell number (1-based) for the given row (1-based).
|
|
pub fn cell(&self, row: OneIndexed) -> Option<OneIndexed> {
|
|
self.row_to_cell.get(row.to_zero_indexed()).copied()
|
|
}
|
|
|
|
/// Returns the row number (1-based) in the cell (1-based) for the
|
|
/// given row (1-based).
|
|
pub fn cell_row(&self, row: OneIndexed) -> Option<OneIndexed> {
|
|
self.row_to_row_in_cell.get(row.to_zero_indexed()).copied()
|
|
}
|
|
|
|
/// Returns an iterator over the row:cell-number pairs (both 1-based).
|
|
pub fn iter(&self) -> impl Iterator<Item = (OneIndexed, OneIndexed)> {
|
|
self.row_to_cell
|
|
.iter()
|
|
.enumerate()
|
|
.map(|(row, cell)| (OneIndexed::from_zero_indexed(row), *cell))
|
|
}
|
|
|
|
/// Translates the given [`LineColumn`] based on the indexing table.
|
|
///
|
|
/// This will translate the row/column in the concatenated source code
|
|
/// to the row/column in the Jupyter Notebook.
|
|
pub fn translate_line_column(&self, source_location: &LineColumn) -> LineColumn {
|
|
LineColumn {
|
|
line: self
|
|
.cell_row(source_location.line)
|
|
.unwrap_or(OneIndexed::MIN),
|
|
column: source_location.column,
|
|
}
|
|
}
|
|
|
|
/// Translates the given [`SourceLocation`] based on the indexing table.
|
|
///
|
|
/// This will translate the line/character in the concatenated source code
|
|
/// to the line/character in the Jupyter Notebook.
|
|
pub fn translate_source_location(&self, source_location: &SourceLocation) -> SourceLocation {
|
|
SourceLocation {
|
|
line: self
|
|
.cell_row(source_location.line)
|
|
.unwrap_or(OneIndexed::MIN),
|
|
character_offset: source_location.character_offset,
|
|
}
|
|
}
|
|
}
|