Re: [Libguestfs] [PATCH 3/3] builder: improve the handling of list formats

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