[Libreoffice] [PATCH] Easy hacks: Remove misuse of SfxItemSet API from libs-core

[Bjoern Michaelsen]

Hi Theo,

On Mon, 28 Mar 2011 11:25:32 +0200
Theo van Klaveren
<theo.van.klaveren at gmail.com> wrote:

> I've started work on removing all calls to
> SfxItemSet::Put(SfxPoolItem &, int which) from the code. Attached the
> first two patches, which contains mostly easy cases: Almost all
> SfxPoolItems already contained the right WhichID, so the extra
> argument could just be removed. But there are some slightly more
> interesting cases in here as well.

Great to see you starting on this. In first patch the changes to
   editeng/source/editeng/editdoc.cxx and
   editeng/source/uno/unoipset.cxx
look good. The changes to
   ChangeFontSizeImpl in editeng/source/editeng/editview.cxx
however are assuming that aSet.Get( EE_CHAR_FONTHEIGHT_CJK ) returns a
item with a which-id of EE_CHAR_FONTHEIGHT_CJK, which unfortunately we
cannot assume currently (we need exactly the changes you are doing now
everywhere, so that we finally can make that assumption). You would
need to create a clone from the item you got and set the which-id
explicitly before putting it in the second set, or leave the code as is
for now.
For the changes to
   ConvertAndPutItems in editeng/source/editeng/editdoc2.cxx
it seems to me that there is a "delete pItem" called now when
eSourceUnit == eDestUnit which has not been there before.

In the second patch:
   svx/source/dialog/imapwnd.cxx
   svx/source/svdraw/svdedxv.cxx (*)
   svx/source/table/tablecontroller.cxx (*)
   svx/source/unodraw/UnoNameItemTable.cxx
are perfect.
In:
   svx/source/unodraw/unomtabl.cxx
I assume XLineEndItem and XLineStartItem to be constructed with the
right which-id already, in which case the SetWhich calls are
superfluous, but that would need to be validated.

> I have more patches ready to go, but I'd like to get these reviewed
> first to check I'm on the right path. The really hard cases are
> in other parts of the code :)

In general, they are looking good, keep them coming!

Best Regards,

Bjoern


(*) Actually, the original is totally wrong here, and likely never
worked as intended.
-- 
https://launchpad.net/~bjoern-michaelsen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20110328/a0cba1f5/attachment.pgp>