Re: [PATCH v1 1/4] lib/vsprintf: Allow to override date and time separator

[Andy Shevchenko]

On Tue, May 11, 2021 at 04:14:56PM +0200, Petr Mladek wrote:
> On Mon 2021-05-10 18:04:10, Andy Shevchenko wrote:
> > ISO 8601 defines 'T' as a separator between date and time. Though,
> > some ABIs use time and date with ' ' separator instead.
> > 
> > Add a flavour to the %pt specifier to override default separator.

...

> >  	%pt[RT]			YYYY-mm-ddTHH:MM:SS
> > +	%pt[RT]s		YYYY-mm-dd HH:MM:SS
> >  	%pt[RT]d		YYYY-mm-dd
> >  	%pt[RT]t		HH:MM:SS
> > -	%pt[RT][dt][r]
> > +	%pt[RT][dt][rs]
> 
> Sigh, we do not have clear rules what [xy] means. It might be:
> 
>       + always use one of them
>       + optionally use one of them
>       + always use any of them
>       + optionally use any of them
> 
> %pt[RT][dt][rs] is a great mix:
> 
>       + R or T is required, the rest is optional
>       + 'd' or 't' can be used but both together are not supported 
>       + any variant of 'r' and 's' is supported including various ordering
> 
> Honestly, I do not know about any magic solution that might make it
> easier to understand these monster modifiers.
> 
> Well, what about using the following at least in this case:
> 
> 	%pt[RT][dt][r][s]
> 
> It might help to understand that both 'r' and 's' can be used at the
> same time.

This is the case, yes, thanks for catching it.

> An attempt to distinguishing all the possibilities might be:
> 
> 	%pt{RT}[{dt}][r][s]
> 
> where [] means that it is optional and {} means one of them must be
> chosen. But I am not sure if it really makes the life easier. Anyway,
> this would be for another patch that updates the entire printk-formats.rst.

No, this is not the case, the d and t can go in any combinations: none, d, t,
dt, or td.

> 
> >  For printing date and time as represented by::
> >  
> > @@ -528,6 +529,9 @@ in human readable format.
> >  By default year will be incremented by 1900 and month by 1.
> >  Use %pt[RT]r (raw) to suppress this behaviour.
> >  
> > +The %pt[RT]s (space) will override ISO 8601 by using ' ' instead of 'T'
> > +between date and time. It won't have any effect when date or time is omitted.

...

> > +	do {
> > +		switch (fmt[count++]) {
> > +		case 'r':
> > +			raw = true;
> > +			break;
> > +		case 's':
> > +			space = true;
> > +			break;
> > +		default:
> > +			found = false;
> > +			break;
> > +		}
> > +	} while (found);
> >  
> >  	if (have_d)
> >  		buf = date_str(buf, end, tm, raw);
> >  	if (have_d && have_t) {
> >  		/* Respect ISO 8601 */
> 
> The comment is slightly misleding now. What about something like?
> 
> 		/* 'T' by ISO 8601. */
> 
> Or maybe call the variable: iso_8601, remove the comment, and
> invert the logic:

Okay, I will think how to improve, thanks!

> 		bool iso_8601 = true;
> 
> 		case 's':
> 			iso_8601 = false;
> 			break;
> 
> 		*buf = iso_8601 ? 'T' : ' ';
> 
> >  		if (buf < end)
> > -			*buf = 'T';
> > +			*buf = space ? ' ' : 'T';
> >  		buf++;
> >  	}

-- 
With Best Regards,
Andy Shevchenko