[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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

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.

>    (* 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 *)
  | 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")

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]