You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by cm...@collab.net on 2002/04/14 07:04:23 UTC
[PATCH] Lining up option descriptions in cmdline binary help
My brain feels super-mushy right now, so rather than commit this chunk
of elementary code, I'll send it to the list for review. I feel sure
that there's some better way to do what I did here (hopefully not
involving a series of single-space apr_pstrcat's) but the old
Thinker's coming up shorthanded. At any rate, the SEGFAULT protection
code needs to be committed.
--
* subversion/clients/cmdline/main.c
(format_option): Fix potential segfault when OPT is NULL. Also,
stop using tabs to supposedly attempt to line up the documentation
strings ... it just doesn't work.
Index: ./subversion/clients/cmdline/main.c
===================================================================
--- ./subversion/clients/cmdline/main.c
+++ ./subversion/clients/cmdline/main.c Sun Apr 14 01:58:22 2002
@@ -351,20 +351,43 @@
apr_pool_t *pool)
{
char *opts;
+ char optsw[24]; /* ### this needs to be at least as long as the
+ longest option string up to the optional ARG. */
+ /* If there is no option, just return a '?' string. */
if (opt == NULL)
- *string = apr_psprintf (pool, "?");
+ {
+ *string = apr_psprintf (pool, "?");
+ return;
+ }
+ /* Else, we have a valid option which may or may not have a "short
+ name" (a single-character alias for the long option). */
if (opt->optch <= 255)
opts = apr_psprintf (pool, "-%c [--%s]", opt->optch, opt->name);
else
opts = apr_psprintf (pool, "--%s", opt->name);
+ /* If option takes an argument, specify this in the string. */
if (opt->has_arg)
opts = apr_pstrcat (pool, opts, " arg", NULL);
+ /* If there exists documentation for the option, add this to the
+ string as well, starting at the sizeof(optsw)'th column. */
if (doc)
- opts = apr_pstrcat (pool, opts, ":\t", opt->description, NULL);
+ {
+ int opts_len = strlen (opts);
+ int optsw_len = sizeof (optsw) / sizeof (*optsw);
+ int i;
+
+ /* Pad the existing OPTS string up to sizeof(optsw) with, well,
+ spaces. */
+ for (i = 0; i < (optsw_len - 1); i++)
+ optsw[i] = ((i < opts_len) ? opts[i] : ' ');
+
+ optsw[optsw_len - 1] = 0;
+ opts = apr_psprintf (pool, "%s : %s", optsw, opt->description);
+ }
*string = opts;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] Lining up option descriptions in cmdline binary help
Posted by Sander Striker <st...@apache.org>.
> From: cmpilato@collab.net [mailto:cmpilato@collab.net]
> Sent: 14 April 2002 09:17
> cmpilato@collab.net writes:
>
>> My brain feels super-mushy right now, so rather than commit this chunk
>> of elementary code, I'll send it to the list for review. I feel sure
>> that there's some better way to do what I did here (hopefully not
>> involving a series of single-space apr_pstrcat's) but the old
>> Thinker's coming up shorthanded.
Well, it took quite some people to get to a simple solution ;)
Committed in rev 1700.
Very nice idea Mike!
$ svn help commit
commit (ci): Send changes from your working copy to the repository.
usage: svn commit [TARGETS]
Be sure to use one of -m or -F to send a log message;
the -r switch is only for use with --xml-file.
Valid options:
-m [--message] arg : specify commit message "ARG"
-F [--file] arg : read data from file ARG
-q [--quiet] : print as little as possible
--targets arg : pass contents of file "ARG" as additional args
--force : force operation to run
--username arg : specify a username ARG
--password arg : specify a password ARG
--xml-file arg : read/write xml to specified file ARG
-r [--revision] arg : specify revision number ARG (or X:Y range)
> At any rate, the SEGFAULT protection code needs to be committed.
Done (rev 1699).
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Lining up option descriptions in cmdline binary help
Posted by cm...@collab.net.
cmpilato@collab.net writes:
> My brain feels super-mushy right now, so rather than commit this chunk
> of elementary code, I'll send it to the list for review. I feel sure
> that there's some better way to do what I did here (hopefully not
> involving a series of single-space apr_pstrcat's) but the old
> Thinker's coming up shorthanded. At any rate, the SEGFAULT protection
> code needs to be committed.
>
> --
>
> * subversion/clients/cmdline/main.c
>
> (format_option): Fix potential segfault when OPT is NULL. Also,
> stop using tabs to supposedly attempt to line up the documentation
> strings ... it just doesn't work.
For those wondering what this patch was all about, my goal was to turn
this (output from, say, `svn help commit'):
[...]
Valid options:
-m [--message] arg: specify commit message "ARG"
-F [--file] arg: read data from file ARG
-q [--quiet]: print as little as possible
--targets arg: pass contents of file "ARG" as additional args
--force: force operation to run
--username arg: specify a username ARG
--password arg: specify a password ARG
--xml-file arg: read/write xml to specified file ARG
-r [--revision] arg: specify revision number ARG (or X:Y range)
into this, which I find soooo much easier to digest:
[...]
Valid options:
-m [--message] arg : specify commit message "ARG"
-F [--file] arg : read data from file ARG
-q [--quiet] : print as little as possible
--targets arg : pass contents of file "ARG" as additional args
--force : force operation to run
--username arg : specify a username ARG
--password arg : specify a password ARG
--xml-file arg : read/write xml to specified file ARG
-r [--revision] arg : specify revision number ARG (or X:Y range)
A total picky thing, I know, but programmers need their artistic sides
catered to as well.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] Lining up option descriptions in cmdline binary help
Posted by Sander Striker <st...@apache.org>.
> From: Karl Fogel [mailto:kfogel@newton.ch.collab.net]
> Sent: 14 April 2002 10:03
> cmpilato@collab.net writes:
> > My brain feels super-mushy right now, so rather than commit this chunk
> > of elementary code, I'll send it to the list for review. I feel sure
> > that there's some better way to do what I did here (hopefully not
> > involving a series of single-space apr_pstrcat's) but the old
> > Thinker's coming up shorthanded. At any rate, the SEGFAULT protection
> > code needs to be committed.
>
> After your instantaneous tweaks to my patch for text base checksums, I
> feel I owe you one :-). My contribution here is relatively minor, but
> you're welcome to it...
>
> > Index: ./subversion/clients/cmdline/main.c
> > ===================================================================
> > --- ./subversion/clients/cmdline/main.c
> > +++ ./subversion/clients/cmdline/main.c Sun Apr 14 01:58:22 2002
> > @@ -351,20 +351,43 @@
> > apr_pool_t *pool)
> > {
> > char *opts;
> > + char optsw[24]; /* ### this needs to be at least as long as the
> > + longest option string up to the optional ARG. */
>
> There's some C syntax for initializing the whole array to the same
> value, right?
Not that I know of. Or at least, I've never seen it ;)
> I'm thinking spaces (' '), fwiw. Don't have my K&R
> here, and google is not as forthcoming as I expected.
>
> If there's not a way, just init by hand I suppose, no big deal.
A simple memset would do the trick:
memset (optsw, ' ', sizeof(optsw));
optsw[sizeof(optsw) - 1] = '\0';
Or something like that. Ofcourse some simple for loop will suffice.
It's only 20 bytes ;)
> > if (doc)
> > - opts = apr_pstrcat (pool, opts, ":\t", opt->description, NULL);
> > + {
> > + int opts_len = strlen (opts);
> > + int optsw_len = sizeof (optsw) / sizeof (*optsw);
> > + int i;
> > +
> > + /* Pad the existing OPTS string up to sizeof(optsw) with, well,
> > + spaces. */
> > + for (i = 0; i < (optsw_len - 1); i++)
> > + optsw[i] = ((i < opts_len) ? opts[i] : ' ');
> > +
> > + optsw[optsw_len - 1] = 0;
> > + opts = apr_psprintf (pool, "%s : %s", optsw, opt->description);
> > + }
> >
>
> Then here, I'm thinking everything between the curly braces can be
> replaced with just this:
>
> opts = apr_psprintf (pool, "%s%s : %s",
> opts, optsw + strlen (ops), opt->description);
Yes, nice ;)
> Untested, and I hereby disclaim all o.b.o.e. playing in this area. :-)
> ("off by one error", for those not familiar)
*grin*
> -Karl
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Lining up option descriptions in cmdline binary help
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
cmpilato@collab.net writes:
> My brain feels super-mushy right now, so rather than commit this chunk
> of elementary code, I'll send it to the list for review. I feel sure
> that there's some better way to do what I did here (hopefully not
> involving a series of single-space apr_pstrcat's) but the old
> Thinker's coming up shorthanded. At any rate, the SEGFAULT protection
> code needs to be committed.
After your instantaneous tweaks to my patch for text base checksums, I
feel I owe you one :-). My contribution here is relatively minor, but
you're welcome to it...
> Index: ./subversion/clients/cmdline/main.c
> ===================================================================
> --- ./subversion/clients/cmdline/main.c
> +++ ./subversion/clients/cmdline/main.c Sun Apr 14 01:58:22 2002
> @@ -351,20 +351,43 @@
> apr_pool_t *pool)
> {
> char *opts;
> + char optsw[24]; /* ### this needs to be at least as long as the
> + longest option string up to the optional ARG. */
There's some C syntax for initializing the whole array to the same
value, right? I'm thinking spaces (' '), fwiw. Don't have my K&R
here, and google is not as forthcoming as I expected.
If there's not a way, just init by hand I suppose, no big deal.
> if (doc)
> - opts = apr_pstrcat (pool, opts, ":\t", opt->description, NULL);
> + {
> + int opts_len = strlen (opts);
> + int optsw_len = sizeof (optsw) / sizeof (*optsw);
> + int i;
> +
> + /* Pad the existing OPTS string up to sizeof(optsw) with, well,
> + spaces. */
> + for (i = 0; i < (optsw_len - 1); i++)
> + optsw[i] = ((i < opts_len) ? opts[i] : ' ');
> +
> + optsw[optsw_len - 1] = 0;
> + opts = apr_psprintf (pool, "%s : %s", optsw, opt->description);
> + }
>
Then here, I'm thinking everything between the curly braces can be
replaced with just this:
opts = apr_psprintf (pool, "%s%s : %s",
opts, optsw + strlen (ops), opt->description);
Untested, and I hereby disclaim all o.b.o.e. playing in this area. :-)
("off by one error", for those not familiar)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Lining up option descriptions in cmdline binary help
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
"Sander Striker" <st...@apache.org> writes:
> > From: Peter Mathiasson [mailto:peter@mathiasson.nu]
> > Like sprintf("%-50s : %s",optsw,opt->description);
>
> Duh. Sometimes things are so simple, you don't think of them ;)
> Thanks Peter.
+ding+! Same here.
:-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] Lining up option descriptions in cmdline binary help
Posted by Sander Striker <st...@apache.org>.
> From: Peter Mathiasson [mailto:peter@mathiasson.nu]
> Sent: 14 April 2002 10:18
> On Sun, Apr 14, 2002 at 02:04:23AM -0500, cmpilato@collab.net wrote:
> > + /* If there exists documentation for the option, add this to the
> > + string as well, starting at the sizeof(optsw)'th column. */
> > if (doc)
> > - opts = apr_pstrcat (pool, opts, ":\t", opt->description, NULL);
> > + {
> > + int opts_len = strlen (opts);
> > + int optsw_len = sizeof (optsw) / sizeof (*optsw);
> > + int i;
> > +
> > + /* Pad the existing OPTS string up to sizeof(optsw) with, well,
> > + spaces. */
> > + for (i = 0; i < (optsw_len - 1); i++)
> > + optsw[i] = ((i < opts_len) ? opts[i] : ' ');
> > +
> > + optsw[optsw_len - 1] = 0;
> > + opts = apr_psprintf (pool, "%s : %s", optsw, opt->description);
> > + }
>
> Wouldn't it be easier just to use length specifier with sprintf? (I'm
> not familiar with apr_psprintf, but I can't imagine that has been
> removed).
> Like sprintf("%-50s : %s",optsw,opt->description);
Duh. Sometimes things are so simple, you don't think of them ;)
Thanks Peter.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Lining up option descriptions in cmdline binary help
Posted by Peter Mathiasson <pe...@mathiasson.nu>.
On Sun, Apr 14, 2002 at 02:04:23AM -0500, cmpilato@collab.net wrote:
> + /* If there exists documentation for the option, add this to the
> + string as well, starting at the sizeof(optsw)'th column. */
> if (doc)
> - opts = apr_pstrcat (pool, opts, ":\t", opt->description, NULL);
> + {
> + int opts_len = strlen (opts);
> + int optsw_len = sizeof (optsw) / sizeof (*optsw);
> + int i;
> +
> + /* Pad the existing OPTS string up to sizeof(optsw) with, well,
> + spaces. */
> + for (i = 0; i < (optsw_len - 1); i++)
> + optsw[i] = ((i < opts_len) ? opts[i] : ' ');
> +
> + optsw[optsw_len - 1] = 0;
> + opts = apr_psprintf (pool, "%s : %s", optsw, opt->description);
> + }
Wouldn't it be easier just to use length specifier with sprintf? (I'm
not familiar with apr_psprintf, but I can't imagine that has been
removed).
Like sprintf("%-50s : %s",optsw,opt->description);
--
Peter Mathiasson, peter at mathiasson dot nu, http://www.mathiasson.nu
GPG Fingerprint: A9A7 F8F6 9821 F415 B066 77F1 7FF5 C2E6 7BF2 F228