----- Original Message -----
> On Aug 8, 2013, at 6:40 AM, Igor Galić <i.ga...@brainsware.org> 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)
>
> These are not terrible, though I'd tend to do something like:
>
> #define debug(fmt, ...) do { \
> if (unlikely(TSIsDebugTagSet(PLUGIN_NAME))) { \
> TSDebug(PLUGIN_NAME, fmt, ##__VA_ARGS__); \
> } \
> } while(0)
>
> so that format string arguments are not evaluated unless we are going to
> print something.
>
> >
> >
> > What'd you think we make this a default API?
>
> I'm happy to make this change for in-tree plugins. I don't think this is
> ready for external API status. I don't much like the dependency on
> PLUGIN_NAME, and there are plugins (eg. spdy) that use the debug tag
> differently.
>
> I'm definitely in favour of having the in-tree plugins log consistently
> though.
I'm perfectly fine with delivering this non-API for our in-tree plugins then.
For compelteness: Can someone please enlighten me as to how
TSTextLogObjectWrite() fits into the this whole picture?
> For an external debug logging API, I'd like to see something more similar to
> the internal Debug API, with real suport for log levels and
> proxy.config.diags.show_location.
+1, go for it.
> J
>
>
--
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.ga...@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
