On Sat, Feb 03, 2018 at 09:28:11PM +0100, Ingo Schwarze wrote:
> Hi Jason,
> 
> Jason McIntyre wrote on Sat, Feb 03, 2018 at 07:23:59PM +0000:
> > On Sat, Feb 03, 2018 at 08:10:14PM +0100, Ingo Schwarze wrote:
> >> Jason McIntyre wrote on Sat, Feb 03, 2018 at 05:41:01PM +0000:
> 
> >>> can of worms, there's still stuff like :exusage
> 
> >> I think that can remain as it is.  It isn't very precise anyway,
> >> with very short half-line descriptions only and not saying how
> >> the ex commands can be abbreviated.  So at least the usage of []
> >> isn't inconsistent in there.
> 
> > i think it's worth taking the time to fix. if it gets deleted, that's
> > another thing, but until then why mislead people? it will be simpler
> > and clearer just to list lowercase variants.
> 
> See below for a patch to do that.
> 

i'll take your word for it that it does what you say ;)

> Note that the man page and :exusage are already out of sync in multiple
> respects, mostly in the sense of using gratuitiously different
> wordings.  I didn't try to fix that in this patch, to not expand
> the scope of the patch beyond what is reasonable, and did not check
> whether anything unrelated in there is actually wrong.
> 

agreed

> >>> and the USD docs to update. so a man page fix will not suffice.
> 
> >> I wouldn't bother.  They are not installed, and the base system
> >> doesn't even provide tools to process them.  If we ever decide
> >> to do anything with them, they will require a full check of accuracy
> >> anyway.
> 
> > actually a fair bit of time has been spent on vi's USD docs. it would be
> > a shame to let that slip. i'm not saying they're 100% in sync, but why
> > worsen it? we could have (another) discussion about removing them but
> > until then, i think it's worth trying to keep this up to date. if
> > nothing else, it will make someone else's job easier in the future if
> > they try to check it.
> 
> Even if we consider maintaining the USD docs worthwhile, i don't think
> that ought to delay fixing the manual page.
> 

hmm. not that i entirely agree with this sentiment but fair enough, i will
look at updating the USD docs.

> > diff reads ok, but one comment:
> [...]
> >> +For
> >> +.Cm e ,
> >> +.Cm fg ,
> >> +.Cm n ,
> >> +.Cm prev ,
> >> +.Cm ta ,
> >> +and
> >> +.Cm vi ,
> >> +if the first letter of the command is capitalized, the current screen is
> >> +split and the new file is displayed in addition to the current screen.
> >> +This feature is only available in
> >> +.Nm vi ,
> >> +not in
> >> +.Nm ex .
> 
> > it would be better, i think, to emphasise the behaviour rather than the
> > commands. so i'd do it the other way round:
> > 
> >     It is possible to start a command in a new screen by ...
> >     This applies to the commands ..., in vi mode only.
> > 
> > like that? again, no biggie.
> 
> Actually, i don't like that.  That makes it sound as if it were a
> general principle applicable to most commands, while it is actually
> a non-standard, non-portable, poorly designed and barely working
> quirk implemented only for a handful of commands.  So i think
> emphasizing the limitations rather than the benefits serves the
> users better.  I'd rather discourage it even more, if i could see
> a good way to do so without additional verbiage.
> 

a good way to do it without adding verbiage would be to not document it!
if it really is a crappy quirk, let's just leave it out then.
i presumed it was all meant to be there...

