Re: [Libreoffice-commits] core.git: [API CHANGE] notebookbar: paragraph spacing controls

classic Classic list List threaded Threaded
3 messages Options
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-commits] core.git: [API CHANGE] notebookbar: paragraph spacing controls

On 06/27/2016 02:47 PM, Szymon Kłos wrote:
> commit 723467bd88a50323ccd2e4046d0a36332c664a66
> Author: Szymon Kłos <[hidden email]>
> Date:   Tue May 31 16:57:13 2016 +0200
>
>     [API CHANGE] notebookbar: paragraph spacing controls

Is this an incompatible change?  (Otherwise, it shouldn't be labelled
"[API CHANGE]".)

>
>     Change-Id: I9d2672cd156f2dcc2ee4c544902e9d42632cab70
>     Reviewed-on: https://gerrit.libreoffice.org/26039
>     Tested-by: Jenkins <[hidden email]>
>     Reviewed-by: Samuel Mehrbrodt <[hidden email]>
>
[...]

> diff --git a/offapi/com/sun/star/frame/status/LeftRightMarginScale.idl b/offapi/com/sun/star/frame/status/LeftRightMarginScale.idl
> new file mode 100644
> index 0000000..b5f66e2
> --- /dev/null
> +++ b/offapi/com/sun/star/frame/status/LeftRightMarginScale.idl
> @@ -0,0 +1,78 @@
> +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
> +/*
> + * This file is part of the LibreOffice project.
> + *
> + * This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/.
> + *
> + * This file incorporates work covered by the following license notice:
> + *
> + *   Licensed to the Apache Software Foundation (ASF) under one or more
> + *   contributor license agreements. See the NOTICE file distributed
> + *   with this work for additional information regarding copyright
> + *   ownership. The ASF licenses this file to you under the Apache
> + *   License, Version 2.0 (the "License"); you may not use this file
> + *   except in compliance with the License. You may obtain a copy of
> + *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
> + */
> +#ifndef __com_sun_star_frame_status_LeftRightMarginScale_idl__
> +#define __com_sun_star_frame_status_LeftRightMarginScale_idl__
> +
> +
> +
> +module com {  module sun {  module star {  module frame {  module status {
> +
> +
> +/** specifies a left and right margin.
> +
> +    @since LibreOffice 5.3
> + */
> +struct LeftRightMarginScale
> +{
> +    /** specifies a left text margin in 1/100th mm.
> +     */
> +    long TextLeft;
> +
> +
> +    /** specifies a left margin in 1/100th mm.
> +     */
> +    long Left;
> +
> +
> +    /** specifies a right margin in 1/100th mm.
> +     */
> +    long Right;
> +
> +
> +     /** specifies a first line indent relative to TextLeft.

also in 1/100th mm?

> +     */
> +    long FirstLine;
> +
> +
> +    /** specifies a scale value for the left margin.

as a percentage value?  why have both fixed values and scaling for these
margins?

also, I wouldn't bother with 'short' and just use 'long' for all these
values

> +     */
> +    short ScaleLeft;
> +
> +
> +    /** specifies a scale value for the right margin.
> +     */
> +    short ScaleRight;
> +
> +
> +    /** specifies a scale value for the first line margin.
> +     */
> +    short ScaleFirstLine;
> +
> +
> +    /** specifies if the automatic calculation of the first line indent occurs.
> +    */
> +    boolean AutoFirstLine;
> +};
> +
> +
> +}; }; }; }; };
> +
> +#endif
> +
> +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
[...]
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
eszka eszka
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-commits] core.git: [API CHANGE] notebookbar: paragraph spacing controls

Dnia 2016-06-27, pon o godzinie 15:00 +0200, Stephan Bergmann pisze:

> On 06/27/2016 02:47 PM, Szymon Kłos wrote:
> > commit 723467bd88a50323ccd2e4046d0a36332c664a66
> > Author: Szymon Kłos <[hidden email]>
> > Date:   Tue May 31 16:57:13 2016 +0200
> >
> >     [API CHANGE] notebookbar: paragraph spacing controls
>
> Is this an incompatible change?  (Otherwise, it shouldn't be labelled
> "[API CHANGE]".)
>

I labelled it with [API CHANGE] because during review Maxim wrote:
"Since we're touching offapi/, please add [API CHANGE] to the commit
message.". I didn't know that patch have to introduce some
 incompatibility to be labelled in this way :)

...

> > +
> > +
> > +     /** specifies a first line indent relative to TextLeft.
>
> also in 1/100th mm?
>

Yes, also in 1/100th mm.

> > +     */
> > +    long FirstLine;
> > +
> > +
> > +    /** specifies a scale value for the left margin.
>
> as a percentage value?  why have both fixed values and scaling for
> these
> margins?
>
> also, I wouldn't bother with 'short' and just use 'long' for all
> these
> values
>

Yes, as a percentage value. SvxLRSpaceItem stores both fixed values and
scaling so I added this fields too.

> > +     */
> > +    short ScaleLeft;
> > +
> > +
> > +    /** specifies a scale value for the right margin.
> > +     */
> > +    short ScaleRight;
> > +
> > +
> > +    /** specifies a scale value for the first line margin.
> > +     */
> > +    short ScaleFirstLine;
> > +
> > +
> > +    /** specifies if the automatic calculation of the first line
> > indent occurs.
> > +    */
> > +    boolean AutoFirstLine;
> > +};
> > +
> > +
> > +}; }; }; }; };
> > +
> > +#endif
> > +
> > +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
> [...]

Should I change these short types to long and add missing metrics to
the description?

Regards,
Szymon
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-commits] core.git: [API CHANGE] notebookbar: paragraph spacing controls

On 06/27/2016 07:04 PM, Szymon Kłos wrote:

> Dnia 2016-06-27, pon o godzinie 15:00 +0200, Stephan Bergmann pisze:
>> On 06/27/2016 02:47 PM, Szymon Kłos wrote:
>>> +     */
>>> +    long FirstLine;
>>> +
>>> +
>>> +    /** specifies a scale value for the left margin.
>>
>> as a percentage value?  why have both fixed values and scaling for
>> these
>> margins?
>>
>> also, I wouldn't bother with 'short' and just use 'long' for all
>> these
>> values
>>
>
> Yes, as a percentage value. SvxLRSpaceItem stores both fixed values and
> scaling so I added this fields too.

Looks a bit odd to me, but I'm anything but a domain expert here, so
don't have any idea whether that's good or bad design.  Just remember
that the entities in the API should make sense from a client's
perspective, not necessarily reflect whatever existing implementation
design.

> Should I change these short types to long and add missing metrics to
> the description?

That's what I'd do, yes.
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice