Re: Review Request: A new runtime debugging output system for KOffice, ported from Amarok.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5767/#review8507
-----------------------------------------------------------


I believe this should go to kdebug.h instead.


trunk/koffice/libs/main/Debug.h
<http://svn.reviewboard.kde.org/r/5767/#comment8864>

    Already done by Q_FUNC_INFO



trunk/koffice/libs/main/Debug.h
<http://svn.reviewboard.kde.org/r/5767/#comment8865>

    Why KWord? seems wrong here.



trunk/koffice/libs/main/Debug.h
<http://svn.reviewboard.kde.org/r/5767/#comment8869>

    says AMAROK ;)
    
    I'll add this to kdebug.h... as KWARNING_NOTIMPLEMENTED?



trunk/koffice/libs/main/Debug.h
<http://svn.reviewboard.kde.org/r/5767/#comment8866>

    I'll put this into kdebug.h instead.



trunk/koffice/libs/main/Debug.h
<http://svn.reviewboard.kde.org/r/5767/#comment8867>

    Nice trick, but wouldn't documenting
    kDebug() << (QVariantList() << anInt << ...); be enough? The typedef already exists, just with another name.



trunk/koffice/libs/main/Debug.cpp
<http://svn.reviewboard.kde.org/r/5767/#comment8868>

    Ouch. Well, this hack is not needed in kofficecore or in libkdecore, K_GLOBAL_STATIC is enough - or in kdebug, the existing singleton.


- David


On 2010-11-04 16:45:23, Mark Kretschmann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5767/
> -----------------------------------------------------------
> 
> (Updated 2010-11-04 16:45:23)
> 
> 
> Review request for KOffice.
> 
> 
> Summary
> -------
> 
> This patch introduces the advanced runtime debugging system of Amarok to KOffice. It is meant to augment or replace the standard calls for debugging output, like qDebug() and kDebug(). While kDebug() certainly fulfills its purpose, it is really not much more than a printf() with some syntactic sugar on top. The new debugging system is far more powerful, much like a kDebug() on steroids.
> 
> 
> Features of the new system:
> 
> * Can be used just as easily as "kDebug()". The call can be replaced with "debug()", and the rest stays the same.
> * Features automatic visual indenting of nested code blocks in the output. This is very useful for quickly getting an overview of the nesting of methods. To use this, all that is needed is putting the macro "DEBUG_BLOCK" in a method.
> * Prints timing information for every method, and as a special bonus it checks if a method takes extremely long (the value is adjustable, default is 5 seconds), and then prints "DELAY". This makes it very easy to grep for methods that seem to block the application.
> * Does some coloring of the output, which helps to spot errors easily. The coloring is optional, which is important for color blind people, or those who simply dislike it.
> * The debugging system is meant to be enabled with a runtime switch, e.g. "koffice --debug", as opposed to compile time. The big advantage of this: Even users without a debug build are able to provide useful information, in case that they see a malfunction in the application. This has helped us numerous times in Amarok.
> * Sprinkling the code with "DEBUG_BLOCK" macros is easy to do, and also easy to remove. This macro is probably the most useful feature of the system, as it often helps with debugging hard-to-spot crashes, e.g. with a trashed stack. The attached screenshots should give you an impression of what it does.
> 
> 
> Pitfalls: 
> 
> One should not overuse the DEBUG_BLOCK feature. While it is very useful, using it too much leads to "debug spam". Ideally one should only put it in important methods like constructors and destructors.
> 
> 
> Known issues:
> 
> * The coding style is still that of Amarok. I would of course change the style to KOffice's later on.
> * The placing of the Debug class in the code tree is not ideal. I've put it in libs/main/ for now, but Boud noted that this is not the best location. I could move it to a better places easily.
> * Features that should be optional (like coloring) are currently hardcoded. This would of course be fixed, if the patch is welcome.
> * So far I have only used DEBUG_BLOCK in a few places in KOffice, for demonstration purposes. This could be extended later on.
> 
> 
> Notes on usage:
> 
> Please refer to the docs (comments) in the code. They explain the available features rather well.
> 
> 
> Diffs
> -----
> 
>   trunk/koffice/libs/main/Debug.h PRE-CREATION 
>   trunk/koffice/libs/main/Debug.cpp PRE-CREATION 
>   trunk/koffice/libs/main/Debug_p.h PRE-CREATION 
>   trunk/koffice/libs/main/KoApplication.cpp 1192436 
>   trunk/koffice/libs/main/KoApplicationAdaptor.cpp 1192436 
>   trunk/koffice/libs/main/KoDockerManager.cpp 1192436 
>   trunk/koffice/libs/main/KoDocument.cpp 1192436 
>   trunk/koffice/libs/main/KoGlobal.cpp 1192436 
>   trunk/koffice/libs/main/KoMainWindow.cpp 1192436 
> 
> Diff: http://svn.reviewboard.kde.org/r/5767/diff
> 
> 
> Testing
> -------
> 
> Started KWord, compared old debugging output to new output. Liked what I saw.
> 
> 
> Screenshots
> -----------
> 
> New Debug system in KWord
>   http://svn.reviewboard.kde.org/r/5767/s/547/
> Debug system as used in Amarok
>   http://svn.reviewboard.kde.org/r/5767/s/548/
> 
> 
> Thanks,
> 
> Mark
> 
>

_______________________________________________
koffice-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/koffice-devel