View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0012650 | Scribus | General | public | 2014-09-02 20:06 | 2015-12-01 23:01 |
| Reporter | william | Assigned To | |||
| Priority | normal | Severity | minor | Reproducibility | N/A |
| Status | new | Resolution | open | ||
| Platform | x86_64 | OS | Linux | OS Version | Fedora 20 |
| Summary | 0012650: [patch] Make debug code in pageitem_textframe.cpp easier to enable. | ||||
| Description | When I looked at layout differences between Scribus 1.4 and 1.5 in http://bugs.scribus.net/view.php?id=12636 , I saw a lot of qDebug() debug code in pageitem_textframe.cpp that was commented out. These patches switch those qDebug() calls to scDebug() so that the debug code can be enabled by setting SC_DEBUG_FILE inside pageitem_textframe.cpp or DEBUG_PAGEITEMTEXTFRAME during the compile instead of adding or removing a lot of comments. The patches also fix some of the commented debug code that no longer compiled. I also extended scdebug.h. * Modules that include scdebug.h can now set the macro SC_DEBUG_MODULE to make unique names for the classes that it defines. Without this change, if some modules enable scdebug and other modules disable it, scribus will crash. * Modules that include scdebug.h can now set the macro SC_DEBUG_LOG_TIME to 0 to suppress adding a time stamp. Suppressing the time stamp makes it easier to compare logs from different runs. If a small change to an SLA or to pageitem_textframe causes unexpected differences in the layout, this patch makes it fast to enable debugging and compare logs from before and after the change to identify the point where the layout goes wrong. These patches should not affect scribus functionality. | ||||
| Tags | No tags attached. | ||||
| Patch | Yes | ||||
|
|
|
|
|
Moving out of Contributor Builds. William, please post patches to main Scribus queue and not Contributor Build. Core devs will see it better. |
|
|
How do I post to the main Scribus queue? Should I have entered a different value in one of the fields? |
|
|
Project - Scribus |
|
|
Making this 'Sticky' so it doesn't suffer "Patch Rot" |
|
|
Hmmm, unconditional debug code in text layout such as below is a no go for me: scDebug() << QString("dropcaps 1pre: chsd=%1 realCharHeight = %2 chstr=%3").arg(chsd).arg(asce).arg(chstr[0]); The arg() function will be executed unconditionally here. |
|
|
Unsticking: as is, this patch cannot be applied. |
|
|
Would it be acceptable if I put all of the debug inside "#ifdef DEBUG_PAGEITEMTEXTFRAME"? |
|
|
william, if you put a .diff extension to your patches (or maybe even .patch) they can be read directly in the bug tracker... |
|
|
on the one side, i agree that it looks like a good idea to be able to enable / disable a set of debug instances with a single switch... ... but my experience shows that as soon as you start using this regularly, i get too many error messages, most of them set by other developers... while i admit that i also like qDebug(), i would say that using the debugger (in qt creator or through gdb) is probably a better choice for tracking the behavior of the code... keep up with the stream of patches! ciao a.l.e |
|
|
verdict? |
|
|
pageitem_textframe.cpp is one of the few Scribus modules with a lot of debug code, and I think that it is easier to enable and disable debug code if you can set a single #if at the top than if you have to add and remove comments in a lot of places. I started looking there because I had documents made by Scribus 1.4 that formatted differently in Scribus 1.5 but apparently it was bugs in Scribus 1.4. It does not matter to me whether these patches are applied or not, but if there are changes that would make the patches acceptable, I can do that. Also, if you want, I can separate the patches to scdebug.h that allow it to be used conditionally with multiple modules. Those patches might be useful on their own separately from the patches to pageitem_textframe.cpp. |
|
|
bump |
|
|
would be good to make a decision on this before patch becomes too outdated |
|
|
Craig & Jean, would this help, for example to not have to compile scribus as much since the debug output would be helpful enough to understand runtime issues ? Anytime we can save an opportunity to build scribus is a good timesaver, right? or am I way off? |
|
|
being able to output debug information that is relevant to the user to better understand bugs is a good idea. but i would avoid to do so by default. do we need a command line switch for it? ad noted above, for programmers, using the real debugger is probably a better choice than using "qDebug() <<" to output to the terminal... i know, it's sometime faster to use the standard output, but i would not implement a way to enable / disable the qDebugs() for such a use case. |
|
|
I've worked with #ifdef DEBUG_MODE_XXX and it prooved very efficient. Another powerfull debug tool is extensive use of user defined asserts including high level asserts. Both of these tools dont require masses of work to be efficient : the changes can be done localy in the code where-when needed, step by step. |
|
|
asserts would be very welcome! |
|
|
http://doc.qt.io/qt-5/qloggingcategory.html <--possibly another way to achieve this ? |
|
|
The code already uses QDebug. Scribus has an scDebug layer over QDebug that allows the generation of debug code to be controlled by a #define at compile time so that production code doesn't have to go to the expense of preparing the lines to print when debugging is not enabled. I think that people on the CTL project have been working on pageitem_textframe.cpp, so the part of the patches that adds debug code to it is probably no longer relevant. The other part of the patches that enhances scDebug might still be useful. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-09-02 20:06 | william | New Issue | |
| 2014-09-02 20:06 | william | File Added: scribus15-pageitem-textframe-debug.pat | |
| 2014-09-23 12:57 | JLuc | Tag Attached: #layout | |
| 2014-10-07 22:13 | Kunda | Note Added: 0033921 | |
| 2014-10-07 22:13 | Kunda | Project | Contributor Builds => Scribus |
| 2014-10-08 00:44 | william | Note Added: 0033927 | |
| 2014-10-08 06:07 | cbradney | Note Added: 0033929 | |
| 2014-10-08 19:57 | JLuc | File Added: project - scribus.jpg | |
| 2014-10-13 15:53 | Kunda | Tag Detached: #layout | |
| 2014-10-15 14:12 | Kunda | Sticky Issue | No => Yes |
| 2014-10-15 14:12 | Kunda | Note Added: 0034014 | |
| 2014-10-15 19:13 | jghali | Note Added: 0034019 | |
| 2014-10-15 19:17 | jghali | Note Edited: 0034019 | |
| 2014-10-15 19:19 | jghali | Sticky Issue | Yes => No |
| 2014-10-15 19:19 | jghali | Note Added: 0034020 | |
| 2014-10-15 20:21 | william | Note Added: 0034021 | |
| 2014-10-16 12:07 | ale | Note Added: 0034024 | |
| 2014-10-16 12:15 | ale | Note Added: 0034025 | |
| 2014-10-21 23:46 | Kunda | Note Added: 0034113 | |
| 2014-10-22 00:15 | william | Note Added: 0034115 | |
| 2014-10-24 22:57 | Kunda | Patch | => Yes |
| 2014-10-29 01:37 | Kunda | Sticky Issue | No => Yes |
| 2014-11-04 17:08 | Kunda | Note Added: 0034188 | |
| 2014-11-04 17:09 | Kunda | File Deleted: project - scribus.jpg | |
| 2015-01-10 17:37 | Kunda | Note Added: 0034362 | |
| 2015-10-07 20:57 | Kunda | Note Added: 0036541 | |
| 2015-10-12 09:04 | ale | Note Added: 0036648 | |
| 2015-10-12 15:25 | JLuc | Note Added: 0036653 | |
| 2015-11-02 18:47 | aliyousuf | Tag Attached: HOST-Oman | |
| 2015-11-03 10:06 |
|
Tag Detached: HOST-Oman | |
| 2015-11-03 10:57 | ale | Note Added: 0037174 | |
| 2015-11-05 12:18 | Kunda | Sticky Issue | Yes => No |
| 2015-12-01 22:32 | Kunda | Note Added: 0037779 | |
| 2015-12-01 23:01 | william | Note Added: 0037781 |
