Improving net help output

Jelmer Vernooij jelmer at samba.org
Sun Jan 31 15:18:50 MST 2010


Hi Matthieu,

Thanks for sending these patches by mail.

On Sun, Jan 31, 2010 at 11:47:35PM +0300, Matthieu Patou wrote:
> From 1fae77d735f90ce61c8aebd2cf9d9887298ee3fd Mon Sep 17 00:00:00 2001
> From: Matthieu Patou <mat at matws.net>
> Date: Fri, 8 Jan 2010 19:47:14 +0300
> Subject: [PATCH 3/5] s4: improve net output
> 
> ---
>  source4/utils/net/net.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/source4/utils/net/net.c b/source4/utils/net/net.c
> index fc34e84..39c5aec 100644
> --- a/source4/utils/net/net.c
> +++ b/source4/utils/net/net.c
> @@ -166,13 +166,16 @@ int net_run_usage(struct net_context *ctx,
>  			const struct net_functable *functable)
>  {
>  	int i;
> +	bool found = false;
>  	PyObject *py_cmds, *py_cmd;
>  
>  	for (i=0; functable[i].name; i++) {
> -		if (strcasecmp_m(argv[0], functable[i].name) == 0)
> +		if (strcasecmp_m(argv[0], functable[i].name) == 0) {
> +			found = true;
>  			if (functable[i].usage) {
>  				return functable[i].usage(ctx, argc-1, argv+1);
>  			}
> +		}
>  	}
>  
>  	py_cmds = py_commands();
> @@ -186,7 +189,7 @@ int net_run_usage(struct net_context *ctx,
>                                                  argv+1);
>  	}
>  
> -	d_printf("No usage information for command: %s\n", argv[0]);
> +	d_printf("%s: %s\n", (found == true)?"No usage information for command":"Unknown command",argv[0]);
Please use a simple:
	if (found)
		printf("FOO")
	else
		printf("BAR")

here, it's more readable than this syntax.

+1 otherwise
>  
>  	return 1;
>  }
> -- 
> 1.6.3.3

 From a7c16d3990ed16b9f6680172b7922dd8281bf567 Mon Sep 17 00:00:00 2001
> From: Matthieu Patou <mat at matws.net>
> Date: Fri, 8 Jan 2010 19:38:21 +0300
> Subject: [PATCH 2/5] s4: make net help foo work and use net_run_usage
+1

> From f5f080827f23260c742094a6de3a5ad550ad0829 Mon Sep 17 00:00:00 2001
> From: Matthieu Patou <mat at matws.net>
> Date: Fri, 8 Jan 2010 19:37:34 +0300
> Subject: [PATCH 1/5] s4: Uniformise net help output between python and builtin functions
+1 

> From 1fc9d9871b63280303b3b2376606676bf46811d5 Mon Sep 17 00:00:00 2001
> From: Matthieu Patou <mat at matws.net>
> Date: Mon, 11 Jan 2010 20:13:28 +0300
> Subject: [PATCH 5/5] s4: improve net docs
> 
> ---
>  source4/utils/net/net.c               |   39 ++++++++++++++++++++++++--------
>  source4/utils/net/net.h               |    1 +
>  source4/utils/net/net_export_keytab.c |   14 +++++++----
>  source4/utils/net/net_join.c          |    8 ++++--
>  source4/utils/net/net_machinepw.c     |    6 +++-
>  source4/utils/net/net_password.c      |   25 +++++++++++----------
>  source4/utils/net/net_time.c          |    2 +
>  source4/utils/net/net_user.c          |   25 +++++++++++++++++----
>  source4/utils/net/net_vampire.c       |   39 +++++++++++++++++++++++++++------
>  9 files changed, 115 insertions(+), 44 deletions(-)
> 
> diff --git a/source4/utils/net/net.c b/source4/utils/net/net.c
> index 850fc0c..7a7fa36 100644
> --- a/source4/utils/net/net.c
> +++ b/source4/utils/net/net.c
> @@ -173,6 +173,9 @@ int net_run_usage(struct net_context *ctx,
>  		if (strcasecmp_m(argv[0], functable[i].name) == 0) {
>  			found = true;
>  			if (functable[i].usage) {
> +				if (argc == 1) {
> +					d_printf("Description: %s",functable[i].desc);
> +				}
^^ The convention is for usage to output a single line, I think we
should follow that. The list of subcommands will already mention 
the descriptions.


> From cfe9cb44ab3013fcd59e5c97619b350d14c0eec4 Mon Sep 17 00:00:00 2001
> From: Matthieu Patou <mat at matws.net>
> Date: Fri, 8 Jan 2010 21:31:17 +0300
> Subject: [PATCH 4/5] s4: make net help include help for switches as well
> 
> ---
>  source4/utils/net/net.c |    7 +++++--
>  source4/utils/net/net.h |    2 ++
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/source4/utils/net/net.c b/source4/utils/net/net.c
> index 39c5aec..850fc0c 100644
> --- a/source4/utils/net/net.c
> +++ b/source4/utils/net/net.c
> @@ -271,6 +271,7 @@ int net_help(struct net_context *ctx, const struct net_functable *ftable)
>  	d_printf("Available commands:\n");
>  	net_help_builtin(ftable);
>  	net_help_python();
> +	poptPrintHelp(*(ctx->con),stdout,0);
>  	return 0;
>  }
>  
> @@ -294,9 +295,9 @@ static int binary_net(int argc, const char **argv)
>  	const char **argv_new;
>  	struct tevent_context *ev;
>  	struct net_context *ctx = NULL;
> +	char str[10];
>  	poptContext pc;
>  	struct poptOption long_options[] = {
> -		POPT_AUTOHELP
>  		POPT_COMMON_SAMBA
>  		POPT_COMMON_CONNECTION
>  		POPT_COMMON_CREDENTIALS
> @@ -371,7 +372,9 @@ static int binary_net(int argc, const char **argv)
>  	ctx->lp_ctx = cmdline_lp_ctx;
>  	ctx->credentials = cmdline_credentials;
>  	ctx->event_ctx = ev;
> -
> +	ctx->con = &pc;
> +	snprintf(str,10,"%c        \0",13);
> +	poptSetOtherOptionHelp(pc, str );
^^ This looks extremely dodgy - why the 10 characters here?

The argument to poptSetOtherOptionHelp should be a string with
additional info - why can't we just pass in "" ?

I'm not sure if there is value at in listing the common options in "net
help". It makes it harder to get rid of the common options (since they
don't make sense for all net subcommands).

Cheers,

Jelmer


More information about the samba-technical mailing list