[Libguestfs] [PATCH 3/3] builder: improve the handling of list formats
Richard W.M. Jones
rjones at redhat.com
Mon Jul 18 11:39:03 UTC 2016
On Mon, Jul 18, 2016 at 01:18:02PM +0200, Pino Toscano wrote:
> Store them directly in List_entries, an adding helper function to
> convert from string.
>
> Also use Getopt.Symbol for them, so there is no need to manually reject
> invalid formats.
> ---
> builder/cmdline.ml | 21 +++++++++++----------
> builder/cmdline.mli | 2 +-
> builder/list_entries.ml | 19 ++++++++++++++++---
> builder/list_entries.mli | 16 +++++++++++++++-
> 4 files changed, 43 insertions(+), 15 deletions(-)
>
> diff --git a/builder/cmdline.ml b/builder/cmdline.ml
> index 846c2e3..0667994 100644
> --- a/builder/cmdline.ml
> +++ b/builder/cmdline.ml
> @@ -42,7 +42,7 @@ type cmdline = {
> delete_on_failure : bool;
> format : string option;
> gpg : string;
> - list_format : [`Short|`Long|`Json];
> + list_format : List_entries.format;
> memsize : int option;
> network : bool;
> ops : Customize_cmdline.ops;
> @@ -88,15 +88,13 @@ let parse_cmdline () =
> let format = ref "" in
> let gpg = ref "gpg" in
>
> - let list_format = ref `Short in
> - let list_set_long () = list_format := `Long in
> + let list_format = ref List_entries.Short in
> + let list_set_long () = list_format := List_entries.Long in
> let list_set_format arg =
> - list_format := match arg with
> - | "short" -> `Short
> - | "long" -> `Long
> - | "json" -> `Json
> - | fmt ->
> - error (f_"invalid --list-format type '%s', see the man page") fmt in
> + (* Do not catch the Invalid_argument that list_format_of_string
> + * throws on invalid input, as it is already checked by the
> + * Getopt handling of Symbol. *)
> + list_format := List_entries.list_format_of_string arg in
>
> let machine_readable = ref false in
>
> @@ -118,6 +116,9 @@ let parse_cmdline () =
> let sync = ref true in
> let warn_if_partition = ref true in
>
> + let formats = List_entries.list_formats
> + and formats_string = String.concat "|" List_entries.list_formats in
> +
> let argspec = [
> [ "--arch" ], Getopt.Set_string ("arch", arch), s_"Set the output architecture";
> [ "--attach" ], Getopt.String ("iso", attach_disk), s_"Attach data disk/ISO during install";
> @@ -144,7 +145,7 @@ let parse_cmdline () =
> [ "--gpg" ], Getopt.Set_string ("gpg", gpg), s_"Set GPG binary/command";
> [ "-l"; "--list" ], Getopt.Unit list_mode, s_"List available templates";
> [ "--long" ], Getopt.Unit list_set_long, s_"Shortcut for --list-format long";
> - [ "--list-format" ], Getopt.String ("short|long|json", list_set_format),
> + [ "--list-format" ], Getopt.Symbol (formats_string, formats, list_set_format),
> s_"Set the format for --list (default: short)";
> [ "--machine-readable" ], Getopt.Set machine_readable, s_"Make output machine readable";
> [ "-m"; "--memsize" ], Getopt.Int ("mb", set_memsize), s_"Set memory size";
> diff --git a/builder/cmdline.mli b/builder/cmdline.mli
> index 854db61..a0517a2 100644
> --- a/builder/cmdline.mli
> +++ b/builder/cmdline.mli
> @@ -30,7 +30,7 @@ type cmdline = {
> delete_on_failure : bool;
> format : string option;
> gpg : string;
> - list_format : [`Short|`Long|`Json];
> + list_format : List_entries.format;
> memsize : int option;
> network : bool;
> ops : Customize_cmdline.ops;
> diff --git a/builder/list_entries.ml b/builder/list_entries.ml
> index 2f053e8..2a1aef4 100644
> --- a/builder/list_entries.ml
> +++ b/builder/list_entries.ml
> @@ -21,11 +21,24 @@ open Common_utils
>
> open Printf
>
> +type format =
> + | Short
> + | Long
> + | Json
> +
> +let list_formats = [ "short"; "long"; "json" ]
> +
> +let list_format_of_string = function
> + | "short" -> Short
> + | "long" -> Long
> + | "json" -> Json
> + | fmt -> invalid_arg fmt
> +
> let rec list_entries ~list_format ~sources index =
> match list_format with
> - | `Short -> list_entries_short index
> - | `Long -> list_entries_long ~sources index
> - | `Json -> list_entries_json ~sources index
> + | Short -> list_entries_short index
> + | Long -> list_entries_long ~sources index
> + | Json -> list_entries_json ~sources index
>
> and list_entries_short index =
> List.iter (
> diff --git a/builder/list_entries.mli b/builder/list_entries.mli
> index a3f35d3..008b906 100644
> --- a/builder/list_entries.mli
> +++ b/builder/list_entries.mli
> @@ -16,4 +16,18 @@
> * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> *)
>
> -val list_entries : list_format:([ `Short | `Long | `Json ]) -> sources:Sources.source list -> Index.index -> unit
> +type format =
> + | Short
> + | Long
> + | Json
> +
> +val list_formats : string list
> +(** The string representation of the available formats. *)
> +
> +val list_format_of_string : string -> format
> +(** Convert from a string to the corresponding format.
> +
> + Throw [Invalid_argument] if the string does not match any
> + valid format. *)
> +
> +val list_entries : list_format:format -> sources:Sources.source list -> Index.index -> unit
> --
> 2.7.4
>
ACK, although it would be nice if you could either rebase these
on top of mine or review my getopt patches (v2).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
More information about the Libguestfs
mailing list