Skip to content

Commit 829c444

Browse files
9999years9999years
committed
textwrap: don't split words at punctuation by default
The default `textwrap::WordSeparator::UnicodeBreakProperties` provides sensible line breaking for (e.g.) emojis and CJK text. Unfortunately, it also considers punctuation like `/` to be an appropriate location for line breaks. This is fine for normal text, but leads to very bad behavior when attempting to wrap error messages. Here, a file path is broken across multiple lines (with box drawing characters added in between parts of the path as well), making it impossible to copy-paste the path out of the error message: ``` Error: × Failed to read Buck2 event log from `buck2 build //aaa/aaaa` via /var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/ │ T/.tmpBgvlUI/buck-log.jsonl.gz ╰─▶ failed to open file `/var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/.tmpBgvlUI/buck-log.jsonl.gz`: No such file or directory (os error 2) ``` In the future, we may want to write our own line break algorithm that breaks between CJK codepoints and emojis but not at punctuation like slashes. For now, I believe it will be better to break lines at ASCII spaces only. Similar changes are made for some other settings: - The default for `break_words` has been changed to `false` for similar reasons. - The default `textwrap::WordSplitter` has been changed to not split words at existing hyphens, to prevent splits like `--foo-bar` into `--foo-` and `bar`.
1 parent df7bcfa commit 829c444

File tree

1 file changed

+44
-43
lines changed

1 file changed

+44
-43
lines changed

‎src/handlers/graphical.rs‎

Lines changed: 44 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::fmt::{self, Write};
22

33
use owo_colors::{OwoColorize, Style, StyledList};
4+
use textwrap::WordSeparator;
5+
use textwrap::WordSplitter;
46
use unicode_width::{UnicodeWidthChar, UnicodeWidthStr};
57

