Re: [pygccxml-development] improving user experience

[Matthias B. <ba...@ir...>]

Roman Yakovenko wrote:
>> - Even though there are several logging streams now I think warning
>> message should still be issued using warning() instead of info() (so
>> that I can set up a file handler at the root logger that only writes
>> true warnings). For example, in calldef_t._exportable_impl() the message
>> about the max_arity thing is still written using info(). Roman, do you
>> mind if I change that to warning()?
> 
> Please do it.

Done.

>> - Most warnings are still "hidden" from the user until he explicitly
>> obtains them using the readme() method. I did a test and modified the
>> code in decl_wrapper_t.get_exportable() so that any time
>> self._exportable_reason is set to a non-empty string this string is also
>> written to the logger.
>> [...]
>> A lot of the above new warnings are about compiler generated
>> constructors (it always seems to be the copy constructor in my case). Is
>> this really worth a message at all? I certainly don't expect pyplusplus
>> to generate wrappers for things that are not there, so I think
>> pyplusplus could just silently ignore those artifical declarations that
>> are only created by gccxml (I don't really know why they are generated
>> at all?).
> 
> I thought, I disabled them. I will check one more time. Can you check, 
> whether you are running with latest version?

Yes, I have updated earlier today. I did see those messages because of 
my own modification that prints every _exportable_reason that is not empty.
Now I've seen your __report_warning() method and realized that the 
messages actually should already have appeared in the logger. But 
because I only saw 5 messages whereas my code from above printed many 
more I thought the user has to call something like readme() himself.
The problem is in __report_warning() which I'd say has a bug. The method 
has the following lines right at the beginning:

     if reason in decl_wrapper_t.ALREADY_REPORTED_MSGS:
         return
     decl_wrapper_t.ALREADY_REPORTED_MSGS.add( reason )

But the string "reason" only contains the "raw" message and doesn't 
contain anything that is unique to the current declaration. This means 
if there is another declaration that cannot be exported for the same 
reason, then the warning message for the second declaration will be 
suppressed (even though this is a new message as it refers to a 
different declaration).
So instead of storing "reason" in ALREADY_REPORTED_MSGS this should 
rather be "msg" (which contains the declaration string). Or maybe it 
would be more efficient if only a reference to the declaration is stored 
instead of an entire string (or can a declaration have more than one 
"reason" messages?).

- Matthias -