Special files fix #56
Conversation
Fix alignment with permission column in case of extra "+" bit Fix README missing --potsf arguments
ls++
Outdated
| $max = 0; | ||
| foreach my $line (split(/^/, $out)) { | ||
| ($perm, $hlink, $user, $group, $size) = split(/\s+/, $line); | ||
| (undef, undef, undef, undef, $size) = split(/\s+/, $line); |
There was a problem hiding this comment.
These variables were never used, so I removed them.
ls++
Outdated
| @@ -362,6 +383,9 @@ sub perm_size_file { | |||
|
|
|||
| sub perm { | |||
| my ($perm) = @_; | |||
| if (substr($perm,-1,1) ne "+"){ | |||
| $perm .= " "; # adding an extra space to align with occasional + | |||
There was a problem hiding this comment.
Didn't want to parse the whole permission column to determine if we needed the extra space. For performance reason, I just add a space by default before the column delimiter. It looks good and centered since there is a space at the beginning of the string anyway.
There was a problem hiding this comment.
shouldn't take much performance, you don't need to parse the permissions just calculate the length of it without spaces and pad everything shorter then the longest one.
I think it would make sense to only show the space when there is need for it.
But maybe just personal feelings as you are totally right with the space at the beginning... no clue... works for me either way 😄
There was a problem hiding this comment.
I wasn't very clear in my comment, but I meant going through each line of the GNU ls output like you did in the get_max_size_len subroutine, which means looping through each line twice just to get the max length. I thought it would be some extra steps for very little benefit since these "+" mask bits are rather uncommon. But I might just be overthinking this (happens often heh).
I'll work on it and add the space only when necessary.
Edit: I've added the extra space only when needed. Thanks. :)
| . fg($c[3], fg('bold', $2)) | ||
| ); | ||
| } | ||
| elsif($size =~ m/^(\d+)/) { | ||
| $size = sprintf("%27s", | ||
| fg($c[7], sprintf("%4d", $1)) | ||
| fg($c[7], sprintf("%*d", $stringlen, $1)) |
There was a problem hiding this comment.
$stringlen is finally used for more than one single thing now. :)
It avoids hardcoding "size" column width and dynamically changes it depending on the maximum size of the "size" string.
So you can still have as little as 4 characters width in directories where files are not special files like shown in the above screenshot.
| @@ -362,6 +387,10 @@ sub perm_size_file { | |||
|
|
|||
| sub perm { | |||
| my ($perm) = @_; | |||
| if (length($perm) < $permlen){ | |||
| # adding an extra space to align with occasional + | |||
| $perm .= " "; | |||
There was a problem hiding this comment.
Extra padding when necessary:
When not needed:
|
Splendid work @glubsy! Merging right away. Thank you, have a cold one! |
|
Thanks! Glad I could help a bit. By the way, I forgot to increment the VERSION. You might want to do that to avoid breaking downstream. ;) |
|
Would be awesome to get the DESTDIR pull request applied before new release, would really aid packaging for distro :)
|
|
Both remarks fixed! Thanks guys. |
Fixes reading size and mtime columns in directories holding special character or block files #50
Fixes misaligned permission column in case the extra "+" mask bit is set on a file.
Adds missing --potsf command line argument to README file
Example screenshot:
Shorter arguments work just fine too (wasn't working before this patch):
