Libguestfs - Feb 2022 - [libguestfs-common PATCH 3/3] add common ("standard") option -w / --wrap

Similarly to how users can can force ANSI colorization with "--colors"
even when stdout / stderr are redirected to a non-tty, allow them to force
wrapping with "--wrap".

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1820221
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    - This patch will require documentation updates for the following
      guestfs-tools utilities (those that already document
"--colours"):
    
        builder/virt-builder-repository.pod
        builder/virt-builder.pod
        customize/virt-customize.pod
        dib/virt-dib.pod
        get-kernel/virt-get-kernel.pod
        resize/virt-resize.pod
        sparsify/virt-sparsify.pod
        sysprep/virt-sysprep.pod
    
      Virt-v2v as well:
    
        docs/virt-v2v.pod
    
      This is evident e.g. from "test-virt-customize-docs.sh" failing
in
      guestfs-tools, and "test-v2v-docs.sh" failing in virt-v2v.
    
      If this patch is accepted, I intend to post such guestfs-tools and
      virt-v2v patches that atomically bump the submodule reference and
      update the documentation.
    
    - Some utilities in libguestfs already use the short "-w" option
      (guestfish, guestmount, virt-rescue); however, those utilities do not
      document "--colours", so they already don't make a TTY-based
      distinction, and will not see the new short "-w" option.
    
    - Easily testable e.g. with virt-resize:
    
      $ guestfish -N fs:ext4 exit
      $ truncate -s 2G test1.resized.img
      $ virt-resize    --expand /dev/sda1 test1.img test1.resized.img
      $ virt-resize -w --expand /dev/sda1 test1.img test1.resized.img
      $ virt-resize    --expand /dev/sda1 test1.img test1.resized.img > log1
      $ virt-resize -w --expand /dev/sda1 test1.img test1.resized.img > log2
      $ cat log1
      $ cat log2

 mlstdutils/std_utils.mli | 6 ++++--
 mlstdutils/std_utils.ml  | 8 ++++++--
 mltools/tools_utils.ml   | 3 ++-
 mltools/test-getopt.sh   | 2 ++
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/mlstdutils/std_utils.mli b/mlstdutils/std_utils.mli
index 534caa80d6a7..87e923e24d29 100644
--- a/mlstdutils/std_utils.mli
+++ b/mlstdutils/std_utils.mli
@@ -376,8 +376,10 @@ val set_trace : unit -> unit
 val trace : unit -> bool
 val set_verbose : unit -> unit
 val verbose : unit -> bool
