ls: fix for gnu test case quote-align compatibility #6555
Conversation
|
GNU testsuite comparison: |
matrixhead
force-pushed
the
quote-align
branch
2 times, most recently
from
31daa7f
to
f9141fb
Compare
|
GNU testsuite comparison: |
|
one thing I noticed while writing tests is that when |
|
GNU testsuite comparison: |
…the name contains any of them.
|
GNU testsuite comparison: |
There was a problem hiding this comment.
The comments I made make me slightly more confident in #6559. I added the patch to fix the quote-align test to it.
Another strong point of 6559 is the number of tests I added. This should help us to avoid regressions in the future.
| pub fn escape_name( | ||
| name: &OsStr, | ||
| style: &QuotingStyle, | ||
| additional_escaped_chars: &[char], | ||
| ) -> String { |
There was a problem hiding this comment.
Because escape_name is used in many places, and its additional_escaped_chars argument is acually used only once, I'd like to avoid having to add &[] to all the other function calls.
The way I did it in #6559 was just to rename the function to escape_name_inner, which actually take the parameter, and escape_name just calls escape_name_inner with &[].
That way, you can create escape_dirname which calls escape_name_inner with &[':'].
| pub fn escape_name( | ||
| name: &OsStr, | ||
| style: &QuotingStyle, | ||
| additional_escaped_chars: &[char], |
There was a problem hiding this comment.
As a broader remark, I decided to go with a simple boolean for dirname ?, rather than a list of escaped characters, because I don't think there are other edge cases.
Yet that absence of other edge cases might be wrong, so I don't really know what to choose here
This pr tries to solve #6554