[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: suggested feature: "date --debug" - print date parsing diagnostics
From: |
Pádraig Brady |
Subject: |
Re: suggested feature: "date --debug" - print date parsing diagnostics |
Date: |
Wed, 10 Aug 2016 10:59:13 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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
- Fwd: suggested feature: "date --debug" - print date parsing diagnostics, Pádraig Brady, 2016/08/09
- Re: Fwd: suggested feature: "date --debug" - print date parsing diagnostics, Paul Eggert, 2016/08/09
- Re: suggested feature: "date --debug" - print date parsing diagnostics, Assaf Gordon, 2016/08/10
- Re: suggested feature: "date --debug" - print date parsing diagnostics,
Pádraig Brady <=
- Re: suggested feature: "date --debug" - print date parsing diagnostics, Assaf Gordon, 2016/08/11
- Re: suggested feature: "date --debug" - print date parsing diagnostics, Pádraig Brady, 2016/08/12
- Re: suggested feature: "date --debug" - print date parsing diagnostics, Assaf Gordon, 2016/08/14
- Re: suggested feature: "date --debug" - print date parsing diagnostics, Pádraig Brady, 2016/08/19
- Re: suggested feature: "date --debug" - print date parsing diagnostics, Assaf Gordon, 2016/08/20
- Re: suggested feature: "date --debug" - print date parsing diagnostics, Jim Meyering, 2016/08/20