Skip to content

Fix character split when strip code #37

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 2 commits into from
Aug 24, 2020
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
17 changes: 15 additions & 2 deletions src/formatter/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{
cmp,
fmt::{self, Display, Write},
iter::once,
};

pub mod style;
Expand Down Expand Up @@ -217,19 +218,30 @@ impl<'a> DisplayList<'a> {
} else {
false
};
// Specifies that it will end on the next character, so it will return
// until the next one to the final condition.
let mut ended = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

can you document this variable? It's a bit unique.

I also feel like there must be a way to make it simpler. The range.0 is always the result of acc.0, so basically the first take(), and the range.1 should be chainable Option on last element of take_while so if you put the iterator as a mutable variable, you should be able to do iter.next().or(last).

The refactor is optional, and I'm happy to land this as-is with a comment explaining the variable and its role.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ended specifies that it will end in the next character so it will return until the next one to the final condition. In order to measure this last character, when end condition is true, in an efficient way.

I will try to document it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I understand the role, but I had to run an iterator in my head to derive to that conclusion so a doc string would help.

let range = text
.char_indices()
.skip(left)
// Complete char iterator with final character
.chain(once((text.len(), '\0')))
// Take until the next one to the final condition
.take_while(|(_, ch)| {
// Fast return to iterate over final byte position
if ended {
return false;
}
// Make sure that the trimming on the right will fall within the terminal width.
// FIXME: `unicode_width` sometimes disagrees with terminals on how wide a `char` is.
// For now, just accept that sometimes the code line will be longer than desired.
taken += unicode_width::UnicodeWidthChar::width(*ch).unwrap_or(1);
if taken > right - left {
return false;
ended = true;
}
true
})
// Reduce to start and end byte position
.fold((None, 0), |acc, (i, _)| {
if acc.0.is_some() {
(acc.0, i)
Expand All @@ -238,7 +250,8 @@ impl<'a> DisplayList<'a> {
}
});

text[range.0.expect("One character at line")..=range.1].fmt(f)?;
// Format text with margins
text[range.0.expect("One character at line")..range.1].fmt(f)?;

if cut_right {
// We have stripped some code after the right-most span end, make it clear we did so.
Expand Down
25 changes: 25 additions & 0 deletions tests/fixtures/no-color/strip_line_char.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
[title]
id = "E0308"
label = "mismatched types"
annotation_type = "Error"

[[slices]]
source = " let _: () = 42ñ"
line_start = 4
origin = "$DIR/whitespace-trimming.rs"

[[slices.annotations]]
label = "expected (), found integer"
annotation_type = "Error"
range = [192, 194]

[opt]
color = false
anonymized_line_numbers = true
[opt.margin]
whitespace_left = 180
span_left = 192
span_right = 194
label_right = 221
column_width = 140
max_line_len = 195
6 changes: 6 additions & 0 deletions tests/fixtures/no-color/strip_line_char.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error[E0308]: mismatched types
--> $DIR/whitespace-trimming.rs:4:193
|
LL | ... let _: () = 42ñ
| ^^ expected (), found integer
|