pr: fix header formatting for custom date formats starting with '+' #9252
Conversation
|
GNU testsuite comparison: |
tests/by-util/test_pr.rs
Outdated
| let mut scenario = new_ucmd!(); | ||
|
|
||
| // Set a specific date format like in the GNU test | ||
| let output = scenario |
There was a problem hiding this comment.
A detail: I would omit the scenario variable as I don't see the benefit of it.
| let mut scenario = new_ucmd!(); | |
| // Set a specific date format like in the GNU test | |
| let output = scenario | |
| // Set a specific date format like in the GNU test | |
| let output = new_ucmd!() |
src/uu/pr/src/pr.rs
Outdated
| /// Returns a five line header content if displaying header is not disabled by | ||
| /// using `NO_HEADER_TRAILER_OPTION` option. | ||
| fn header_content(options: &OutputOptions, page: usize) -> Vec<String> { | ||
| if options.display_header_and_trailer { |
There was a problem hiding this comment.
As you are rewriting this function anyway, it might make sense to do an early return if no header should be shown. It would allow you to get rid of an indentation level for the rest of the function.
src/uu/pr/src/pr.rs
Outdated
| let filename_len = filename.chars().count(); | ||
| let page_len = page_part.chars().count(); | ||
|
|
||
| let first_line = if date_len + filename_len + page_len + 2 < total_width { |
There was a problem hiding this comment.
first_line is misleading as it is the third line in the header. Maybe header_line, like in the test, would be a better name?
src/uu/pr/src/pr.rs
Outdated
| let first_line = if date_len + filename_len + page_len + 2 < total_width { | ||
| // Check if we're using a custom date format that needs centered alignment | ||
| // This preserves backward compatibility while fixing the GNU time-style test | ||
| let needs_centering = date_part.starts_with('+'); | ||
|
|
||
| if needs_centering { |
There was a problem hiding this comment.
I would combine the two conditions. And either use
let first_line = if date_len + filename_len + page_len + 2 < total_width && date_part.starts_with('+') {or use vars:
let first_line = {
let fits = date_len + filename_len + page_len + 2 < total_width;
let needs_centering = fits && date_part.starts_with('+');It allows you to get rid of one of the (duplicate) else cases.
Should fix tests/misc/time-style.sh
CodSpeed Performance ReportMerging #9252 will degrade performances by 2.64%Comparing Summary
Benchmarks breakdown
Footnotes
|
|
GNU testsuite comparison: |
Great, kudos! |
2 participants
Should fix tests/misc/time-style.sh