Laszlo Ersek
2022-Feb-09 13:17 UTC
[Libguestfs] [guestfs-tools PATCH] virt-resize: replace "wrap" calls with calls to "info"
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
Richard W.M. Jones
2022-Feb-09 13:36 UTC
[Libguestfs] [guestfs-tools PATCH] virt-resize: replace "wrap" calls with calls to "info"
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.g30247aa5d201Yes 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