[Libguestfs] [guestfs-tools PATCH] virt-resize: replace "wrap" calls with calls to "info"

[Richard W.M. Jones]

On Wed, Feb 09, 2022 at 02:17:38PM +0100, Laszlo Ersek wrote:
> Utilities shouldn't directly call "Std_utils.wrap" for printing
> informative messages; the "Tools_utils.info" function handles that better.
> 
> Because "info" prints a trailing newline automatically, strip one newline
> from the originally wrapped messages. While at it, rewrap (in the source
> code) the "Resize operation completed with no errors" message, for better
> readability.
> 
> The patch introduces some visible (but, arguably, correct) changes to the
> output:
> 
> > virt-resize: /dev/sda1: This partition will be resized from 1023.9M to
> > 2.0G.  The filesystem ext4 on /dev/sda1 will be expanded using the
> > ‘resize2fs’ method.
> > [...]
> > virt-resize: Resize operation completed with no errors.  Before deleting
> > the old disk, carefully check that the resized disk boots and works
> > correctly.
> 
> These messages now carry the "virt-resize: " prefix, and they are printed
> in magenta.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1820221
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  resize/resize.ml | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/resize/resize.ml b/resize/resize.ml
> index dad453ff99b7..b77e680d49c8 100644
> --- a/resize/resize.ml
> +++ b/resize/resize.ml
> @@ -938,7 +938,7 @@ read the man page virt-resize(1).
>                ""
>              ) in
>  
> -      wrap (text ^ "\n\n") in
> +      info "%s" (text ^ "\n") in
>  
>      List.iter print_summary partitions;
>  
> @@ -969,7 +969,7 @@ read the man page virt-resize(1).
>                  ""
>                ) in
>  
> -            wrap (text ^ "\n\n")
> +            info "%s" (text ^ "\n")
>      ) lvs;
>  
>      if surplus > 0L then (
> @@ -983,7 +983,7 @@ read the man page virt-resize(1).
>          ) else
>            s_"  The surplus space will be ignored.  Run a partitioning program in the guest to partition this extra space if you want." in
>  
> -      wrap (text ^ "\n\n")
> +      info "%s" (text ^ "\n")
>      );
>  
>      printf "**********\n";
> @@ -1440,7 +1440,9 @@ read the man page virt-resize(1).
>  
>    if not (quiet ()) then (
>      print_newline ();
> -    wrap (s_"Resize operation completed with no errors.  Before deleting the old disk, carefully check that the resized disk boots and works correctly.\n");
> +    info "%s" (s_"Resize operation completed with no errors.  Before deleting \
> +               the old disk, carefully check that the resized disk boots and \
> +               works correctly.");
>    )
>  
>  let () = run_main_and_handle_errors main
> 
> base-commit: 7d5d5e921d3d483a997f40566c1ccabf8a689a8a
> -- 
> 2.19.1.3.g30247aa5d201

Yes this is fine, ACK.

Should we also hide the wrap function?  From a very quick look at the
code it seems we could move the function from mlstdutils/std_utils.ml
to mltools/tools_utils.ml, and of course drop the public interface in
mlstdutils/std_utils.mli.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW