Bug 91055 - cleanup weirdo lcl_createWindowInfo method signature ...
Summary: cleanup weirdo lcl_createWindowInfo method signature ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
4.4.3.2 release
Hardware: Other All
: medium normal
Assignee: Daniel L Robertson
URL:
Whiteboard: target:5.1.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2015-05-03 18:13 UTC by Michael Meeks
Modified: 2016-10-25 19:20 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2015-05-03 18:13:00 UTC 
vcl/source/window/window.cxx has a method:

    OString lcl_createWindowInfo(const vcl::Window& i_rWindow)

which takes a reference that is immediately converted into a pointer to operate on internally; and all callers pass a de-referenced pointer into it which is ugly & lame =) it needs changing to:

    OString lcl_createWindowInfo(const vcl::Window *pWindow)

And the callees simplifying =)

Thanks !
Comment 1 Daniel L Robertson 2015-06-27 04:50:32 UTC 
Thanks Michael! I just submitted a patch to gerrit. It was my first patch I've submitted to gerrit, so any constructive criticism is more than welcome. https://gerrit.libreoffice.org/#/c/16537/
Comment 2 Commit Notification 2015-06-29 07:28:57 UTC 
danlrobertson committed a patch related to this issue.
It has been pushed to "master":

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

tdf#91055 Pass by Address - lcl_createWindowInfo

It will be available in 5.1.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 3 Michael Meeks 2015-06-29 09:04:51 UTC 
Noel beat me to it =) Thanks for the cleanup Daniel ! great to have your code included in LibreOffice ... what's next ?
Comment 4 Daniel L Robertson 2015-06-29 23:25:41 UTC 
Thanks Michael! I saw that the o3tl has at least one instance of the now depreciated `unary_function`, so I was going to look into modifying the code that uses it. Again, any advice or pointers are welcome.

I set the status of this bug to resolved... I assume that's what I'm supposed to do.
Comment 5 Michael Meeks 2015-06-30 09:00:40 UTC 
Hey; yes quite right to close :-) and cleaning up o3tl sounds great. Ultimately as you read the code - you should find enough inelegance / mess to spend a lifetime cleaning =) but of course, really this is all a spin-up for hacking on fun features instead =)

Ooh - which reminds me - if you're good with templates, mail me - we have a rather debilitating performance problem affecting Android & LibreOffice on-line around bitmap scaling which should be easy to fix =)
Comment 6 Robinson Tryon (qubit) 2015-12-16 00:11:38 UTC Comment hidden (obsolete)