68
use crate::diagnostic_chain::{DiagnosticChain, ErrorKind};
@@ -61,7 +63,7 @@ impl GraphicalReportHandler {
6163
tab_width: 4,
6264
with_cause_chain: true,
6365
wrap_lines: true,
64-
break_words: true,
66+
break_words: false,
6567
with_primary_span_start: true,
6668
word_separator: None,
6769
word_splitter: None,
@@ -83,7 +85,7 @@ impl GraphicalReportHandler {
8385
wrap_lines: true,
8486
with_cause_chain: true,
8587
with_primary_span_start: true,
86-
break_words: true,
88+
break_words: false,
8789
word_separator: None,
8890
word_splitter: None,
8991
highlighter: MietteHighlighter::default(),
@@ -262,16 +264,16 @@ impl GraphicalReportHandler {
262264
if let Some(footer) = &self.footer {
263265
writeln!(f)?;
264266
let width = self.termwidth.saturating_sub(2);
265-
let mut opts = textwrap::Options::new(width)
267+
let opts = textwrap::Options::new(width)
266268
.initial_indent(" ")
267269
.subsequent_indent(" ")
268-
.break_words(self.break_words);
269-
if let Some(word_separator) = self.word_separator {
270-
opts = opts.word_separator(word_separator);
271-
}
272-
if let Some(word_splitter) = self.word_splitter.clone() {
273-
opts = opts.word_splitter(word_splitter);
274-
}
270+
.break_words(self.break_words)
271+
.word_separator(self.word_separator.unwrap_or(WordSeparator::AsciiSpace))
272+
.word_splitter(
273+
self.word_splitter
274+
.clone()
275+
.unwrap_or(WordSplitter::NoHyphenation),
276+
);
275277

276278
writeln!(f, "{}", self.wrap(footer, opts))?;
277279
}
@@ -340,16 +342,16 @@ impl GraphicalReportHandler {
340342
let initial_indent = format!(" {} ", severity_icon.style(severity_style));
341343
let rest_indent = format!(" {} ", self.theme.characters.vbar.style(severity_style));
342344
let width = self.termwidth.saturating_sub(2);
343-
let mut opts = textwrap::Options::new(width)
345+
let opts = textwrap::Options::new(width)
344346
.initial_indent(&initial_indent)
345347
.subsequent_indent(&rest_indent)
346-
.break_words(self.break_words);
347-
if let Some(word_separator) = self.word_separator {
348-
opts = opts.word_separator(word_separator);
349-
}
350-
if let Some(word_splitter) = self.word_splitter.clone() {
351-
opts = opts.word_splitter(word_splitter);
352-
}
348+
.break_words(self.break_words)
349+
.word_separator(self.word_separator.unwrap_or(WordSeparator::AsciiSpace))
350+
.word_splitter(
351+
self.word_splitter
352+
.clone()
353+
.unwrap_or(WordSplitter::NoHyphenation),
354+
);
353355

354356
writeln!(f, "{}", self.wrap(&diagnostic.to_string(), opts))?;
355357

@@ -386,16 +388,16 @@ impl GraphicalReportHandler {
386388
)
387389
.style(severity_style)
388390
.to_string();
389-
let mut opts = textwrap::Options::new(width)
391+
let opts = textwrap::Options::new(width)
390392
.initial_indent(&initial_indent)
391393
.subsequent_indent(&rest_indent)
392-
.break_words(self.break_words);
393-
if let Some(word_separator) = self.word_separator {
394-
opts = opts.word_separator(word_separator);
395-
}
396-
if let Some(word_splitter) = self.word_splitter.clone() {
397-
opts = opts.word_splitter(word_splitter);
398-
}
394+
.break_words(self.break_words)
395+
.word_separator(self.word_separator.unwrap_or(WordSeparator::AsciiSpace))
396+
.word_splitter(
397+
self.word_splitter
398+
.clone()
399+
.unwrap_or(WordSplitter::NoHyphenation),
400+
);
399401

400402
match error {
401403
ErrorKind::Diagnostic(diag) => {
@@ -428,17 +430,16 @@ impl GraphicalReportHandler {
428430
if let Some(help) = diagnostic.help() {
429431
let width = self.termwidth.saturating_sub(2);
430432
let initial_indent = " help: ".style(self.theme.styles.help).to_string();
431-
let mut opts = textwrap::Options::new(width)
433+
let opts = textwrap::Options::new(width)
432434
.initial_indent(&initial_indent)
433435
.subsequent_indent(" ")
434-
.break_words(self.break_words);
435-
if let Some(word_separator) = self.word_separator {
436-
opts = opts.word_separator(word_separator);
437-
}
438-
if let Some(word_splitter) = self.word_splitter.clone() {
439-
opts = opts.word_splitter(word_splitter);
440-
}
441-
436+
.break_words(self.break_words)
437+
.word_separator(self.word_separator.unwrap_or(WordSeparator::AsciiSpace))
438+
.word_splitter(
439+
self.word_splitter
440+
.clone()
441+
.unwrap_or(WordSplitter::NoHyphenation),
442+
);
442443
writeln!(f, "{}", self.wrap(&help.to_string(), opts))?;
443444
}
444445
Ok(())
@@ -489,16 +490,16 @@ impl GraphicalReportHandler {
489490
.style(severity_style)
490491
.to_string();
491492

492-
let mut opts = textwrap::Options::new(width)
493+
let opts = textwrap::Options::new(width)
493494
.initial_indent(&initial_indent)
494495
.subsequent_indent(&rest_indent)
495-
.break_words(self.break_words);
496-
if let Some(word_separator) = self.word_separator {
497-
opts = opts.word_separator(word_separator);
498-
}
499-
if let Some(word_splitter) = self.word_splitter.clone() {
500-
opts = opts.word_splitter(word_splitter);
501-
}
496+
.break_words(self.break_words)
497+
.word_separator(self.word_separator.unwrap_or(WordSeparator::AsciiSpace))
498+
.word_splitter(
499+
self.word_splitter
500+
.clone()
501+
.unwrap_or(WordSplitter::NoHyphenation),
502+
);
502503

503504
let mut inner = String::new();
504505

0 commit comments

Comments
 (0)