[4/5] argp: Improve comments.
Commit Message
* argp/argp-help.c: Add sectioning comments. Write NULL to designate a
null pointer.
(struct hol_entry): Fix comment regarding sort order of group.
(hol_entry_short_iterate, hol_entry_long_iterate): Add comment.
(until_short, canon_doc_option, hol_entry_qcmp): Improve comment.
(hol_cluster_is_child, argp_hol): Move functions.
---
argp/argp-help.c | 108 +++++++++++++++++++++++++++++++++----------------------
1 file changed, 65 insertions(+), 43 deletions(-)
Comments
On 06/01/2021 22:06, Bruno Haible wrote:
> * argp/argp-help.c: Add sectioning comments. Write NULL to designate a
> null pointer.
> (struct hol_entry): Fix comment regarding sort order of group.
> (hol_entry_short_iterate, hol_entry_long_iterate): Add comment.
> (until_short, canon_doc_option, hol_entry_qcmp): Improve comment.
> (hol_cluster_is_child, argp_hol): Move functions.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> argp/argp-help.c | 108 +++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 65 insertions(+), 43 deletions(-)
>
> diff --git a/argp/argp-help.c b/argp/argp-help.c
> index d686a23..637abca 100644
> --- a/argp/argp-help.c
> +++ b/argp/argp-help.c
> @@ -87,6 +87,8 @@ char *strerror (int errnum);
> # define SIZE_MAX ((size_t) -1)
> #endif
>
> +/* ========================================================================== */
> +
> /* User-selectable (using an environment variable) formatting parameters.
>
> These may be specified in an environment variable called `ARGP_HELP_FMT',
> @@ -253,6 +255,8 @@ fill_in_uparams (const struct argp_state *state)
> }
> }
>
> +/* ========================================================================== */
> +
> /* Returns true if OPT hasn't been marked invisible. Visibility only affects
> whether OPT is displayed or used in sorting, not option shadowing. */
> #define ovisible(opt) (! ((opt)->flags & OPTION_HIDDEN))
> @@ -345,6 +349,9 @@ find_char (char ch, char *beg, char *end)
> return 0;
> }
>
> +/* -------------------------------------------------------------------------- */
> +/* Data structure: HOL = Help Option List */
> +
> struct hol_cluster; /* fwd decl */
>
> struct hol_entry
> @@ -363,11 +370,11 @@ struct hol_entry
> char *short_options;
>
> /* Entries are sorted by their group first, in the order:
> - 1, 2, ..., n, 0, -m, ..., -2, -1
> + 0, 1, 2, ..., n, -m, ..., -2, -1
> and then alphabetically within each group. The default is 0. */
> int group;
>
> - /* The cluster of options this entry belongs to, or 0 if none. */
> + /* The cluster of options this entry belongs to, or NULL if none. */
> struct hol_cluster *cluster;
>
> /* The argp from which this option came. */
> @@ -389,7 +396,7 @@ struct hol_cluster
> same depth (clusters always follow options in the same group). */
> int group;
>
> - /* The cluster to which this cluster belongs, or 0 if it's at the base
> + /* The cluster to which this cluster belongs, or NULL if it's at the base
> level. */
> struct hol_cluster *parent;
>
> @@ -422,7 +429,7 @@ struct hol
> };
>
> /* Create a struct hol from the options in ARGP. CLUSTER is the
> - hol_cluster in which these entries occur, or 0, if at the root. */
> + hol_cluster in which these entries occur, or NULL if at the root. */
> static struct hol *
> make_hol (const struct argp *argp, struct hol_cluster *cluster)
> {
> @@ -540,6 +547,9 @@ hol_free (struct hol *hol)
> free (hol);
> }
>
> +/* Iterate across the short_options of the given ENTRY. Call FUNC for each.
> + Stop when such a call returns a non-zero value, and return this value.
> + If all FUNC invocations returned 0, return 0. */
> static int
> hol_entry_short_iterate (const struct hol_entry *entry,
> int (*func)(const struct argp_option *opt,
> @@ -565,6 +575,9 @@ hol_entry_short_iterate (const struct hol_entry *entry,
> return val;
> }
>
> +/* Iterate across the long options of the given ENTRY. Call FUNC for each.
> + Stop when such a call returns a non-zero value, and return this value.
> + If all FUNC invocations returned 0, return 0. */
> static inline int
> __attribute__ ((always_inline))
> hol_entry_long_iterate (const struct hol_entry *entry,
> @@ -589,7 +602,7 @@ hol_entry_long_iterate (const struct hol_entry *entry,
> return val;
> }
>
> -/* Iterator that returns true for the first short option. */
> +/* A filter that returns true for the first short option of a given ENTRY. */
> static inline int
> until_short (const struct argp_option *opt, const struct argp_option *real,
> const char *domain, void *cookie)
> @@ -605,7 +618,7 @@ hol_entry_first_short (const struct hol_entry *entry)
> entry->argp->argp_domain, 0);
> }
>
> -/* Returns the first valid long option in ENTRY, or 0 if there is none. */
> +/* Returns the first valid long option in ENTRY, or NULL if there is none. */
> static const char *
> hol_entry_first_long (const struct hol_entry *entry)
> {
> @@ -617,7 +630,7 @@ hol_entry_first_long (const struct hol_entry *entry)
> return 0;
> }
>
> -/* Returns the entry in HOL with the long option name NAME, or 0 if there is
> +/* Returns the entry in HOL with the long option name NAME, or NULL if there is
> none. */
> static struct hol_entry *
> hol_find_entry (struct hol *hol, const char *name)
> @@ -652,6 +665,9 @@ hol_set_group (struct hol *hol, const char *name, int group)
> entry->group = group;
> }
>
> +/* -------------------------------------------------------------------------- */
> +/* Sorting the entries in a HOL. */
> +
> /* Order by group: 0, 1, 2, ..., n, -m, ..., -2, -1.
> EQ is what to return if GROUP1 and GROUP2 are the same. */
> static int
> @@ -694,18 +710,8 @@ hol_cluster_base (struct hol_cluster *cl)
> cl = cl->parent;
> return cl;
> }
> -
> -/* Return true if CL1 is a child of CL2. */
> -static int
> -hol_cluster_is_child (const struct hol_cluster *cl1,
> - const struct hol_cluster *cl2)
> -{
> - while (cl1 && cl1 != cl2)
> - cl1 = cl1->parent;
> - return cl1 == cl2;
> -}
>
> -/* Given the name of a OPTION_DOC option, modifies NAME to start at the tail
> +/* Given the name of an OPTION_DOC option, modifies *NAME to start at the tail
> that should be used for comparisons, and returns true iff it should be
> treated as a non-option. */
> static int
> @@ -796,7 +802,7 @@ hol_entry_cmp (const struct hol_entry *entry1,
> return group_cmp (group1, group2, 0);
> }
>
> -/* Version of hol_entry_cmp with correct signature for qsort. */
> +/* Variant of hol_entry_cmp with correct signature for qsort. */
> static int
> hol_entry_qcmp (const void *entry1_v, const void *entry2_v)
> {
> @@ -814,6 +820,9 @@ hol_sort (struct hol *hol)
> hol_entry_qcmp);
> }
>
> +/* -------------------------------------------------------------------------- */
> +/* Constructing the HOL. */
> +
> /* Append MORE to HOL, destroying MORE in the process. Options in HOL shadow
> any in MORE with the same name. */
> static void
> @@ -909,6 +918,32 @@ hol_append (struct hol *hol, struct hol *more)
> hol_free (more);
> }
>
> +/* Make a HOL containing all levels of options in ARGP. CLUSTER is the
> + cluster in which ARGP's entries should be clustered, or 0. */
> +static struct hol *
> +argp_hol (const struct argp *argp, struct hol_cluster *cluster)
> +{
> + const struct argp_child *child = argp->children;
> + struct hol *hol = make_hol (argp, cluster);
> + if (child)
> + while (child->argp)
> + {
> + struct hol_cluster *child_cluster =
> + ((child->group || child->header)
> + /* Put CHILD->argp within its own cluster. */
> + ? hol_add_cluster (hol, child->group, child->header,
> + child - argp->children, cluster, argp)
> + /* Just merge it into the parent's cluster. */
> + : cluster);
> + hol_append (hol, argp_hol (child->argp, child_cluster)) ;
> + child++;
> + }
> + return hol;
> +}
> +
> +/* -------------------------------------------------------------------------- */
> +/* Printing the HOL. */
> +
> /* Inserts enough spaces to make sure STREAM is at column COL. */
> static void
> indent_to (argp_fmtstream_t stream, unsigned col)
> @@ -953,7 +988,7 @@ arg (const struct argp_option *real, const char *req_fmt, const char *opt_fmt,
> /* State used during the execution of hol_help. */
> struct hol_help_state
> {
> - /* PREV_ENTRY should contain the previous entry printed, or 0. */
> + /* PREV_ENTRY should contain the previous entry printed, or NULL. */
> struct hol_entry *prev_entry;
>
> /* If an entry is in a different group from the previous one, and SEP_GROUPS
> @@ -1031,6 +1066,16 @@ print_header (const char *str, const struct argp *argp,
> free ((char *) fstr);
> }
>
> +/* Return true if CL1 is a child of CL2. */
> +static int
> +hol_cluster_is_child (const struct hol_cluster *cl1,
> + const struct hol_cluster *cl2)
> +{
> + while (cl1 && cl1 != cl2)
> + cl1 = cl1->parent;
> + return cl1 == cl2;
> +}
> +
> /* Inserts a comma if this isn't the first item on the line, and then makes
> sure we're at least to column COL. If this *is* the first item on a line,
> prints any pending whitespace/headers that should precede this line. Also
> @@ -1344,29 +1389,6 @@ hol_usage (struct hol *hol, argp_fmtstream_t stream)
> }
> }
>
> -/* Make a HOL containing all levels of options in ARGP. CLUSTER is the
> - cluster in which ARGP's entries should be clustered, or 0. */
> -static struct hol *
> -argp_hol (const struct argp *argp, struct hol_cluster *cluster)
> -{
> - const struct argp_child *child = argp->children;
> - struct hol *hol = make_hol (argp, cluster);
> - if (child)
> - while (child->argp)
> - {
> - struct hol_cluster *child_cluster =
> - ((child->group || child->header)
> - /* Put CHILD->argp within its own cluster. */
> - ? hol_add_cluster (hol, child->group, child->header,
> - child - argp->children, cluster, argp)
> - /* Just merge it into the parent's cluster. */
> - : cluster);
> - hol_append (hol, argp_hol (child->argp, child_cluster)) ;
> - child++;
> - }
> - return hol;
> -}
> -
> /* Calculate how many different levels with alternative args strings exist in
> ARGP. */
> static size_t
>
@@ -87,6 +87,8 @@ char *strerror (int errnum);
# define SIZE_MAX ((size_t) -1)
#endif
+/* ========================================================================== */
+
/* User-selectable (using an environment variable) formatting parameters.
These may be specified in an environment variable called `ARGP_HELP_FMT',
@@ -253,6 +255,8 @@ fill_in_uparams (const struct argp_state *state)
}
}
+/* ========================================================================== */
+
/* Returns true if OPT hasn't been marked invisible. Visibility only affects
whether OPT is displayed or used in sorting, not option shadowing. */
#define ovisible(opt) (! ((opt)->flags & OPTION_HIDDEN))
@@ -345,6 +349,9 @@ find_char (char ch, char *beg, char *end)
return 0;
}
+/* -------------------------------------------------------------------------- */
+/* Data structure: HOL = Help Option List */
+
struct hol_cluster; /* fwd decl */
struct hol_entry
@@ -363,11 +370,11 @@ struct hol_entry
char *short_options;
/* Entries are sorted by their group first, in the order:
- 1, 2, ..., n, 0, -m, ..., -2, -1
+ 0, 1, 2, ..., n, -m, ..., -2, -1
and then alphabetically within each group. The default is 0. */
int group;
- /* The cluster of options this entry belongs to, or 0 if none. */
+ /* The cluster of options this entry belongs to, or NULL if none. */
struct hol_cluster *cluster;
/* The argp from which this option came. */
@@ -389,7 +396,7 @@ struct hol_cluster
same depth (clusters always follow options in the same group). */
int group;
- /* The cluster to which this cluster belongs, or 0 if it's at the base
+ /* The cluster to which this cluster belongs, or NULL if it's at the base
level. */
struct hol_cluster *parent;
@@ -422,7 +429,7 @@ struct hol
};
/* Create a struct hol from the options in ARGP. CLUSTER is the
- hol_cluster in which these entries occur, or 0, if at the root. */
+ hol_cluster in which these entries occur, or NULL if at the root. */
static struct hol *
make_hol (const struct argp *argp, struct hol_cluster *cluster)
{
@@ -540,6 +547,9 @@ hol_free (struct hol *hol)
free (hol);
}
+/* Iterate across the short_options of the given ENTRY. Call FUNC for each.
+ Stop when such a call returns a non-zero value, and return this value.
+ If all FUNC invocations returned 0, return 0. */
static int
hol_entry_short_iterate (const struct hol_entry *entry,
int (*func)(const struct argp_option *opt,
@@ -565,6 +575,9 @@ hol_entry_short_iterate (const struct hol_entry *entry,
return val;
}
+/* Iterate across the long options of the given ENTRY. Call FUNC for each.
+ Stop when such a call returns a non-zero value, and return this value.
+ If all FUNC invocations returned 0, return 0. */
static inline int
__attribute__ ((always_inline))
hol_entry_long_iterate (const struct hol_entry *entry,
@@ -589,7 +602,7 @@ hol_entry_long_iterate (const struct hol_entry *entry,
return val;
}
-/* Iterator that returns true for the first short option. */
+/* A filter that returns true for the first short option of a given ENTRY. */
static inline int
until_short (const struct argp_option *opt, const struct argp_option *real,
const char *domain, void *cookie)
@@ -605,7 +618,7 @@ hol_entry_first_short (const struct hol_entry *entry)
entry->argp->argp_domain, 0);
}
-/* Returns the first valid long option in ENTRY, or 0 if there is none. */
+/* Returns the first valid long option in ENTRY, or NULL if there is none. */
static const char *
hol_entry_first_long (const struct hol_entry *entry)
{
@@ -617,7 +630,7 @@ hol_entry_first_long (const struct hol_entry *entry)
return 0;
}
-/* Returns the entry in HOL with the long option name NAME, or 0 if there is
+/* Returns the entry in HOL with the long option name NAME, or NULL if there is
none. */
static struct hol_entry *
hol_find_entry (struct hol *hol, const char *name)
@@ -652,6 +665,9 @@ hol_set_group (struct hol *hol, const char *name, int group)
entry->group = group;
}
+/* -------------------------------------------------------------------------- */
+/* Sorting the entries in a HOL. */
+
/* Order by group: 0, 1, 2, ..., n, -m, ..., -2, -1.
EQ is what to return if GROUP1 and GROUP2 are the same. */
static int
@@ -694,18 +710,8 @@ hol_cluster_base (struct hol_cluster *cl)
cl = cl->parent;
return cl;
}
-
-/* Return true if CL1 is a child of CL2. */
-static int
-hol_cluster_is_child (const struct hol_cluster *cl1,
- const struct hol_cluster *cl2)
-{
- while (cl1 && cl1 != cl2)
- cl1 = cl1->parent;
- return cl1 == cl2;
-}
-/* Given the name of a OPTION_DOC option, modifies NAME to start at the tail
+/* Given the name of an OPTION_DOC option, modifies *NAME to start at the tail
that should be used for comparisons, and returns true iff it should be
treated as a non-option. */
static int
@@ -796,7 +802,7 @@ hol_entry_cmp (const struct hol_entry *entry1,
return group_cmp (group1, group2, 0);
}
-/* Version of hol_entry_cmp with correct signature for qsort. */
+/* Variant of hol_entry_cmp with correct signature for qsort. */
static int
hol_entry_qcmp (const void *entry1_v, const void *entry2_v)
{
@@ -814,6 +820,9 @@ hol_sort (struct hol *hol)
hol_entry_qcmp);
}
+/* -------------------------------------------------------------------------- */
+/* Constructing the HOL. */
+
/* Append MORE to HOL, destroying MORE in the process. Options in HOL shadow
any in MORE with the same name. */
static void
@@ -909,6 +918,32 @@ hol_append (struct hol *hol, struct hol *more)
hol_free (more);
}
+/* Make a HOL containing all levels of options in ARGP. CLUSTER is the
+ cluster in which ARGP's entries should be clustered, or 0. */
+static struct hol *
+argp_hol (const struct argp *argp, struct hol_cluster *cluster)
+{
+ const struct argp_child *child = argp->children;
+ struct hol *hol = make_hol (argp, cluster);
+ if (child)
+ while (child->argp)
+ {
+ struct hol_cluster *child_cluster =
+ ((child->group || child->header)
+ /* Put CHILD->argp within its own cluster. */
+ ? hol_add_cluster (hol, child->group, child->header,
+ child - argp->children, cluster, argp)
+ /* Just merge it into the parent's cluster. */
+ : cluster);
+ hol_append (hol, argp_hol (child->argp, child_cluster)) ;
+ child++;
+ }
+ return hol;
+}
+
+/* -------------------------------------------------------------------------- */
+/* Printing the HOL. */
+
/* Inserts enough spaces to make sure STREAM is at column COL. */
static void
indent_to (argp_fmtstream_t stream, unsigned col)
@@ -953,7 +988,7 @@ arg (const struct argp_option *real, const char *req_fmt, const char *opt_fmt,
/* State used during the execution of hol_help. */
struct hol_help_state
{
- /* PREV_ENTRY should contain the previous entry printed, or 0. */
+ /* PREV_ENTRY should contain the previous entry printed, or NULL. */
struct hol_entry *prev_entry;
/* If an entry is in a different group from the previous one, and SEP_GROUPS
@@ -1031,6 +1066,16 @@ print_header (const char *str, const struct argp *argp,
free ((char *) fstr);
}
+/* Return true if CL1 is a child of CL2. */
+static int
+hol_cluster_is_child (const struct hol_cluster *cl1,
+ const struct hol_cluster *cl2)
+{
+ while (cl1 && cl1 != cl2)
+ cl1 = cl1->parent;
+ return cl1 == cl2;
+}
+
/* Inserts a comma if this isn't the first item on the line, and then makes
sure we're at least to column COL. If this *is* the first item on a line,
prints any pending whitespace/headers that should precede this line. Also
@@ -1344,29 +1389,6 @@ hol_usage (struct hol *hol, argp_fmtstream_t stream)
}
}
-/* Make a HOL containing all levels of options in ARGP. CLUSTER is the
- cluster in which ARGP's entries should be clustered, or 0. */
-static struct hol *
-argp_hol (const struct argp *argp, struct hol_cluster *cluster)
-{
- const struct argp_child *child = argp->children;
- struct hol *hol = make_hol (argp, cluster);
- if (child)
- while (child->argp)
- {
- struct hol_cluster *child_cluster =
- ((child->group || child->header)
- /* Put CHILD->argp within its own cluster. */
- ? hol_add_cluster (hol, child->group, child->header,
- child - argp->children, cluster, argp)
- /* Just merge it into the parent's cluster. */
- : cluster);
- hol_append (hol, argp_hol (child->argp, child_cluster)) ;
- child++;
- }
- return hol;
-}
-
/* Calculate how many different levels with alternative args strings exist in
ARGP. */
static size_t