Hi David, David Pirotte <da...@altosw.be> writes:
> Hi Maxim, > >> ... >> My only concern about doing this, rephrasing what I wrote on the chat, >> is that it'd be hard to validate the input value, as that validation >> would need to be specialized to handlers, e.g. for some class we'd >> want to disallow 'line as it wouldn't apply. > >> That's why I suggested keeping the flush on emit switch as an on/off >> boolean, which can live in the base class of all <log-handler>, and >> perhaps subclass <log-handler> into <stream-handler> which would >> accept a #:buffering-mode keyword whose accepted values would mirror >> what setvbuf allows (line, block or none). I think that'd simplify >> things at the implementation level and avoid user error or surprises. > >> Our current loggers could be derived from the new <stream-handler> >> class, while something like a <syslog-handler> would inherit directly >> from <log-handler>, avoiding to handle users providing 'line or 'block >> to #:buffering-mode, which would need to throw a user error. > >> Does that explain my point better (does it make sense?) If so, we can >> keep the patch here as-is, and the work to add a new <stream-handler> >> with a #:buffering-mode keyword would become future work. > > Yes, it explains your point in a much better way, thanks > Please go ahead and push the patch 'as is' ... Excellent, thanks for the heads-up. I've rebased my local branch on current devel and pushed this last commit (9c75b17). I've forgotten the patman metadata in the commit message (sorry), so you may want to reword it on your side before merging to master. I don't expect to be working on the adding the new <stream-handler> class and what was discussed as further work yet, so if you were aiming to produce a new release, the timing should be good now. -- Thanks, Maxim
