[Libguestfs] [libguestfs-common PATCH 3/3] add common ("standard") option -w / --wrap
Laszlo Ersek
lersek at redhat.com
Mon Feb 14 17:57:14 UTC 2022
On 02/12/22 19:18, Richard W.M. Jones wrote:
> 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.
>
I've pushed this series as commit range 5b5fac3e0b10..41126802097f, with
the short option "-w" dropped. I'll follow up with patches for the
dependent projects (libguestfs, guestfs-tools, virt-v2v).
Thanks!
Laszlo
More information about the Libguestfs
mailing list