+1
On Aug 8, 2013, at 6:40 AM, Igor Galić <[email protected]> wrote:
>
>
> ----- Original Message -----
>>
>>
>> Hi folks,
>>
>> right now many of plugins send error messages without prefixing the plugin
>> they come from, this bad practice is also continued in our examples.
>>
>> many error messages also close with `\n`, although this is not necessary.
>>
>> We should fix this by "normalizing" the way we send those error messages.
>>
>> My proposal is either:
>>
>> TSError("%s: cannot parse file %s", PLUGIN_NAME, filename);
>>
>> xor:
>>
>> TSError("[%s] cannot parse file %s", PLUGIN_NAME, filename);
>>
>> I have no strong leanings towards either. BUT: We should have one consistent
>> format between examples and plugins, as well as newly imported code.
>>
>> The word "error" should not be repeated, as it is already in the "severity
>> (`TSError()`). There should be *no* `\n` at the end of the message.
>>
>> Normally I would just grep the source and see which use-case is the
>> prevailing one, but I'm too tired and I'm flying to Ireland tomorrow,
>> so I'm leaving this as "an exercise to the student" ;)
>>
>> reference issue: https://issues.apache.org/jira/browse/TS-2106
>
>
>
> I've been looking at this for the past two days and I must say, so far
> the combination I like best are the macros in the gzip plugin:
>
>
>
> #define debug(fmt, args...) do { \
> TSDebug(PLUGIN_NAME, "DEBUG: [%s:%d] [%s] " fmt, __FILE__, __LINE__,
> __FUNCTION__ , ##args ); \
> } while (0)
>
> #define info(fmt, args...) do { \
> TSDebug(PLUGIN_NAME, "INFO: " fmt, ##args ); \
> } while (0)
>
> #define warning(fmt, args...) do { \
> TSDebug(PLUGIN_NAME, "WARNING: " fmt, ##args ); \
> } while (0)
>
> #define error(fmt, args...) do { \
> TSError("[%s], [%s:%d] [%s] ERROR: " fmt, PLUGIN_NAME, __FILE__, __LINE__,
> __FUNCTION__ , ##args ); \
> TSDebug(PLUGIN_NAME, "[%s:%d] [%s] ERROR: " fmt, __FILE__, __LINE__,
> __FUNCTION__ , ##args ); \
> } while (0)
>
> #define fatal(fmt, args...) do { \
> error(fmt, ##args ); \
> exit(-1); \
> } while (0)
>
>
> What'd you think we make this a default API?
>
> perhaps under the name of TSPlugin$Severity(...) ?
> This of course assumes that there *is* define (or constant) PLUGIN_NAME,
> as is our practice.. is this a good idea?
>
> --i
