Re: Mdocdate

[Ingo Schwarze <schwarze@usta.de>]

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 <tests
>                                 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 <tests | uniq
>                                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