-(** Stores the colours ([--colours]), quiet ([--quiet]), trace ([-x]) and
-    verbose ([-v]) flags in global variables. *)
+val set_wrap : unit -> unit
+val wrap : unit -> bool
+(** Stores the colours ([--colours]), quiet ([--quiet]), trace ([-x]), verbose
+    ([-v]) and wrap ([-w]) flags in global variables. *)
 
 val with_open_in : string -> (in_channel -> 'a) -> 'a
 (** [with_open_in filename f] calls function [f] with [filename]
diff --git a/mlstdutils/std_utils.ml b/mlstdutils/std_utils.ml
index 58ac058c1b3a..52f44760c490 100644
--- a/mlstdutils/std_utils.ml
+++ b/mlstdutils/std_utils.ml
@@ -580,8 +580,8 @@ let which executable  (* Program name. *)
 let prog = Filename.basename Sys.executable_name
 
-(* Stores the colours (--colours), quiet (--quiet), trace (-x) and
- * verbose (-v) flags in a global variable.
+(* Stores the colours (--colours), quiet (--quiet), trace (-x), verbose (-v)
+ * and wrap (-w) flags in a global variable.
  *)
 let colours = ref false
 let set_colours () = colours := true
@@ -599,6 +599,10 @@ let verbose = ref false
 let set_verbose () = verbose := true
 let verbose () = !verbose
 
+let wrap = ref false
+let set_wrap () = wrap := true
+let wrap () = !wrap
+
 let with_open_in filename f    let chan = open_in filename in
   protect ~f:(fun () -> f chan) ~finally:(fun () -> close_in chan)
diff --git a/mltools/tools_utils.ml b/mltools/tools_utils.ml
index 4c5188988e03..eb8da290eec6 100644
--- a/mltools/tools_utils.ml
+++ b/mltools/tools_utils.ml
@@ -126,7 +126,7 @@ let message fs  type wrap_break_t = WrapEOS | WrapSpace |
WrapNL
 
 let rec wrap ?(chan = stdout) ?(indent = 0) str -  if istty chan then
+  if Std_utils.wrap () || istty chan then
     let len = String.length str in
     _wrap chan indent 0 0 len str
   else (
@@ -388,6 +388,7 @@ let create_standard_options argspec ?anon_fun ?(key_opts =
false)
   add_argspec ([ S 'q'; L"quiet" ],   Getopt.Unit set_quiet, 
s_"Don?t print progress messages");
   add_argspec ([ L"color"; L"colors";
                  L"colour"; L"colours" ], Getopt.Unit
set_colours, s_"Use ANSI colour sequences even if not tty");
+  add_argspec ([ S 'w'; L"wrap" ],    Getopt.Unit set_wrap,  
s_"Wrap log messages even if not tty");
 
   if key_opts then (
     let parse_key_selector arg diff --git a/mltools/test-getopt.sh
b/mltools/test-getopt.sh
index d9919a9a915a..1d7bb7b1dc4f 100755
--- a/mltools/test-getopt.sh
+++ b/mltools/test-getopt.sh
@@ -68,6 +68,7 @@ $t --short-options | grep '^-is'
 $t --short-options | grep '^-t'
 $t --short-options | grep '^-V'
 $t --short-options | grep '^-v'
+$t --short-options | grep '^-w'
 $t --short-options | grep '^-x'
 
 # --long-options
@@ -89,6 +90,7 @@ $t --long-options | grep '^--ii'
 $t --long-options | grep '^--is'
 $t --long-options | grep '^--version'
 $t --long-options | grep '^--verbose'
+$t --long-options | grep '^--wrap'
 
 # -a/--add parameter.
 $t | grep '^adds = \[\]'
-- 
2.19.1.3.g30247aa5d201

On Fri, Feb 11, 2022 at 04:32:25PM +0100, Laszlo Ersek
wrote:
> Similarly to how users can can force ANSI colorization with
"--colors"
> even when stdout / stderr are redirected to a non-tty, allow them to force
> wrapping with "--wrap".
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1820221
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
> 
> Notes:
>     - This patch will require documentation updates for the following
>       guestfs-tools utilities (those that already document
"--colours"):
>     
>         builder/virt-builder-repository.pod
>         builder/virt-builder.pod
>         customize/virt-customize.pod
>         dib/virt-dib.pod
>         get-kernel/virt-get-kernel.pod
>         resize/virt-resize.pod
>         sparsify/virt-sparsify.pod
>         sysprep/virt-sysprep.pod
>     
>       Virt-v2v as well:
>     
>         docs/virt-v2v.pod
>     
>       This is evident e.g. from "test-virt-customize-docs.sh"
failing in
>       guestfs-tools, and "test-v2v-docs.sh" failing in virt-v2v.
>     
>       If this patch is accepted, I intend to post such guestfs-tools and
>       virt-v2v patches that atomically bump the submodule reference and
>       update the documentation.
>     
>     - Some utilities in libguestfs already use the short "-w"
option
>       (guestfish, guestmount, virt-rescue); however, those utilities do not
>       document "--colours", so they already don't make a
TTY-based
>       distinction, and will not see the new short "-w" option.
Yes, this is awkward.

Do we have to have the short version?  I'd really like to not burn
through useful-looking single letter options such as -w -- even if not
currently used in all tools -- for a pretty obscure feature that about
0.0001% of people are going to care about.  And as you say guestfish
establishes a clear precedence for this option meaning "writable".
>     - Easily testable e.g. with virt-resize:
>     
>       $ guestfish -N fs:ext4 exit
>       $ truncate -s 2G test1.resized.img
>       $ virt-resize    --expand /dev/sda1 test1.img test1.resized.img
>       $ virt-resize -w --expand /dev/sda1 test1.img test1.resized.img
>       $ virt-resize    --expand /dev/sda1 test1.img test1.resized.img >
log1
>       $ virt-resize -w --expand /dev/sda1 test1.img test1.resized.img >
log2
>       $ cat log1
>       $ cat log2
> 
>  mlstdutils/std_utils.mli | 6 ++++--
>  mlstdutils/std_utils.ml  | 8 ++++++--
>  mltools/tools_utils.ml   | 3 ++-
>  mltools/test-getopt.sh   | 2 ++
>  4 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/mlstdutils/std_utils.mli b/mlstdutils/std_utils.mli
> index 534caa80d6a7..87e923e24d29 100644
> --- a/mlstdutils/std_utils.mli
> +++ b/mlstdutils/std_utils.mli
> @@ -376,8 +376,10 @@ val set_trace : unit -> unit
>  val trace : unit -> bool
>  val set_verbose : unit -> unit
>  val verbose : unit -> bool
> -(** Stores the colours ([--colours]), quiet ([--quiet]), trace ([-x]) and
> -    verbose ([-v]) flags in global variables. *)
> +val set_wrap : unit -> unit
> +val wrap : unit -> bool
> +(** Stores the colours ([--colours]), quiet ([--quiet]), trace ([-x]),
verbose
> +    ([-v]) and wrap ([-w]) flags in global variables. *)
>  
>  val with_open_in : string -> (in_channel -> 'a) -> 'a
>  (** [with_open_in filename f] calls function [f] with [filename]
> diff --git a/mlstdutils/std_utils.ml b/mlstdutils/std_utils.ml
> index 58ac058c1b3a..52f44760c490 100644
> --- a/mlstdutils/std_utils.ml
> +++ b/mlstdutils/std_utils.ml
> @@ -580,8 +580,8 @@ let which executable >  (* Program name. *)
>  let prog = Filename.basename Sys.executable_name
>  
> -(* Stores the colours (--colours), quiet (--quiet), trace (-x) and
> - * verbose (-v) flags in a global variable.
> +(* Stores the colours (--colours), quiet (--quiet), trace (-x), verbose
(-v)
> + * and wrap (-w) flags in a global variable.
>   *)
>  let colours = ref false
>  let set_colours () = colours := true
> @@ -599,6 +599,10 @@ let verbose = ref false
>  let set_verbose () = verbose := true
>  let verbose () = !verbose
>  
> +let wrap = ref false
> +let set_wrap () = wrap := true
> +let wrap () = !wrap
> +
>  let with_open_in filename f >    let chan = open_in filename in
>    protect ~f:(fun () -> f chan) ~finally:(fun () -> close_in chan)
> diff --git a/mltools/tools_utils.ml b/mltools/tools_utils.ml
> index 4c5188988e03..eb8da290eec6 100644
> --- a/mltools/tools_utils.ml
> +++ b/mltools/tools_utils.ml
> @@ -126,7 +126,7 @@ let message fs >  type wrap_break_t = WrapEOS |
WrapSpace | WrapNL
>  
>  let rec wrap ?(chan = stdout) ?(indent = 0) str > -  if istty chan then
> +  if Std_utils.wrap () || istty chan then
>      let len = String.length str in
>      _wrap chan indent 0 0 len str
>    else (
> @@ -388,6 +388,7 @@ let create_standard_options argspec ?anon_fun
?(key_opts = false)
>    add_argspec ([ S 'q'; L"quiet" ],   Getopt.Unit
set_quiet,    s_"Don?t print progress messages");
>    add_argspec ([ L"color"; L"colors";
>                   L"colour"; L"colours" ], Getopt.Unit
set_colours, s_"Use ANSI colour sequences even if not tty");
> +  add_argspec ([ S 'w'; L"wrap" ],    Getopt.Unit
set_wrap,     s_"Wrap log messages even if not tty");
... So what I'd do is drop the -w version (keep the --wrap version of
course).
>  
>    if key_opts then (
>      let parse_key_selector arg > diff --git a/mltools/test-getopt.sh
b/mltools/test-getopt.sh
> index d9919a9a915a..1d7bb7b1dc4f 100755
> --- a/mltools/test-getopt.sh
> +++ b/mltools/test-getopt.sh
> @@ -68,6 +68,7 @@ $t --short-options | grep '^-is'
>  $t --short-options | grep '^-t'
>  $t --short-options | grep '^-V'
>  $t --short-options | grep '^-v'
> +$t --short-options | grep '^-w'
>  $t --short-options | grep '^-x'
>  
>  # --long-options
> @@ -89,6 +90,7 @@ $t --long-options | grep '^--ii'
>  $t --long-options | grep '^--is'
>  $t --long-options | grep '^--version'
>  $t --long-options | grep '^--verbose'
> +$t --long-options | grep '^--wrap'
>  
>  # -a/--add parameter.
>  $t | grep '^adds = \[\]'
> -- 
> 2.19.1.3.g30247aa5d201
With the appropriate change to drop -w in various places in this patch:

Reviewed-by: Richard W.M. Jones <rjones at redhat.com>

You might as well push the docs & submodule updates in the other tools
without review.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top