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 <[email protected]> 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);
>