> Yours,
>   Ingo
> 
> 
> Index: ex/ex_cmd.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/ex/ex_cmd.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 ex_cmd.c
> --- ex/ex_cmd.c       19 Nov 2015 07:53:31 -0000      1.10
> +++ ex/ex_cmd.c       3 Feb 2018 20:10:59 -0000
> @@ -111,7 +111,7 @@ EXCMDLIST const cmds[] = {
>       {"bg",          ex_bg,          E_VIONLY,
>           "",
>           "bg",
> -         "put a foreground screen into the background"},
> +         "put the current screen into the background"},
>  /* C_CHANGE */
>       {"change",      ex_change,      E_ADDR2|E_ADDR_ZERODEF,
>           "!ca",
> @@ -150,12 +150,12 @@ EXCMDLIST const cmds[] = {
>  /* C_EDIT */
>       {"edit",        ex_edit,        E_NEWSCREEN,
>           "f1o",
> -         "[Ee][dit][!] [+cmd] [file]",
> +         "e[dit][!] [+cmd] [file]",
>           "begin editing another file"},
>  /* C_EX */
>       {"ex",          ex_edit,        E_NEWSCREEN,
>           "f1o",
> -         "[Ee]x[!] [+cmd] [file]",
> +         "ex[!] [+cmd] [file]",
>           "begin editing another file"},
>  /* C_EXUSAGE */
>       {"exusage",     ex_usage,       0,
> @@ -170,7 +170,7 @@ EXCMDLIST const cmds[] = {
>  /* C_FG */
>       {"fg",          ex_fg,          E_NEWSCREEN|E_VIONLY,
>           "f1o",
> -         "[Ff]g [file]",
> +         "fg [file]",
>           "bring a backgrounded screen into the foreground"},
>  /* C_GLOBAL */
>       {"global",      ex_global,      E_ADDR2_ALL,
> @@ -225,7 +225,7 @@ EXCMDLIST const cmds[] = {
>  /* C_NEXT */
>       {"next",        ex_next,        E_NEWSCREEN,
>           "!fN",
> -         "[Nn][ext][!] [+cmd] [file ...]",
> +         "n[ext][!] [+cmd] [file ...]",
>           "edit (and optionally specify) the next file"},
>  /* C_NUMBER */
>       {"number",      ex_number,      E_ADDR2|E_CLRFLAG,
> @@ -250,7 +250,7 @@ EXCMDLIST const cmds[] = {
>  /* C_PREVIOUS */
>       {"previous",    ex_prev,        E_NEWSCREEN,
>           "!",
> -         "[Pp]rev[ious][!]",
> +         "prev[ious][!]",
>           "edit the previous file in the file argument list"},
>  /* C_PUT */
>       {"put",         ex_put, 
> @@ -262,7 +262,7 @@ EXCMDLIST const cmds[] = {
>       {"quit",        ex_quit,        0,
>           "!",
>           "q[uit][!]",
> -         "exit ex/vi"},
> +         "close the current screen"},
>  /* C_READ */
>       {"read",        ex_read,        E_ADDR1|E_ADDR_ZERO|E_ADDR_ZERODEF,
>           "s",
> @@ -331,7 +331,7 @@ EXCMDLIST const cmds[] = {
>  /* C_TAG */
>       {"tag",         ex_tag_push,    E_NEWSCREEN,
>           "!w1o",
> -         "[Tt]a[g][!] [string]",
> +         "ta[g][!] [string]",
>           "edit the file containing the tag"},
>  /* C_TAGNEXT */
>       {"tagnext",     ex_tag_next,    0,
> @@ -386,7 +386,7 @@ EXCMDLIST const cmds[] = {
>  /* C_VISUAL_VI */
>       {"visual",      ex_edit,        E_NEWSCREEN,
>           "f1o",
> -         "[Vv]i[sual][!] [+cmd] [file]",
> +         "vi[sual][!] [+cmd] [file]",
>           "edit another file (from vi mode only)"},
>  /* C_VIUSAGE */
>       {"viusage",     ex_viusage,     0,
> @@ -407,12 +407,12 @@ EXCMDLIST const cmds[] = {
>       {"wq",          ex_wq,          E_ADDR2_ALL|E_ADDR_ZERODEF,
>           "!s",
>           "[line [,line]] wq[!] [>>] [file]",
> -         "write the file and exit"},
> +         "write the file and close the current screen"},
>  /* C_XIT */
>       {"xit",         ex_xit,         E_ADDR2_ALL|E_ADDR_ZERODEF,
>           "!f1o",
>           "[line [,line]] x[it][!] [file]",
> -         "exit"},
> +         "write if modified and close the current screen"},
>  /* C_YANK */
>       {"yank",        ex_yank,        E_ADDR2,
>           "bca",
> Index: ex/ex_usage.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/ex/ex_usage.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 ex_usage.c
> --- ex/ex_usage.c     12 Nov 2014 04:28:41 -0000      1.8
> +++ ex/ex_usage.c     3 Feb 2018 20:10:59 -0000
> @@ -58,7 +58,7 @@ ex_usage(SCR *sp, EXCMD *cmdp)
>       ARGS *ap;
>       EXCMDLIST const *cp;
>       int newscreen;
> -     char *name, *p, nb[MAXCMDNAMELEN + 5];
> +     char *p;
>  
>       switch (cmdp->argc) {
>       case 1:
> @@ -96,30 +96,11 @@ ex_usage(SCR *sp, EXCMD *cmdp)
>               }
>               break;
>       case 0:
> -             for (cp = cmds; cp->name != NULL && !INTERRUPTED(sp); ++cp) {
> -                     /*
> -                      * The ^D command has an unprintable name.
> -                      *
> -                      * XXX
> -                      * We display both capital and lower-case versions of
> -                      * the appropriate commands -- no need to add in extra
> -                      * room, they're all short names.
> -                      */
> -                     if (cp == &cmds[C_SCROLL])
> -                             name = "^D";
> -                     else if (F_ISSET(cp, E_NEWSCREEN)) {
> -                             nb[0] = '[';
> -                             nb[1] = toupper(cp->name[0]);
> -                             nb[2] = cp->name[0];
> -                             nb[3] = ']';
> -                             for (name = cp->name + 1,
> -                                 p = nb + 4; (*p++ = *name++) != '\0';);
> -                             name = nb;
> -                     } else
> -                             name = cp->name;
> -                     (void)ex_printf(sp,
> -                         "%*s: %s\n", MAXCMDNAMELEN, name, cp->help);
> -             }
> +             for (cp = cmds; cp->name != NULL && !INTERRUPTED(sp); ++cp)
> +                     (void)ex_printf(sp, "%*s: %s\n", MAXCMDNAMELEN,
> +                         /* The ^D command has an unprintable name. */
> +                         cp == &cmds[C_SCROLL] ? "^D" : cp->name,
> +                         cp->help);
>               break;
>       default:
>               abort();
> 

Reply via email to