Re: suggested feature: "date --debug" - print date parsing diagnostics

[Pádraig Brady]

On 10/08/16 05:07, Assaf Gordon wrote:
> Hello Paul, Pádraig,
> 
> Thank you for the review and suggestions.
> Attached is a suggested draft patch to address some of the issues:
> 
>> On Aug 9, 2016, at 12:32, Paul Eggert <address@hidden> wrote:
>>
>> These days, there's little reason to use macros instead of static functions. 
>> So I suggest renaming DEBUG0 to debug0 and making it a static function, and 
>> similarly for the other macros.
> 
> These macros where just thin wrappers adding an "if (debug)" before calling 
> the actual debug functions.
> In the attached patches, I removed the macros completely (each of the first 5 
> changesets removes one macro, I hope they are self-explanatory).
> 
>> parse-datetime should really be made reentrant, on systems that support 
>> that, by using the time_rz module. This is a larger task, though, and can be 
>> deferred.
> 
> The 6th changeset replaces the global variable (parse_datetime_debug) with a 
> member variable in the parser_control struct.
> It adds an additional function 'parse_datetime2' (temporary name) which 
> accepts 'bool debug' as a parameter.
> By eliminating the global variable, this 'debug' feature will not make things 
> harder when the time comes to make the parser reentrant.
> 
>> debug_stfdatetime can use nstrftime (in the strftime module) instead of 
>> doing things by hand.  nstrftime deals with time zones and nanoseconds and 
>> should be well-behaved with unusual time stamps.
> 
> The 7th changeset uses 'nstrftime', but not yet for the timezone, since 
> timezone in parse-datetime.y is stored as a long, not a 'timezone_t' .
> 
>> Come to think of it, should we even be translating the debugging output at 
>> all? It's unlikely to be useful to anybody lacking access to the source 
>> code, which is in English. Plus, some of the strings are not being 
>> translated anyway, for alignment reasons.
> 
> Regarding translation:
> I hope that this 'date --debug' feature will be used similarly to 'sort 
> --debug' - that is, by users who are somewhat confused by the output of 
> 'date'. Thus, it should be self-explanatory,and should not require looking at 
> the source code.
> By translating the messages users in other languages might be able to 
> diagnose problems themselves.
> (Sort's debug message are also translatable).
> 
> If we decide to remove translation, I can send additional patch for it.
> 
> Comments very welcomed.
> (Once this is decided upon, I'll send the corresponding 'date' patch to 
> coreutils).
> 
> regards,
>  - assaf
> 
> Assaf Gordon (7):
>   parse-datetime.y: remove DEBUG0
>   parse-datetime.y: remove DEBUG, PROGRESS macros
>   parse-datetime.y: remove DEBUG_PRINT_CURRENT_TIME macro
>   parse-datetime.y: remove DEBUG_PRINT_RELATIVE_TIME macro
>   parse-datetime.y: remove DEBUG_MKTIME_NOT_OK macro
>   parse-datetime: refactor global debug variable to a local parameter
>   parse-datetime.y: use nstrftime module (instead of snprintf)
> 
>  lib/parse-datetime.h   |   6 +-
>  lib/parse-datetime.y   | 332 
> ++++++++++++++++++++++++++-----------------------
>  modules/parse-datetime |   1 +
>  3 files changed, 178 insertions(+), 161 deletions(-)

Those look good to squash and apply,
modulo s/setftime/strftime/ in modules/parse-datetime

thanks,
Pádraig