[Libguestfs] [PATCH 2/2] OCaml tools: add output selection for --machine-readable

[Pino Toscano]

On Wednesday, 22 August 2018 12:29:41 CEST Richard W.M. Jones wrote:
> On Tue, Aug 21, 2018 at 05:44:30PM +0200, Pino Toscano wrote:
> > +let machine_readable_printf fs =
> > +  let get_machine_readable_channel () =
> > +    let open_machine_readable_channel () =
> > +      match !machine_readable_output with
> > +      | NoOutput ->
> > +        (* Trying to use machine_readable_printf when --machine-readable was
> > +         * not enabled, and thus machine_readable () returns false.
> > +         *)
> > +        failwith "internal error: machine_readable_printf used with no --machine-readable"
> 
> I wonder if there's a way to avoid this error at compile time.
> 
> Replace the ‘machine_readable ()’ function that returns boolean with
> one which returns an optional printf function.  Then caller code would
> do:
> 
>   match machine_readable () with
>   | None -> ()  (* ie. not --machine-readable *)
>   | Some pr ->
>       pr "stuff\n";
>       exit 0

Good idea, this matches more or less what is done currently.

> Of course the devil will be in the details as to whether this actually
> works with our existing callers.

> > +  fprintf (get_machine_readable_channel ()) fs
> 
> I'm surprised this works and you didn't need to use ksprintf.

get_machine_readable_channel () returns an out_channel, so this is
effectively like calling

  fprintf stderr "string" ...

> >    (* No elements and machine-readable mode?  Print some facts. *)
> >    if elements = [] && machine_readable () then (
> > -    printf "virt-dib\n";
> > +    machine_readable_printf "virt-dib\n";
> >      let formats_list = Output_format.list_formats () in
> > -    List.iter (printf "output:%s\n") formats_list;
> > +    List.iter (machine_readable_printf "output:%s\n") formats_list;
> >      exit 0
> >    );
> 
> So this caller would become:
> 
>   match machine_readable (), elements = [] with
>   | None, _ -> ()
>   | Some pr, [] -> 
>      (* existing code, replacing printf with pr *)

OK.

>   | Some _, _ ->
>      (* this is a new case giving an error when the user
>         uses --machine-readable + a list of elements, which I believe
>         is not caught in the current code *)
>      error (f_"--machine-readable cannot be used with elements on the command line")

So far the expectation is that --machine-readable will produce more
"machine parseable" output, so I'd not rule out this option.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20180822/5ddb2f08/attachment.sig>