[liblouis-liblouisxml] Re: Including the logging branch in master
- From: Bert Frees <bertfrees@xxxxxxxxx>
- To: liblouis-liblouisxml@xxxxxxxxxxxxx
- Date: Wed, 14 May 2014 12:49:54 +0200
Hi Michael, answers inline:
Michael Whapples writes:
> Hello,
> I agree generally with what you say.
>
> As I think I may have said in other messages, there may be other logging
> related functions which are less clear whether to remove or not from the
> API (eg. lou_logFile, lou_logEnd). I possibly would say to keep them as
> they deal with configuring the default handler/callback.
I agree.
> Mentioning about LibLouisUTDML, yes what you say makes sense, but it
> could be a bit of work, unfortunately due to the historic decision to do
> it as it is.
>
> I get the feeling to have similar code in LibLouisUTDML, may almost end
> up being code duplication (LibLouisUTDML will need code to register a
> callback, to direct log calls to any registered callback, deciding
> whether to call the callback depending on log level, etc), may be the
> logging stuff should be a separate module which could be shared amongst
> liblouis and liblouisutdml?
I don't feel very comfortable with that idea. A bit of code duplication
is o.k., liblouis and liblouisutdml are separate libraries after all.
Making a shared library just for logging maybe goes a bit too far. It
doesn't seem /that/ much code. But let's keep that option open. Is it ok
if we continue this discussion on friday?
In the meantime I'll try to add some tests and update the documentation.
Does anybody feel prompted to look add the Python bindings? It would be
nice if there was a new binding for at least lou_registerLogCallback.
Bert
> So under that design, on Windows:
> logging.dll (the logging code)
> liblouis.dll depends on logging.dll (liblouis mainly calls log(level,
> message, ...))
> liblouisutdml.dll depends on liblouis.dll and logging.dll (liblouisutdml
> calls log(level, message, ...))
>
> Then BrailleBlaster (as an example of an application) would depend
> directly on logging.dll and use the functions for registering and
> setting log levels.
>
> Would that work? Does that make sense to be more efficient?
>
> Michael Whapples
> On 14/05/2014 10:28, Bert Frees wrote:
>> Hi Michael,
>>
>> I talked to Christian and we have a feeling that maybe not only
>> lou_logPrint but also lou_log should be removed from the public API. The
>> reasoning is that liblouis shouldn't be responsible for routing
>> liblouisutdml's (or others') log messages to the right place. Shouldn't
>> it be the other way around? Shouldn't the programs that use liblouis be
>> responsible for redirecting liblouis's messages?
>>
>> In practice, this would mean that liblouisutdml would get it's own log
>> callback hook, and when e.g. BrailleBlaster would register a callback
>> with liblouisutdml, liblouisutdml would in his turn register that
>> callback with liblouis.
>>
>> This seems much cleaner to me, but I'd like to know what you think. Can
>> we discuss this on friday on the channel when both Christian and me have
>> some more time (and have put some more thought into this)?
>>
>> Other than that, the features seem pretty complete to me. I'd like to
>> have the documentation updated accordingly but if you want I can do
>> that.
>>
>>
>> Cheers,
>> Bert
>>
>>
>> Bert Frees writes:
>>
>>> Hello Michael,
>>>
>>> Michael Whapples writes:
>>>
>>>> Hello,
>>>> I am feeling that in the way of features, unless anyone can think of
>>>> anything else then may be the logging branch is near complete and ready
>>>> for comments and final adjustments before inclusion in to master.
>>>>
>>>> Things of concern to me:
>>>> * I know that in the log method there is some ugly code to force MSVC
>>>> into loading floating point support. Unfortunately this seems to be
>>>> unavoidable, the floating point support is dynamically loaded when
>>>> needed, but due to the function taking varargs it is not possible for VC
>>>> to work this out automatically. The ugly code seems to be one of the
>>>> suggested solutions by Microsoft. Unless anyone can think of another
>>>> solution I say keep this code, may be more comments to explain why it is
>>>> as it is.
>>>> * There was a question over the code to log widechar strings, whether it
>>>> duplicates existing code. Unfortunately I cannot remember the file the
>>>> duplicate code is in, but it was certainly within tests and the
>>>> logWidecharBuf function is for library runtime not just test time, thus
>>>> the solution is not altering logging code but rather moving the test
>>>> code to use core code.
>>>> * Do we want to remove old log functions from the API? This would break
>>>> old applications. Also what functions should be removed (eg.
>>>> lou_logPrint I am sure should go, but the lou_logFile function might be
>>>> useful for configuring the default callback handler).
>>> If I understand correctly, these functions still work exactly as before,
>>> the only difference is that they are not used internally anymore (except
>>> lou_logPrint which is used as the default log handler). IMO that is the
>>> most important, and from now on we should only use the new logging
>>> functions internally.
>>>
>>> I'd rather wait with changing the API, because I think there are some
>>> other API changes in the pipeline, so we should combine all of them in
>>> one major version update.
>>>
>>>> Any thoughts on the above, or any other thoughts of what might need
>>>> changing?
>>> Maybe we should already remove the old log functions from the
>>> documentation though.
>>>
>>>> When might we look at merging this to master? How does one deal with
>>>> that with Git?
>>> Git is all about merging :) To merge a branch back onto master, you
>>> simply do:
>>>
>>> git checkout master
>>> git merge your_branch_name
>>>
>>> But you don't have to worry too much about merging stuff to
>>> master. Mesar, Christian or I can take care of that.
>>>
>>>
>>> /Bert
>> For a description of the software, to download it and links to
>> project pages go to http://www.abilitiessoft.com
>
> For a description of the software, to download it and links to
> project pages go to http://www.abilitiessoft.com
For a description of the software, to download it and links to
project pages go to http://www.abilitiessoft.com
Other related posts:
