Jason McIntyre <[email protected]> writes:
> 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.
>>
Even if I was fine with option 1, I was going to ok Ingo's diff, since
it helps people who use mandoc but don't care about RCS keywords.
However I think Jason may have a point.
> 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)
Well, if a style warning is not a mistake, then should the exit status
be 1?
~/src/ratpoison/doc$ mandoc -Tlint ratpoison.mdoc.1 ; echo $?
mandoc: ratpoison.mdoc.1:25:5: STYLE: Mdocdate missing: Dd May
mandoc: ratpoison.mdoc.1: STYLE: RCS id missing
1
> and leave things for a bit longer to see
> whether STYLE by default is too much.
But are those actually style warnings, or errors? "STYLE" alone is a bit of
a misnomer, the warnings above talk about cvs because OpenBSD and NetBSD
use certain RCS keywords. Maybe this kind of error should only trigger
when checking OpenBSD/NetBSD manpages, and the error message should
mention "OS" or OPENBSD" (or whatever is needed for people to decide
whether they care). Wouldn't this kind of check be more suitable for
the "manlint" target in bsd.man.mk?
Also, what would be the default behavior for portable mandoc?
Sorry for bringing more questions than answers. :)
> 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);
>>
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
