Re: [PATCH v2 15/20] parse-options: move doc to parse-options.h

[Heba Waly <heba.waly@gmail.com>]

On Mon, Nov 11, 2019 at 3:45 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Heba Waly via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > + * Steps to parse options
> > + * ----------------------
> > + *
> > + * - `#include "parse-options.h"`
> > + *
> > + * - define a NULL-terminated
> > + *   `static const char * const builtin_foo_usage[]` array
> > + *   containing alternative usage strings
> > + *
> > + * - define `builtin_foo_options` array as described below
> > + *   in section 'Data Structure'.
> > + *
> > + * - in `cmd_foo(int argc, const char **argv, const char *prefix)`
> > + *   call
> > + *
> > + *   argc = parse_options(argc, argv, prefix, builtin_foo_options, builtin_foo_usage, flags);
> > + *
> > + * `parse_options()` will filter out the processed options of `argv[]` and leave the
> > + * non-option arguments in `argv[]`.
> > + * `argc` is updated appropriately because of the assignment.
> > + *
> > + * You can also pass NULL instead of a usage array as the fifth parameter of
> > + * parse_options(), to avoid displaying a help screen with usage info and
> > + * option list. This should only be done if necessary, e.g. to implement
> > + * a limited parser for only a subset of the options that needs to be run
> > + * before the full parser, which in turn shows the full help message.
>
> After just looking at Documentation/technical/api-parse-options.txt
> for real reasons (i.e. not for the purpose of reviewing this patch,
> but to review a patch that uses parse-options API), I must say that
> applying this change would have made the documentation quite less
> useful, at least for my purpose.
>
> My reading began by "git grep PARSE_OPT_STOP_AT_NON_OPTION" to find
> the page that this patch proposes to remove, and in that document,
> the list of flags the PARSE_OPT_STOP_AT_NON_OPTION belongs to comes
> immediately after the above message, so it was much easier to see
> where in the overall API usage the option comes into picture.
>
> With the description moved close to enum{}, I know there would be
> less risk of adding a new member without documenting it, but that is
> for convenience by the developers of the parse-options API, and not
> necessarily makes it convenient to learn and use the API.  I would
> have been forced to jump around to hunt for the necessary pieces of
> info sprinkled in the header file to form the overall picture, that
> a simple flat-file text document would have much easily given me.
>
> So, I dunno.

Hi Junio,

I can see your point, let me share my perspective as I've thought
about that several times while moving the docs. As a new contributor
to the code base, when I come across a function or a data structure
that I want to understand what it does, I'd first try to find it in
the .h or .c files, looking for its documentation, unless I find a
link there to a file named
Documentation/technical/api-parse-options.txt I won't know about its
existence and I'll feel frustrated to find no documentation in the
code.

On the other hand some doc files had too many details for a header
file (or that's what I thought) like for example
Documentation/technical/api-trace2.txt. So the approach I took there
which is pending on review/discussions was to remove the functions
documentation from the text file to the .h file and leave the rest of
the details as is in the text file. (you can refer to this patch here
(https://public-inbox.org/git/a337f88a55ddaafd5754492ae0399137966738a8.1573507684.git.gitgitgadget@gmail.com/)).

So my proposal for this matter is to investigate the possibility of
using a doc generators that'd extract the documentations from the code
to a single doc file per library.

Thanks,
Heba