From koffice-devel Thu Sep 23 15:22:36 2010 From: "Marijn Kruisselbrink" Date: Thu, 23 Sep 2010 15:22:36 +0000 To: koffice-devel Subject: Re: Review Request: Make the attribute() and property() methods in Message-Id: <20100923152236.11613.29300 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=koffice-devel&m=128525539921486 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============0543729649==" --===============0543729649== Content-Type: multipart/alternative; boundary="===============3012251955945182331==" --===============3012251955945182331== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On 2010-09-23 15:16:57, Thomas Zander wrote: > > Making the methods non-private makes me think we should no longer have = those methods be inlined as that would mean the implementation would be cop= ied in the library of all users. > > Can you please move the implementation to the cpp file? The various setters of this class are also all inlined, so not sure if it w= ould actually gain us much by changing that for the getters. (and I assume = there was a good reason to make most of the methods in this class inlined i= n the first place, otherwise they could of course all be move to the cpp fi= le). > On 2010-09-23 15:16:57, Thomas Zander wrote: > > trunk/koffice/libs/odf/KoGenStyle.h, line 398 > > > > > > What is this part about? This is to mimic the behavior of the addProperty methods. When you add a pr= operty with type=3D=3DDefaultType, it is actually added with type=3Dm_prope= rtyType, so this results in that same behavior on lookup. - Marijn ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5431/#review7731 ----------------------------------------------------------- On 2010-09-23 14:31:27, Marijn Kruisselbrink wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://svn.reviewboard.kde.org/r/5431/ > ----------------------------------------------------------- > = > (Updated 2010-09-23 14:31:27) > = > = > Review request for KOffice. > = > = > Summary > ------- > = > This patch makes the attribute() and property() methods in KoGenStyle pub= lic instead of private. The main use-case for this class is of course only = to set properies and attributes, then insert styles in a KoGenStyles and fo= rget about the actual contents of the style, but sometimes when creating od= f files it is useful to be able to later lookup properties of earlier gener= ated styles, without having to double-store that information. > For example in the xlsx filter this is used because the cell style for me= rged cells is stored in one cell style in odf, but in the xlsx file it is a= combination of properties from the cell styles for all cells that are merg= ed. With this change I can simply merge the cell styles when I know that ce= lls are merged, without having to keep the cell-styles around in another fo= rmat than the KoGenStyle that already contains all relevant information any= way. > = > = > Diffs > ----- > = > trunk/koffice/libs/odf/KoGenStyle.h 1178191 = > = > Diff: http://svn.reviewboard.kde.org/r/5431/diff > = > = > Testing > ------- > = > = > Thanks, > = > Marijn > = > --===============3012251955945182331== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable

This is an automatically generated e-mail. To reply, visit: http://svn.reviewb= oard.kde.org/r/5431/

On September 23rd, 2010, 3:16 p.m., Thomas = Zander wrote:

Making th=
e methods non-private makes me think we should no longer have those methods=
 be inlined as that would mean the implementation would be copied in the li=
brary of all users.
Can you please move the implementation to the cpp file?

The various=
 setters of this class are also all inlined, so not sure if it would actual=
ly gain us much by changing that for the getters. (and I assume there was a=
 good reason to make most of the methods in this class inlined in the first=
 place, otherwise they could of course all be move to the cpp file).

On September 23rd, 2010, 3:16 p.m., Thomas = Zander wrote:
= = =
trunk/koffice/lib= s/odf/KoGenStyle.h (Diff revision 1)
public:
398
        if (type =3D=3D DefaultType) {
What is t=
his part about?

This is to mimic the behavior of the addProperty methods. When you a=
dd a property with type=3D=3DDefaultType, it is actually added with type=3D=
m_propertyType, so this results in that same behavior on lookup.

- Marijn

On September 23rd, 2010, 2:31 p.m., Marijn Kruisselbrink wrote:

Review request for KOffice.

By Marijn Kruisselbrink.

Updated 2010-09-23 14:31:27

Descripti= on

This patch makes the attribute() and property() methods in K=
oGenStyle public instead of private. The main use-case for this class is of=
 course only to set properies and attributes, then insert styles in a KoGen=
Styles and forget about the actual contents of the style, but sometimes whe=
n creating odf files it is useful to be able to later lookup properties of =
earlier generated styles, without having to double-store that information.
For example in the xlsx filter this is used because the cell style for merg=
ed cells is stored in one cell style in odf, but in the xlsx file it is a c=
ombination of properties from the cell styles for all cells that are merged=
. With this change I can simply merge the cell styles when I know that cell=
s are merged, without having to keep the cell-styles around in another form=
at than the KoGenStyle that already contains all relevant information anywa=
y.

Diffs=

trunk/koffice/libs/odf/KoGenStyle.h (11781= 91)

--===============3012251955945182331==-- --===============0543729649== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ koffice-devel mailing list koffice-devel@kde.org https://mail.kde.org/mailman/listinfo/koffice-devel --===============0543729649==--