[Libguestfs] [PATCH] builder: read all the available notes from the index

Richard W.M. Jones rjones at redhat.com
Wed Jan 22 22:51:02 UTC 2014


On Wed, Jan 22, 2014 at 11:22:47PM +0100, Pino Toscano wrote:
> Switch the internal storage for the notes of each entry to a sorted list
> with all the subkeys available (which should represent the translations
> to various languages).
> The current outputs are the same (i.e. still the untranslated notes), so
> this is just internal refactoring/preparation.
> ---
>  builder/builder.ml       |  4 ++--
>  builder/index_parser.ml  | 19 +++++++++++++++----
>  builder/index_parser.mli |  2 +-
>  builder/list_entries.ml  | 10 +++++++---
>  4 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/builder/builder.ml b/builder/builder.ml
> index bb0b108..19d1e42 100644
> --- a/builder/builder.ml
> +++ b/builder/builder.ml
> @@ -200,9 +200,9 @@ let main () =
>    (match mode with
>    | `Notes ->                           (* --notes *)
>      (match entry with
> -    | { Index_parser.notes = Some notes } ->
> +    | { Index_parser.notes = ("", notes) :: _ } ->
>        print_endline notes;
> -    | { Index_parser.notes = None } ->
> +    | { Index_parser.notes = _ } ->

If you have to match on _, especially:

> --- a/builder/list_entries.ml
> +++ b/builder/list_entries.ml
> @@ -71,10 +71,10 @@ and list_entries_long ~sources index =
>            printf "%-24s %s\n" (s_"Download size:") (human_size size);
>          );
>          (match notes with
> -        | None -> ()
> -        | Some notes ->
> +        | ("", notes) :: _ ->
>            printf "\n";
>            printf (f_"Notes:\n\n%s\n") notes
> +        | _ -> ()

then you've done something wrong.  The problem is that the _ stops the
compiler from finding missing cases in your code, since _ matches any
case.  It's a bit like why it's bad practice to catch any exception in Java.

Since the list is going to contain something like one of these:

  [ "", "some notes" ]
  [ "en", "some notes"; "ru", "..." ]
  [ "ru", "..."; "", "..." ]
  []

the pattern
  | ("", notes) :: _ ->
is only going to match the first of those, not the second or third.

I think for this refactoring what you really want is:

  | [_, notes] :: _ -> (* print the notes *)
  | [] -> (* no notes *)

which will print the first set of notes that happen to be in the index
(in any language), which is fine for the current index that only has
one set of notes per template.

Later when you actually implement language support you're going to
have to use List.assoc, so:

  | notes when List.mem_assoc "en" notes ->
     printf "notes = %s" (List.assoc "en" notes)
  | [] ->
     printf "no notes"

(or something cleverer to deal with the current locale).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/




More information about the Libguestfs mailing list