Re: [PATCH v2 0/3] More ls improvements

[Glenn Washburn]

Ping. Wondering if this is still in the queue to be looked at.

Glenn

On Sat,  8 Jun 2024 16:58:30 -0500
Glenn Washburn <development@efficientek.com> wrote:

> Currently when given a path to a file, ls will open the file to determine
> if its is valid and then run the appropriate print function, in contrast to
> directory arguments that use the directory iterator and callback on each
> file. One issue with this is that opening a file does not allow access to
> its modification time information, whereas the info object from the callback
> called by the directory iterator does and the longlist print function will
> print the modification time if present. The result is that when longlisting
> ls arguments, directory arguments show moditication times but file arguments
> do not. Patch 1 rectifies this an in the process simplifies the code path
> by using the directory iterator for file arguments as well.
> 
> Patches 2 and 3 aim to make the output of GRUB's ls look more like GNU's
> ls output. And patch 3 also fixes an issue where there are blank lines
> between consecutive file arguments.
> 
> v2:
>  * Remove first patch of v1
>  * Reset grub_errno in grub_ls_list_files instead
> 
> Glenn Washburn (3):
>   commands/ls: Allow printing mtime for file arguments
>   commands/ls: Add directory header for dir args and print full paths
>     for file args
>   commands/ls: Proper line breaks between arguments
> 
>  grub-core/commands/ls.c | 123 ++++++++++++++++++++++++----------------
>  1 file changed, 75 insertions(+), 48 deletions(-)
> 
> Range-diff against v1:
> 1:  c9066a31480c < -:  ------------ disk: Reset grub_errno upon entering 
> grub_disk_read()
> 2:  8947f5257962 ! 1:  187a173d6e8a commands/ls: Allow printing mtime for 
> file arguments
>     @@ grub-core/commands/ls.c: grub_ls_list_files (char *dirname, int 
> longlist, int al
>             if (grub_errno == GRUB_ERR_BAD_FILE_TYPE
>                 && path[grub_strlen (path) - 1] != '/')
>               {
>     -           /* PATH might be a regular file.  */
>     +-          /* PATH might be a regular file.  */
>      -          char *p;
>      -          grub_file_t file;
>      -          struct grub_dirhook_info info;
>     @@ grub-core/commands/ls.c: grub_ls_list_files (char *dirname, int 
> longlist, int al
>      -            goto fail;
>      -
>      -          grub_file_close (file);
>     --
>     ++          /*
>     ++           * Reset errno as it is currently set, but will cause 
> subsequent code
>     ++           * to think there is an error.
>     ++           */
>     ++          grub_errno = GRUB_ERR_NONE;
>     + 
>      -          p = grub_strrchr (dirname, '/') + 1;
>      -          ctx.dirname = grub_strndup (dirname, p - dirname);
>     ++          /* PATH might be a regular file.  */
>      +          ctx.filename = grub_strrchr (dirname, '/') + 1;
>      +          ctx.dirname = grub_strndup (dirname, ctx.filename - dirname);
>                 if (ctx.dirname == NULL)
> 3:  ce655165f508 ! 2:  cd02a1fa4512 commands/ls: Add directory header for dir 
> args and print full paths for file args
>     @@ grub-core/commands/ls.c: grub_ls_list_files (char *dirname, int 
> longlist, int al
>       
>             (fs->fs_dir) (dev, path, print_files, &ctx);
>      @@ grub-core/commands/ls.c: grub_ls_list_files (char *dirname, int 
> longlist, int all, int human)
>     -           && path[grub_strlen (path) - 1] != '/')
>     -         {
>     +           grub_errno = GRUB_ERR_NONE;
>     + 
>                 /* PATH might be a regular file.  */
>      +          ctx.print_dirhdr = 0;
>                 ctx.filename = grub_strrchr (dirname, '/') + 1;
> 4:  1a14220825c2 = 3:  6988321447be commands/ls: Proper line breaks between 
> arguments