Bug 60739 - cut/paste coding redux
Summary: cut/paste coding redux
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.0.0.0.beta2
Hardware: Other All
: medium normal
Assignee: Michaël Lefèvre
URL:
Whiteboard: ToBeReviewed
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2013-02-12 16:28 UTC by Michael Meeks
Modified: 2017-02-14 08:57 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2013-02-12 16:28:27 UTC
It seems that large chunks of code are cut/pasted between a number of modules in writer:

source/ui/shells/annotsh.cxx:        case FN_WORDCOUNT_DIALOG:
source/ui/shells/annotsh.cxx:                pVFrame->ToggleChildWindow(FN_WORDCOUNT_DIALOG);
source/ui/shells/drawsh.cxx:        case FN_WORDCOUNT_DIALOG:
source/ui/shells/drawsh.cxx:                pVFrame->ToggleChildWindow(FN_WORDCOUNT_DIALOG);
source/ui/shells/drwtxtex.cxx:        case FN_WORDCOUNT_DIALOG:
source/ui/shells/drwtxtex.cxx:                pVFrame->ToggleChildWindow(FN_WORDCOUNT_DIALOG);
source/ui/shells/frmsh.cxx:        case FN_WORDCOUNT_DIALOG:
source/ui/shells/frmsh.cxx:                pVFrame->ToggleChildWindow(FN_WORDCOUNT_DIALOG);
source/ui/shells/textsh.cxx:    SFX_CHILDWINDOW_REGISTRATION(FN_WORDCOUNT_DIALOG);
source/ui/shells/textsh1.cxx:    case FN_WORDCOUNT_DIALOG:
source/ui/shells/textsh1.cxx:            pVFrame->ToggleChildWindow(FN_WORDCOUNT_DIALOG);

if would be excellent to find a common place for that code to go, and to factor it out into a single shared module.

At least - it seems so to me :-)
Comment 1 Michael Meeks 2013-02-12 16:29:14 UTC
Michael - it'd be great to have some idea of where that duplication should ultimately end-up :-) [ assuming it can end up somewhere ].
Comment 2 Michael Stahl (allotropia) 2013-02-13 10:21:52 UTC
Cedric, we need somebody who knows anything about those
mysterious "shells" to answer comment #2.
Comment 3 Cédric Bosdonnat 2013-02-13 13:25:52 UTC
(In reply to comment #2)
> Cedric, we need somebody who knows anything about those
> mysterious "shells" to answer comment #2.

Those shells are different UI parts. The only place where such a factorization could happen is the SwView as it seems to be handling already most of the job. We still have to Invalidate the shell, but that would simplify the code a bit.
Comment 4 Björn Michaelsen 2013-10-04 18:46:24 UTC
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility.

see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Comment 5 Robinson Tryon (qubit) 2013-10-23 16:49:18 UTC Comment hidden (obsolete)
Comment 6 Cédric Bosdonnat 2014-01-20 08:57:23 UTC Comment hidden (noise)
Comment 7 Lenka 2014-03-05 20:10:54 UTC
Hi people, I would like to solve this bug. Any advice for the common place for that code?
Comment 8 Michael Meeks 2014-03-05 20:32:24 UTC
Lenka - it'd be great to have you working on that =) what I'd suggest you do is to whack it in one of the source/ui pieces - and then some purist will come along and sort that out; the key piece is doing the careful de-duplication - analysing the problem & hacking out the bloat: then trying to work out if the special cases / differences are deliberate or accidental ;-)

Beyond that the naming of the new headers / classes is up to your creativity - it's easy to search/replace them in the patch usually as long as they are reasonably unique.

Thanks so much for looking at this !
Comment 9 Björn Michaelsen 2014-12-02 10:53:13 UTC
adding LibreOffice developer list as CC to unresolved Writer EasyHacks for better visibility.

see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Comment 10 Michaël Lefèvre 2015-01-26 13:32:43 UTC
Factorise FN_WORDCOUNT_DIALOG case in https://gerrit.libreoffice.org/14182 .

More cases could be cleaned up :
FN_FORMAT_FOOTNOTE_DLG
FN_NUMBERING_OUTLINE_DLG:
SID_OPEN_XML_FILTERSETTINGS
...

I'm planning to change the other ones, referencing this bug. Any objection ?
Comment 11 Commit Notification 2015-01-28 06:23:02 UTC
Michaël Lefèvre committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d57cd80479fac60a2486c74257a8840e36935e20

tdf#60739 code factorisation

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 12 Commit Notification 2015-01-28 06:24:30 UTC
Michaël Lefèvre committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0bac0582adac70380fe30ed16247b1b9f91d2680

tdf#60739 code factorisation

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2015-02-05 11:12:18 UTC
Michaël Lefèvre committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0473818b3ebd8446a40cffa6a83b985dc3d9d1a0

tdf#60739 code factorisation

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2015-02-05 11:13:33 UTC
Michaël Lefèvre committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=71c49bcfa0677f013684030defbf5ead21695d85

tdf#60739 code factorisation

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 15 Robinson Tryon (qubit) 2015-12-14 06:35:40 UTC Comment hidden (obsolete)
Comment 16 Robinson Tryon (qubit) 2016-02-18 14:52:10 UTC Comment hidden (obsolete)