On Wed, Jun 21, 2017 at 05:22:12PM +0200, Ingo Schwarze wrote: > Hi Sebastien and Jeremie, > > Sebastien Marie wrote on Wed, Jun 21, 2017 at 12:26:14PM +0200: > > On Wed, Jun 21, 2017 at 11:39:53AM +0200, Jeremie Courreges-Anglas wrote: > >> Sebastien Marie <sema...@online.fr> writes: > > >>> But mandoc -Tlint complains about missing Mdocdate. > >>> $ mandoc -Tlint sysclean.8 > >>> mandoc: sysclean.8:17:5: STYLE: Mdocdate missing: Dd June > > >> With another project, I get this warning about Dd, plus a message > >> about a missing RCS Id. > > > ah, I don't have this one. But after checking, my man page has a > > $OpenBSD$ tag inside it... it should come from the mdoc.template I > > used to start writing the file. :) > > If I remove it, I have the same STYLE warning. > > You currently have the following options: > > 1. Ignore the two style messages. > As jmc@ rightly observed, and as the mandoc(1) manual says > below DIAGNOSTICS, a style recommendation is not an error, > and style messages are occasionally false positives. > Both of these messages occur at most once per page. > > Of course, having to ignore a message is not fully satisfactory, > and keeping false positives down to a minimum *is* among my > goals, even for style recommendations. > > 2. Put a dummy .\" $OpenBSD$ line and a manually maintained > .Dd $Mdocdate Month day year$ line into the file even though > your VCS won't expand it. > > That is icky, working around an issue in a non-intuitive way > instead of solving the root cause, so i can't really recommend it. > > 3. Use "mandoc -Ios= -Tlint" for checking pages that are supposed > to be portable. > > That is not fully satisfactory because it disables *all* operating > system specific style messages. It is likely that you do want to > follow OpenBSD style in general, just not those parts that apply > to the base system only. > > 4. Put something like ".Os ratpoison" into the manual page. > > I clearly recommend against that. It not only suppers from > the same problem as option 3 of disabling more than intended; > it also results in an inconsistent user experience with the > bottom line of manual pages (admittedly a minor point, though). > > 5. Use "mandoc -Tlint -Wwarning" for these projects. > > I clearly recommend against that. It disables even more than > option 3, namely, all style recommendations, most of which are > just as useful for portable projects as for base system manuals. > > As there is no fully satisfactory option, i propose the patch below. > It add the "-W portable" command line option, which is a variant of > "-W style" hiding messages that only apply to base system manuals. >
evening. i'm not entirely convinced... are there likely to be other style warnings that apply to our base manuals but not "portable"? if there's a list of things then maybe it makes sense. otherwise i wonder if we can;t try and make things clearer (style warning is not a mistake) and leave things for a bit longer to see whether STYLE by default is too much. jmc > In general, i don't like adding knobs. But when the choice is > between having innocent people harassed with false positives and > between providing a knob for saying which checks you want, in a way > that is actually needed in practice, then i prefer the knob to the > harassment. > > OK for the patch below? > > > According to mandoc man page, -Wwarning will avoid all styles warning. > > Here the complete list (taken from mandoc.h): > > The list of messages is also in the mandoc(1) manual, with an > explanation of each message. > > Yours, > Ingo > > > Index: main.c > =================================================================== > RCS file: /cvs/src/usr.bin/mandoc/main.c,v > retrieving revision 1.195 > diff -u -p -r1.195 main.c > --- main.c 3 Jun 2017 12:16:19 -0000 1.195 > +++ main.c 21 Jun 2017 15:09:46 -0000 > @@ -68,8 +68,8 @@ enum outt { > > struct curparse { > struct mparse *mp; > - enum mandoclevel wlevel; /* ignore messages below this */ > int wstop; /* stop after a file with a warning */ > + enum mandocerr mmin; /* ignore messages below this */ > enum outt outtype; /* which output to use */ > void *outdata; /* data for output */ > struct manoutput *outopts; /* output options */ > @@ -159,7 +159,7 @@ main(int argc, char *argv[]) > > memset(&curp, 0, sizeof(struct curparse)); > curp.outtype = OUTT_LOCALE; > - curp.wlevel = MANDOCLEVEL_BADARG; > + curp.mmin = MANDOCERR_MAX; > curp.outopts = &conf.output; > options = MPARSE_SO | MPARSE_UTF8 | MPARSE_LATIN1; > defos = NULL; > @@ -416,7 +416,7 @@ main(int argc, char *argv[]) > moptions(&options, auxpaths); > > mchars_alloc(); > - curp.mp = mparse_alloc(options, curp.wlevel, mmsg, defos); > + curp.mp = mparse_alloc(options, curp.mmin, mmsg, defos); > > /* > * Conditionally start up the lookaside buffer before parsing. > @@ -907,7 +907,7 @@ toptions(struct curparse *curp, char *ar > curp->outtype = OUTT_ASCII; > else if (0 == strcmp(arg, "lint")) { > curp->outtype = OUTT_LINT; > - curp->wlevel = MANDOCLEVEL_STYLE; > + curp->mmin = MANDOCERR_STYLE; > } else if (0 == strcmp(arg, "tree")) > curp->outtype = OUTT_TREE; > else if (0 == strcmp(arg, "man")) > @@ -936,16 +936,17 @@ static int > woptions(struct curparse *curp, char *arg) > { > char *v, *o; > - const char *toks[8]; > + const char *toks[9]; > > toks[0] = "stop"; > toks[1] = "all"; > toks[2] = "style"; > - toks[3] = "warning"; > - toks[4] = "error"; > - toks[5] = "unsupp"; > - toks[6] = "fatal"; > - toks[7] = NULL; > + toks[3] = "portable"; > + toks[4] = "warning"; > + toks[5] = "error"; > + toks[6] = "unsupp"; > + toks[7] = "fatal"; > + toks[8] = NULL; > > while (*arg) { > o = arg; > @@ -955,19 +956,22 @@ woptions(struct curparse *curp, char *ar > break; > case 1: > case 2: > - curp->wlevel = MANDOCLEVEL_STYLE; > + curp->mmin = MANDOCERR_STYLE; > break; > case 3: > - curp->wlevel = MANDOCLEVEL_WARNING; > + curp->mmin = MANDOCERR_PORTABLE; > break; > case 4: > - curp->wlevel = MANDOCLEVEL_ERROR; > + curp->mmin = MANDOCERR_WARNING; > break; > case 5: > - curp->wlevel = MANDOCLEVEL_UNSUPP; > + curp->mmin = MANDOCERR_ERROR; > break; > case 6: > - curp->wlevel = MANDOCLEVEL_BADARG; > + curp->mmin = MANDOCERR_UNSUPP; > + break; > + case 7: > + curp->mmin = MANDOCERR_MAX; > break; > default: > warnx("-W %s: Bad argument", o); > Index: mandoc.1 > =================================================================== > RCS file: /cvs/src/usr.bin/mandoc/mandoc.1,v > retrieving revision 1.125 > diff -u -p -r1.125 mandoc.1 > --- mandoc.1 17 Jun 2017 23:06:43 -0000 1.125 > +++ mandoc.1 21 Jun 2017 15:09:46 -0000 > @@ -158,7 +158,12 @@ or > .Cm unsupp ; > .Cm all > is an alias for > -.Cm style . > +.Cm style , > +and > +.Cm portable > +is a variant of > +.Cm style > +hiding style recommendations that only apply to base system manuals. > By default, > .Nm > is silent. > @@ -746,7 +751,7 @@ are hidden unless their level, or a lowe > option or > .Fl T Cm lint > output mode. > -.Ss Style messages > +.Pp > As indicated below, some style checks are only performed if a > specific operating system name occurs in the arguments of the > .Ic \&Os > @@ -756,6 +761,10 @@ command line option, or, if neither are > of the > .Xr uname 3 > function. > +.Ss Style messages for base system manuals > +These are not shown if > +.Fl W Cm portable > +is specified. > .Bl -ohang > .It Sy "Mdocdate found" > .Pq mdoc , Nx > @@ -778,6 +787,17 @@ macro does not use CVS > keyword substitution, but using it is conventionally expected in the > .Ox > base system. > +.It Sy "RCS id missing" > +.Pq Ox , Nx > +The manual page lacks the comment line with the RCS identifier > +generated by CVS > +.Ic OpenBSD > +or > +.Ic NetBSD > +keyword substitution as conventionally used in these operating systems. > +.El > +.Ss Style messages for all manual pages > +.Bl -ohang > .It Sy "legacy man(7) date format" > .Pq mdoc > The > @@ -791,14 +811,6 @@ Consider using the conventional > date format > .Dq "Month dd, yyyy" > instead. > -.It Sy "RCS id missing" > -.Pq Ox , Nx > -The manual page lacks the comment line with the RCS identifier > -generated by CVS > -.Ic OpenBSD > -or > -.Ic NetBSD > -keyword substitution as conventionally used in these operating systems. > .It Sy "duplicate RCS id" > A single manual page contains two copies of the RCS identifier for > the same operating system. > Index: mandoc.h > =================================================================== > RCS file: /cvs/src/usr.bin/mandoc/mandoc.h,v > retrieving revision 1.174 > diff -u -p -r1.174 mandoc.h > --- mandoc.h 17 Jun 2017 23:06:43 -0000 1.174 > +++ mandoc.h 21 Jun 2017 15:09:46 -0000 > @@ -48,8 +48,11 @@ enum mandocerr { > > MANDOCERR_MDOCDATE, /* Mdocdate found: Dd ... */ > MANDOCERR_MDOCDATE_MISSING, /* Mdocdate missing: Dd ... */ > - MANDOCERR_DATE_LEGACY, /* legacy man(7) date format: Dd ... */ > MANDOCERR_RCS_MISSING, /* RCS id missing */ > + > + MANDOCERR_PORTABLE, /* ===== style hints for portable ===== */ > + > + MANDOCERR_DATE_LEGACY, /* legacy man(7) date format: Dd ... */ > MANDOCERR_RCS_REP, /* duplicate RCS id: ... */ > MANDOCERR_MACRO_USELESS, /* useless macro: macro */ > MANDOCERR_BX, /* consider using OS macro: macro */ > @@ -448,7 +451,7 @@ const char *mchars_uc2str(int); > int mchars_num2uc(const char *, size_t); > int mchars_spec2cp(const char *, size_t); > const char *mchars_spec2str(const char *, size_t, size_t *); > -struct mparse *mparse_alloc(int, enum mandoclevel, mandocmsg, const > char *); > +struct mparse *mparse_alloc(int, enum mandocerr, mandocmsg, const > char *); > void mparse_free(struct mparse *); > void mparse_keep(struct mparse *); > int mparse_open(struct mparse *, const char *); > Index: read.c > =================================================================== > RCS file: /cvs/src/usr.bin/mandoc/read.c,v > retrieving revision 1.150 > diff -u -p -r1.150 read.c > --- read.c 17 Jun 2017 23:06:43 -0000 1.150 > +++ read.c 21 Jun 2017 15:09:46 -0000 > @@ -52,7 +52,7 @@ struct mparse { > const char *defos; /* default operating system */ > mandocmsg mmsg; /* warning/error message handler */ > enum mandoclevel file_status; /* status of current parse */ > - enum mandoclevel wlevel; /* ignore messages below this */ > + enum mandocerr mmin; /* ignore messages below this */ > int options; /* parser options */ > int gzip; /* current input file is gzipped */ > int filenc; /* encoding of the current file */ > @@ -82,12 +82,15 @@ static const enum mandocerr mandoclimits > static const char * const mandocerrs[MANDOCERR_MAX] = { > "ok", > > - "generic style suggestion", > + "base system convention", > > "Mdocdate found", > "Mdocdate missing", > - "legacy man(7) date format", > "RCS id missing", > + > + "generic style suggestion", > + > + "legacy man(7) date format", > "duplicate RCS id", > "useless macro", > "consider using OS macro", > @@ -738,7 +741,7 @@ mparse_open(struct mparse *curp, const c > } > > struct mparse * > -mparse_alloc(int options, enum mandoclevel wlevel, mandocmsg mmsg, > +mparse_alloc(int options, enum mandocerr mmin, mandocmsg mmsg, > const char *defos) > { > struct mparse *curp; > @@ -746,7 +749,7 @@ mparse_alloc(int options, enum mandoclev > curp = mandoc_calloc(1, sizeof(struct mparse)); > > curp->options = options; > - curp->wlevel = wlevel; > + curp->mmin = mmin; > curp->mmsg = mmsg; > curp->defos = defos; > > @@ -838,12 +841,12 @@ mandoc_msg(enum mandocerr er, struct mpa > { > enum mandoclevel level; > > + if (er < m->mmin && er != MANDOCERR_FILE) > + return; > + > level = MANDOCLEVEL_UNSUPP; > while (er < mandoclimits[level]) > level--; > - > - if (level < m->wlevel && er != MANDOCERR_FILE) > - return; > > if (m->mmsg) > (*m->mmsg)(er, level, m->file, ln, col, msg); >