Date: Tue, 10 Dec 2019 10:14:47 +0200
From: Timo Teras <timo.teras@iki.fi>
To: Rich Ireland <r.ireland@computer.org>
Cc: ~alpine/devel@lists.alpinelinux.org
Subject: Re: [PATCH] Add missing command_groups flags.
Message-ID: <20191210101447.1efb7ea8@vostro>
In-Reply-To: <20191107211947.29488-1-r.ireland@computer.org>
References: <20191107211947.29488-1-r.ireland@computer.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

Hi,

Sorry for the late reply. I've been terribly busy past few months.
Finally catching up backlog.

On Thu,  7 Nov 2019 14:19:47 -0700
Rich Ireland <r.ireland@computer.org> wrote:

> Some apk commands are missing from the top level help text.
> ---
>  src/audit.c  | 1 +
>  src/search.c | 1 +
>  src/stats.c  | 1 +
>  src/ver.c    | 1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/src/audit.c b/src/audit.c
> index 0c1fbfc..1c3ed39 100644
> --- a/src/audit.c
> +++ b/src/audit.c
> @@ -350,6 +350,7 @@ static struct apk_applet apk_audit = {
>  	.help = "Audit the directories for changes",
>  	.arguments = "[directory to audit]...",
>  	.open_flags =
> APK_OPENF_READ|APK_OPENF_NO_SCRIPTS|APK_OPENF_NO_REPOS,
> +	.command_groups = APK_COMMAND_GROUP_REPO,
>  	.context_size = sizeof(struct audit_ctx),
>  	.optgroups = { &optgroup_global, &optgroup_applet },
>  	.main = audit_main,

This is not true. It has the NO_REPOS open flag, and operates only on
using the installed db. Should go to APK_COMMAND_GROUP_SYSTEM.

> diff --git a/src/search.c b/src/search.c
> index 9a71747..0b00a3b 100644
> --- a/src/search.c
> +++ b/src/search.c
> @@ -209,6 +209,7 @@ static struct apk_applet apk_search = {
>  	.help = "Search package by PATTERNs or by indexed
> dependencies", .arguments = "PATTERN",
>  	.open_flags = APK_OPENF_READ | APK_OPENF_NO_STATE,
> +	.command_groups = APK_COMMAND_GROUP_QUERY,
>  	.context_size = sizeof(struct search_ctx),
>  	.optgroups = { &optgroup_global, &optgroup_applet },
>  	.main = search_main,

This was explicitly removed in commit d379edd5. But I am not sure if
the list applet is ready yet. If this is re-intrduced, I'd rather
revert the original commit.

> diff --git a/src/stats.c b/src/stats.c
> index 5dc2561..032fbbf 100644
> --- a/src/stats.c
> +++ b/src/stats.c
> @@ -57,6 +57,7 @@ static struct apk_applet stats_applet = {
>  	.name = "stats",
>  	.help = "Show statistics about repositories and
> installations", .open_flags = APK_OPENF_READ,
> +	.command_groups = APK_COMMAND_GROUP_REPO,
>  	.main = stats_main,
>  };

The main function is to show information about system, not the
repository. It also does not accept the repository options.
I'd put it in APK_COMMAND_GROUP_QUERY instead.

> diff --git a/src/ver.c b/src/ver.c
> index 3a08359..4097c4d 100644
> --- a/src/ver.c
> +++ b/src/ver.c
> @@ -202,6 +202,7 @@ static struct apk_applet apk_ver = {
>  	.help = "Compare package versions (in installed database vs.
> available) " "or do tests on literal version strings",
>  	.open_flags = APK_OPENF_READ,
> +	.command_groups = APK_COMMAND_GROUP_QUERY,
>  	.context_size = sizeof(struct ver_ctx),
>  	.optgroups = { &optgroup_global, &optgroup_applet },
>  	.main = ver_main,

This looks good.

Thanks for submitting this.

Timo