[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Assaf Gordon
Subject: Re: suggested feature: "date --debug" - print date parsing diagnostics
Date: Wed, 10 Aug 2016 00:07:15 -0400

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 

 - 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(-)

Attachment: parse-datetime-debug-2016-08-09.patch.xz
Description: Binary data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]