From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from scc-mailout-kit-01-web.scc.kit.edu (scc-mailout-kit-01-web.scc.kit.edu [129.13.231.93]) by fantadrom.bsd.lv (OpenSMTPD) with ESMTP id 4e7f4dbe for ; Thu, 12 Nov 2015 16:31:40 -0500 (EST) Received: from asta-nat.asta.uni-karlsruhe.de ([172.22.63.82] helo=hekate.usta.de) by scc-mailout-kit-01.scc.kit.edu with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (envelope-from ) id 1ZwzSy-0000RP-1U; Thu, 12 Nov 2015 22:31:37 +0100 Received: from donnerwolke.usta.de ([172.24.96.3]) by hekate.usta.de with esmtp (Exim 4.77) (envelope-from ) id 1ZwzSx-0002uY-V0; Thu, 12 Nov 2015 22:31:35 +0100 Received: from athene.usta.de ([172.24.96.10]) by donnerwolke.usta.de with esmtp (Exim 4.84) (envelope-from ) id 1ZwzSx-00079B-Qf; Thu, 12 Nov 2015 22:31:35 +0100 Received: from localhost (1031@localhost [local]); by localhost (OpenSMTPD) with ESMTPA id 434a6381; Thu, 12 Nov 2015 22:31:35 +0100 (CET) Date: Thu, 12 Nov 2015 22:31:35 +0100 From: Ingo Schwarze To: Dag-Erling Smoergrav Cc: discuss@mdocml.bsd.lv Subject: Re: Mdocdate Message-ID: <20151112213135.GL20023@athene.usta.de> References: <86r3jva7s9.fsf@desk.des.no> X-Mailinglist: mdocml-discuss Reply-To: discuss@mdocml.bsd.lv MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <86r3jva7s9.fsf@desk.des.no> User-Agent: Mutt/1.5.23 (2014-03-12) Hi Dag-Erling, thanks for caring about mandoc! Dag-Erling Smoergrav wrote on Thu, Nov 12, 2015 at 10:25:42AM +0100: > The attached patch improves the Mdocdate code and updates the > documentation to match. > > - in a2time(): > > Skip past "$Mdocdate: " if it exists instead of relying on the > caller to specify it in the format string. No, that would introduce a bug. The format is different depending on whether or not Mdocdate is used. Maybe deplorable, but about a decade too late to change. Well, it could be changed with a lot of churn in the tree, but for what benefit? People don't need to type in the Mdocdate format manually, after all, and even if they do that and get it wrong, cvs(1) will helpfully fix it for them. > Accept trailing text after a successful match (e.g. "$"). Why? That seems like a step backwards, weakening syntax validation. > If the first character after a successful match is 'Z', use UTC > instead of the local time zone. That makes no sense. Neither .TH nor .Dd syntax allows time formats more fine-grained than days, so it makes no sense to worry about time zones. > - in time2a(): > > Use a sufficiently large stack buffer and mandoc_strdup() instead of > trying to guess at the correct length. This does look useful, it simplifies the code. > - in mandoc_normdate(): > > Fix several logic errors (inverted tests), Err, what? Can you be more specific? I don't see any logic errors in there. But your changes break several features. > add a test for the Subversion %d format, That is not valid mdoc(7) .Dd syntax, so it can't be added. > and simplify the code. Yes, some of that seems useful. > - in the man page: > > Document how to use Mdocdate with Subversion. No. Please don't introduce yet another format, use something like Mdocdate="%b %d %Y " or whatever the subversion syntax is for svn:keywords. > Given the following test cases: > > % cat tests > February 7 2036 > February 7 2036 > February 07 2036 These are all invalid and must generate a warning. > February 7, 2036 > February 7, 2036 > February 07, 2036 These are valid, but not with Mdocdate. > Feb 7 2036 > Feb 7 2036 > Feb 07 2036 Invalid. > Feb 7, 2036 > Feb 7, 2036 > Feb 07, 2036 > 2036-02-07 > 2036-02-7 > 2036-2-07 > 2036-2-7 Valid, but not with Mdocdate. > $Mdocdate: February 7 2036 $ Valid. > $Mdocdate: 2036-02-07 06:28:16Z $ Invalid. > The current code (1.13.3) produces the following output: > > % while read d ; do echo ".Dd $d" | mandoc | tail -1 ; done February 7 2036 > February 7 2036 > February 07 2036 > February 7, 2036 > February 7, 2036 > February 7, 2036 > Feb 7 2036 > Feb 7 2036 > Feb 07 2036 > February 7, 2036 > February 7, 2036 > February 7, 2036 > 2036-02-07 > 2036-02-7 > 2036-2-07 > 2036-2-7 > February 7, 2036 > $Mdocdate: 2036-02-07 06:28:16Z $ Yes, that is correct behaviour. > My version produces the same output for all cases: > > % while read d ; do echo ".Dd $d" | ./mandoc | tail -1 ; done February 7, 2036 That is wrong. I admit that validity rules for .Dd and .TH dates are a mess, for historical reasons. They allow much more than they should. So i would gladly make them stricter, but that needs care to not break too many existing documents. I should probably look into that anyway, time seems about ripe. But adding to the mess by allowing yet more different formats looks like the opposite of the direction we should go. > --- mandoc.c.orig > +++ mandoc.c > @@ -468,13 +468,15 @@ > char *pp; > > memset(&tm, 0, sizeof(struct tm)); > + if (strncmp(p, "$" "Mdocdate: ", 11) == 0) > + p += 11; Wrong, see above, syntax differs. > pp = NULL; > #if HAVE_STRPTIME > pp = strptime(p, fmt, &tm); > #endif > - if (NULL != pp && '\0' == *pp) { > - *t = mktime(&tm); > + if (NULL != pp) { > + *t = ('Z' == *p) ? timegm(&tm) : mktime(&tm); > return(1); > } Wrong, "Z" is not allowed here. > char * > mandoc_normdate(struct mparse *parse, char *in, int ln, int pos) > { > - char *out; > time_t t; > > - if (NULL == in || '\0' == *in || > - 0 == strcmp(in, "$" "Mdocdate$")) { > + if (NULL == in || '\0' == *in || 0 == strcmp(in, "$" "Mdocdate$")) { I see no change here? > mandoc_msg(MANDOCERR_DATE_MISSING, parse, ln, pos, NULL); > - time(&t); > + return(time2a(time(NULL))); > } Yes, that seems simpler. > - else if (a2time(&t, "%Y-%m-%d", in)) > - t = 0; > - else if (!a2time(&t, "$" "Mdocdate: %b %d %Y $", in) && > - !a2time(&t, "%b %d, %Y", in)) { > + else if (a2time(&t, "%Y-%m-%d %H:%M:%S", in) || Wrong, see above. > + a2time(&t, "%Y-%m-%d", in) || a2time(&t, "%b %d, %Y", in) || > + a2time(&t, "%b %d %Y", in)) { > + return(time2a(t)); Wrong in several ways. Including two invalid formats and converting the legacy man(7) format to the mdoc(7) format. > + } > + else { > mandoc_msg(MANDOCERR_DATE_BAD, parse, ln, pos, in); > - t = 0; > + return(mandoc_strdup(in)); Yes, that seems simpler. > } > - out = t ? time2a(t) : NULL; > - return(out ? out : mandoc_strdup(in)); > } > > int > --- mdoc.7.orig > +++ mdoc.7 > @@ -1225,6 +1225,12 @@ > the special string > .Dq $\&Mdocdate$ > can be given as an argument. > +The same effect can be achieved with > +.Xr svn 1 > +by setting the > +.Va svn:keywords > +property to > +.Li Mdocdate=%d . > .It > The traditional, purely numeric > .Xr man 7 > @@ -1240,7 +1246,10 @@ > Examples: > .Dl \&.Dd $\&Mdocdate$ > .Dl \&.Dd $\&Mdocdate: July 21 2007$ > +.Dl \&.Dd $\&Mdocdate: 2007-07-21 12:34:56Z foo $ > .Dl \&.Dd July 21, 2007 > +.Dl \&.Dd July 21 2007 No, the latter is plain wrong. > +.Dl \&.Dd 2007-07-21 That kind of works with mandoc, which is probably a bug. It doesn't work with groff, so it certainly mustn't be advertised in the manual. I'll test the useful simplifications and probably commit them. Thanks! Ingo -- To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv