Skip to content

LogStorage: fix log limits (lines/length/size) with multi-line log msgs#134

Merged
syphar merged 1 commit into
rust-lang:mainfrom
syphar:log-by-line
May 22, 2026
Merged

LogStorage: fix log limits (lines/length/size) with multi-line log msgs#134
syphar merged 1 commit into
rust-lang:mainfrom
syphar:log-by-line

Conversation

@syphar
Copy link
Copy Markdown
Member

@syphar syphar commented May 21, 2026

coming from rust-lang/docs.rs#3346 I realized that we assume that one log message is only one log line.

But we also get single log-messages with multi-line content.

So this PR goes through the log message line by line.

@syphar syphar self-assigned this May 21, 2026
@syphar syphar requested a review from GuillaumeGomez May 21, 2026 10:36
Comment thread src/logging.rs
Comment on lines +193 to +196
let mut length = max_line_length - 3;
while !message.is_char_boundary(length) {
length -= 1;
}
Copy link
Copy Markdown
Contributor

@Skgland Skgland May 21, 2026

Choose a reason for hiding this comment

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

Suggested change
let mut length = max_line_length - 3;
while !message.is_char_boundary(length) {
length -= 1;
}
let length = message.floor_char_boundary(max_line_length - 3);

Could this use str::floor_char_boundary or is this newer than rustwides msrv?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we currently have 1.89 in there, that method case with 1.91.

I feel like this is not reason enough to push the msrv?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, sounds reasonable to not bump the msrv for just this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm also always wondering how many real-world users outside of the rust org actually exist, so if the msrv la really important to them

@syphar syphar merged commit fb48dc3 into rust-lang:main May 22, 2026
9 checks passed
@syphar syphar deleted the log-by-line branch May 22, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants