Skip to content
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

ls: incorrect column widths in -R mode #961

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

mknos
Copy link
Contributor

@mknos mknos commented Feb 25, 2025

  • The column widths for "ls -R" didn't always look right
  • List() is called for each directory/subdirectory
  • Global variable $Maxlen is increased by 1 unconditionally, despite the value being saved from the previous call
  • When I tested "perl ls -R .." I observed $Maxlen values from 21-59, incrementing by 1 for each directory
  • The entries for the subdirectories are printed with too much space
  • When I compare the output of GNU ls, the column widths are calculated for each directory listed
  • Fix this by making $Maxlen a non-global; it is only used in List() and the value between calls should not be saved
  • Extra: remove unused variable $BlockSize
%# BEFORE: maxlen of ../bin takes value from previous .. then adds 1
%perl ls -R ..
maxlen[21]
..:
CNAME                 LICENSE-BSD2          Makefile.PL           data                  util
CONTRIBUTING.md       LICENSE-GPL2          PROGRAMMING_STYLE.md  lib                   xt
Changes               MANIFEST              README.pod            packed                
INSTALL.SKIP          MANIFEST.SKIP         TODO                  packer                
LICENSE-ARTISTIC2     META.yml              bin                   t                     

maxlen[22]
../bin:
YES.diff               dc                     ln                     pwd                    txt1
_die.log               deroff                 lock                   rain                   uname
addbib                 diff                   look                   random                 unexpand
apply                  dirname                ls                     rev                    uniq
ar                     du                     mail                   rm                     units
arch                   echo                   maze                   rmdir                  unlink
arithmetic             ed                     mimedecode             rnd.bin                unpar
asa                    env                    mkdir                  robots                 unshar
awk                    expand                 mkfifo                 rot13                  uudecode
banner                 expr                   moo                    seq                    uuencode
base64                 factor                 morse                  shar                   wc
basename               false                  morse2                 sleep                  what
bc                     file                   names                  sort                   which
bcd                    find                   nl                     spell                  whoami
cal                    fish                   od                     split                  whois
cat                    fmt                    par                    strings                words
chgrp                  fold                   paste                  sum                    wump
chgrp.orig             fortune                patch                  tac                    xargs
ching                  from                   perldoc                tail                   yes
chmod                  fuzz_pike.sh           perlpowertools         tar                    yes.ORig
chown                  glob                   pig                    tee                    yes.Orig
clear                  grep                   ping                   test                   yes.diff
cmp                    hangman                pom                    test.pl                yes.orig
col                    head                   ppt                    time                   yes.rej
colrm                  hexdump                pr                     touch                  yes2
comm                   id                     prices                 tr                     
cp                     install                primes                 true                   
cut                    join                   printenv               tsort                  
date                   kill                   printf                 tty                    
<SNIP>

%# AFTER: maxlen of "../bin" is less than for "..", which means we can fit more text columns
%perl ls -R ..
maxlen[21]
..:
CNAME                 LICENSE-BSD2          Makefile.PL           data                  util
CONTRIBUTING.md       LICENSE-GPL2          PROGRAMMING_STYLE.md  lib                   xt
Changes               MANIFEST              README.pod            packed                
INSTALL.SKIP          MANIFEST.SKIP         TODO                  packer                
LICENSE-ARTISTIC2     META.yml              bin                   t                     

maxlen[15]
../bin:
YES.diff        clear           find            maze            primes          tail            wc
_die.log        cmp             fish            mimedecode      printenv        tar             what
addbib          col             fmt             mkdir           printf          tee             which
apply           colrm           fold            mkfifo          pwd             test            whoami
ar              comm            fortune         moo             rain            test.pl         whois
arch            cp              from            morse           random          time            words
arithmetic      cut             fuzz_pike.sh    morse2          rev             touch           wump
asa             date            glob            names           rm              tr              xargs
awk             dc              grep            nl              rmdir           true            yes
banner          deroff          hangman         od              rnd.bin         tsort           yes.ORig
base64          diff            head            par             robots          tty             yes.Orig
basename        dirname         hexdump         paste           rot13           txt1            yes.diff
bc              du              id              patch           seq             uname           yes.orig
bcd             echo            install         perldoc         shar            unexpand        yes.rej
cal             ed              join            perlpowertools  sleep           uniq            yes2
cat             env             kill            pig             sort            units           
chgrp           expand          ln              ping            spell           unlink          
chgrp.orig      expr            lock            pom             split           unpar           
ching           factor          look            ppt             strings         unshar          
chmod           false           ls              pr              sum             uudecode        
chown           file            mail            prices          tac             uuencode        

* The column widths for "ls -R" didn't always look right
* List() is called for each directory/subdirectory
* Global variable $Maxlen is increased by 1 unconditionally, despite the value being saved from the previous call
* When I tested "perl ls -R .." I observed $Maxlen values from 21-59, incrementing by 1 for each directory
* The entries for the subdirectories are printed with too much space
* When I compare the output of GNU ls, the column widths are calculated for each directory listed
* Fix this by making $Maxlen a non-global; it is only used in List() and the value between calls should not be saved
* Extra: remove unused variable $BlockSize
@github-actions github-actions bot added Type: enhancement improve a feature that already exists Priority: low get to this whenever Program: ls The ls program labels Feb 25, 2025
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:33 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 00:34 — with GitHub Actions Inactive
@coveralls
Copy link

coveralls commented Feb 25, 2025

Pull Request Test Coverage Report for Build 13510878103

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 72.349%

Totals Coverage Status
Change from base Build 13503527620: 0.0%
Covered Lines: 348
Relevant Lines: 481

💛 - Coveralls

@briandfoy briandfoy self-assigned this Feb 25, 2025
@briandfoy briandfoy merged commit 555d2a6 into briandfoy:master Feb 25, 2025
23 checks passed
@github-actions github-actions bot added the Status: accepted The fix is accepted label Feb 25, 2025
@github-actions github-actions bot added the Status: accepted The fix is accepted label Feb 25, 2025
@briandfoy briandfoy added Type: bug an existing feature does not work and removed Type: enhancement improve a feature that already exists Priority: low get to this whenever labels Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Program: ls The ls program Status: accepted The fix is accepted Type: bug an existing feature does not work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants