Re: [Libreoffice-commits] .: writerperfect/source

classic Classic list List threaded Threaded
6 messages Options
Kevin Hunter Kevin Hunter
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-commits] .: writerperfect/source

At 7:24pm -0400 Mon, 24 Oct 2011, Fridrich Štrba wrote:
> New commits:
> commit 38cf494f90d19d1673773437cd52e8f0cfbf4eb5
> Author: Fridrich Štrba <[hidden email]>
> Date:   Tue Oct 25 01:11:49 2011 +0200
>
>     White-space change to fix messy mix of different coding styles

[...]

> diff --git a/writerperfect/source/filter/FontStyle.cxx b/writerperfect/source/filter/FontStyle.cxx
> index 5743e1e..ac501c6 100644

>   void FontStyleManager::clean()
>   {
>       for (std::map<WPXString, FontStyle *, ltstr>::iterator iter = mHash.begin();
> -         iter != mHash.end(); ++iter)
> +            iter != mHash.end(); iter++)
>       {

> @@ -65,7 +63,7 @@ void FontStyleManager::writeFontsDeclaration(OdfDocumentHandler *pHandler) const

>       for (std::map<WPXString, FontStyle *, ltstr>::const_iterator iter = mHash.begin();
> -         iter != mHash.end(); ++iter)
> +            iter != mHash.end(); iter++)

Err, in terms of coding style, is a pre to post increment operator
merely a whitespace change?  I seem to recall a message or two on the
subject but I'm having a difficult time tracking them down just now.
About 8 months ago, perhaps?

Kevin

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Kevin Hunter Kevin Hunter
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-commits] .: writerperfect/source

At 9:48pm -0400 Mon, 24 Oct 2011, Kevin Hunter wrote:

> At 7:24pm -0400 Mon, 24 Oct 2011, Fridrich Štrba wrote:
>> for (std::map<WPXString, FontStyle *, ltstr>::const_iterator iter =
>> mHash.begin();
>> - iter != mHash.end(); ++iter)
>> + iter != mHash.end(); iter++)
>
> Err, in terms of coding style, is a pre to post increment operator
> merely a whitespace change? I seem to recall a message or two on the
> subject but I'm having a difficult time tracking them down just now.
> About 8 months ago, perhaps?

Kevin: open mouth, insert foot.  I apologize.  Just perusing commits,
should've kept going.  I take it the pre to post is needed to "fix the
build".

Kevin

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

Re: [Libreoffice-commits] .: writerperfect/source

On 10/25/2011 04:04 AM, Kevin Hunter wrote:

> At 9:48pm -0400 Mon, 24 Oct 2011, Kevin Hunter wrote:
>> At 7:24pm -0400 Mon, 24 Oct 2011, Fridrich Štrba wrote:
>>> for (std::map<WPXString, FontStyle *, ltstr>::const_iterator iter =
>>> mHash.begin();
>>> - iter != mHash.end(); ++iter)
>>> + iter != mHash.end(); iter++)
>>
>> Err, in terms of coding style, is a pre to post increment operator
>> merely a whitespace change? I seem to recall a message or two on the
>> subject but I'm having a difficult time tracking them down just now.
>> About 8 months ago, perhaps?
>
> Kevin: open mouth, insert foot. I apologize. Just perusing commits,
> should've kept going. I take it the pre to post is needed to "fix the
> build".

This looks more like "fix the build" reverted more than was really
necessary in this case.

In general, and if the expression's value is not used, prefix
increment/decrement is preferable to postfix, as the former conceptually
avoids creation of a temporary that makes the old, unmodified value
available as the expression's value.

That said, you see both forms with more-or-less similar frequency in the
wild.

-Stephan
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Fridrich Strba-4 Fridrich Strba-4
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-commits] .: writerperfect/source

In reply to this post by Kevin Hunter
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Actually, the post/preincrement change was an accidental one. The
problem I have is that the writeperfect source code lives in two
different places. The original code lives in
sourceforge.new/projects/libwpd as writerperfect module. That one
produces standalone converters for all the file-format. And that is
the place where it all started, kind of historical original place. I
normally fix that one and then use the same patch to patch the
libreoffice tree.

It is a bit hard for me to sync those things when the change is done
only in libreoffice and some larger changes are difficult to sync
using patches, so I just copy the corresponding classes from the
SF.net project and reformat the code according to the libreoffice
coding standards. This pre/postincrement change did not happen in the
sf.net repo and during the sync got lost. Although, just in the for
loops, I don't see much difference in the two.

I am thinking about making the writerperfect generators just a library
and use it as any other external library inside libreoffice build. But
that is not for tomorrow.

Cheers

F.

On 25/10/11 04:04, Kevin Hunter wrote:

> At 9:48pm -0400 Mon, 24 Oct 2011, Kevin Hunter wrote:
>> At 7:24pm -0400 Mon, 24 Oct 2011, Fridrich Štrba wrote:
>>> for (std::map<WPXString, FontStyle *, ltstr>::const_iterator
>>> iter = mHash.begin(); - iter != mHash.end(); ++iter) + iter !=
>>> mHash.end(); iter++)
>>
>> Err, in terms of coding style, is a pre to post increment
>> operator merely a whitespace change? I seem to recall a message
>> or two on the subject but I'm having a difficult time tracking
>> them down just now. About 8 months ago, perhaps?
>
> Kevin: open mouth, insert foot.  I apologize.  Just perusing
> commits, should've kept going.  I take it the pre to post is needed
> to "fix the build".
>
> Kevin
>

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.16 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6me3AACgkQu9a1imXPdA+rRwCdFxwc/C6abM2x8gQ8yo1EqHhg
MHMAnA+MiUxLe1EFJCQyh2heenV0CmMw
=fnJH
-----END PGP SIGNATURE-----
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Terrence Enger Terrence Enger
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-commits] .: writerperfect/source

In reply to this post by sberg
On Tue, 2011-10-25 at 09:16 +0200, Stephan Bergmann wrote:
>
> In general, and if the expression's value is not used, prefix
> increment/decrement is preferable to postfix, as the former conceptually
> avoids creation of a temporary that makes the old, unmodified value
> available as the expression's value.

Just to be picky, if my memory of C++ serves, ...

(*) For built-in types, it is true that prefix and postfix
    increment/decrement have the same effect on the variable.

(*) For STL iterators, I guess that the same statement is true.  At
    least, I think that a violation of the symmetry would have
    provoked my "Oh, eeeuuuw!" reflex so strongly that I would
    remember it.  <grin />

(*) For class types in general, the prefix and postfix operators can
    be completely different.  In my opinion, it is evil to do this;
    but the language allows it.

Terry.

>
> That said, you see both forms with more-or-less similar frequency in the
> wild.
>
> -Stephan


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

Re: [Libreoffice-commits] .: writerperfect/source

On 10/27/2011 02:54 PM, Terrence Enger wrote:
> (*) For class types in general, the prefix and postfix operators can
>      be completely different.  In my opinion, it is evil to do this;
>      but the language allows it.

Sure.  Blind clean up can therefore cause trouble, in principle at least
(as can so many refactorings in C++).

Stephan
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice