-
-
Notifications
You must be signed in to change notification settings - Fork 817
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
Add a --quote
option to put quotation marks around output files/dirs
#1366
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,43 @@ fn print_trailing_slash<W: Write>( | |
Ok(()) | ||
} | ||
|
||
// Trying to copy: https://www.gnu.org/software/coreutils/quotes.html | ||
fn path_needs_quoting(path: &str) -> i8 { | ||
// If it contains any special chars we return single quote | ||
if path.contains(" ") || path.contains("$") || path.contains("\"") { | ||
return 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be better to use an enumeration instead of an |
||
// If it ONLY contains a ' we return double quote | ||
} else if path.contains("'") { | ||
return 2; | ||
} | ||
|
||
return 0; | ||
} | ||
|
||
// Quote a path with coreutils style quoting to make copy/paste | ||
// more friendly for shells | ||
fn quote_path(path_str: &str) -> String { | ||
let quote_type = path_needs_quoting(path_str); | ||
let mut tmp_str:String = path_str.into(); | ||
|
||
// Quote with single quotes | ||
if quote_type == 1 { | ||
// Escape single quotes in path | ||
tmp_str = str::replace(&tmp_str, "'", "'\\''"); | ||
|
||
format!("'{}'", tmp_str) | ||
// Quote with double quotes | ||
} else if quote_type == 2 { | ||
// Escape double quotes in path | ||
tmp_str = str::replace(&tmp_str, "\"", "\\\""); | ||
|
||
format!("\"{}\"", tmp_str) | ||
// No quoting required | ||
} else { | ||
path_str.to_string() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This unnecessarily copies the string again even if there is no change |
||
} | ||
} | ||
|
||
// TODO: this function is performance critical and can probably be optimized | ||
fn print_entry_colorized<W: Write>( | ||
stdout: &mut W, | ||
|
@@ -62,9 +99,22 @@ fn print_entry_colorized<W: Write>( | |
ls_colors: &LsColors, | ||
) -> io::Result<()> { | ||
// Split the path between the parent and the last component | ||
let mut offset = 0; | ||
let path = entry.stripped_path(config); | ||
let path_str = path.to_string_lossy(); | ||
let mut offset = 0; | ||
let path = entry.stripped_path(config); | ||
let mut path_str = path.to_string_lossy(); | ||
let mut needs_quoting = false; | ||
|
||
// Wrap the path in quotes | ||
if config.use_quoting { | ||
let tmp_str = quote_path(&path_str); | ||
|
||
// If the quoted string is new, then we flag that to tweak the offset | ||
// so the colors line up | ||
if tmp_str != path_str { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has to compare every character of the string. It would be more efficient if you did something like have return if let Some(quoted) = quote_path(&path_str) {
path_str = quoted.into();
needs_quoting = true;
} |
||
path_str = tmp_str.into(); | ||
needs_quoting = true; | ||
} | ||
} | ||
|
||
if let Some(parent) = path.parent() { | ||
offset = parent.to_string_lossy().len(); | ||
|
@@ -78,6 +128,11 @@ fn print_entry_colorized<W: Write>( | |
} | ||
|
||
if offset > 0 { | ||
|
||
if needs_quoting { | ||
offset += 2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is correct. If it is only quoted shouldn't the offset be just 1 more? And if it is double quoted, and has escaped characters, then the proper offset will depend on how many characters in the parent ended up getting escaped. |
||
} | ||
|
||
let mut parent_str = Cow::from(&path_str[..offset]); | ||
if let Some(ref separator) = config.path_separator { | ||
*parent_str.to_mut() = replace_path_separator(&parent_str, separator); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably not careful enough. See https://github.com/tavianator/bfs/blob/b8ed989642b9f0f6c1301bcff6f1498935cbd81c/src/bfstd.c#L906-L918 for example