in Bin_conf, discard *all* output from cmdliner #2736
Conversation
lib/bin_conf/Bin_conf.ml
Outdated
| lazy | ||
| (Cmd.eval_value ~err:discard_formatter ~help:discard_formatter | ||
| (Cmd.v info global_lib_term) ) | ||
| (let discard = Format.formatter_of_buffer (Buffer.create 0) in |
There was a problem hiding this comment.
I like that we no longer have to maintain the formatter_out_functions definition that seems unstable.
Do you think we could use Format.make_formatter to also avoid writing to a buffer ?
There was a problem hiding this comment.
Ah, I looked for a function like this, but didn't see it.
We could use it, but I slightly prefer the buffer as it's just obviously correct (with the other version, you need to consult the documentation). And the formatter and the buffer are going to get gc'ed shortly after the lazy is forced (unless cmdliner is doing weird things).
I think what would make more sense is to stop suppressing errors, and move the command line parsing upfront, but maybe there's a reason why things are the way they are.
There was a problem hiding this comment.
Obviously correct doesn't exist with Format :p I don't think reading the doc is a problem for a piece of code that will probably stay correct and unchanged for a long time. You can add a comment to clarify.
I think what would make more sense is to stop suppressing errors, and move the command line parsing upfront, but maybe there's a reason why things are the way they are.
That'd make sense. It's in a lazy because build_config can be called several time, for example in the LSP server. This code is hopefully not called from the executable, which explains why the output is discarded. There are probably things that can be improved.
There was a problem hiding this comment.
Ok, I used Format.make_formatter.
That'd make sense. It's in a lazy because build_config can be called several time, for example in the LSP server. This code is hopefully not called from the executable, which explains why the output is discarded. There are probably things that can be improved.
The weirdness is not the laziness, it's the fact that command line parsing is normally done upfront, not in the middle of libraries. I looked at making the information flow explicit in v-gb@push-knzpolvnnptp and you can see it's very much called from the rpc lib (the change is probably not quite equivalent to the current behavior, but I suspect the current behavior is whatever the code produces, rather than something that was designed to work like it does).
Currently, Cmdliner is given a formatter that drops any text, but Cmdliner can (and does) print whitespace via the non-overridden out_spaces, out_indent and out_newlines.
v-gb
force-pushed
the
push-vtnqxpopymny
branch
from
7c6fd95 to
6a2a4fe
Compare
2 participants
Currently, Cmdliner is given a formatter that drops any text, but Cmdliner can (and does) print whitespace via the non-overridden out_spaces, out_indent and out_newlines.