[Kde-pim] Review Request 119576: how to protect a command from a change by using astyle

[David Jarvie]

> On Aug. 4, 2014, 1:20 p.m., Dan Vrátil wrote:
> > Sorry, this is not the right solution - there are many other places in Akonadi where we use X-AKAPPEND that could potentially get broken by astyle and we can't just workaround everything by making it a string. Altering the code just so that astyle understands it is not the right approach :)
> > 
> > As I mentioned in my email to you, next time I just have to be more careful when reviewing the changes.
> 
> Kevin Krammer wrote:
>     Are you sure?
>     Here it is a string, just that the quotes are added by the macro.
>     
>     Do we have any code that uses a character that looks like a minus but isn't and is not inside a string literal?
> 
> Guy Maurel wrote:
>     NO, we don't have (akonadi) *many* places where we use X-AKAPPEND.
>     The first place is libs/protocol_p.h:84:#define AKONADI_CMD_ITEMCREATE       "X-AKAPPEND"
>     Many other places are such server/tests/unittest/akappendhandlertest.cpp:157:        return "C: 2 X-AKAPPEND "...
>     using it as a string.
>     Under kdepimlibs/akonadi you can find:
>     src/core/jobs/itemcreatejob.cpp:176:        command += " X-AKAPPEND ";
>     
>     Having a look to src/5/kde/kdepimlibs/akonadi
>     There are more than 7000 lines with a minus-sign.
>     All are arithmetic operators, or strings with "" or charcter with ''.

Even if the existing code is changed so that astyle can be run without breaking it, there is always the potential for future changes which could be broken by astyle. So I would agree with Dan that altering the code is the wrong approach, since astyle can never be guaranteed to work without issues.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119576/#review63774
-----------------------------------------------------------


On Aug. 2, 2014, 5:03 p.m., Guy Maurel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119576/
> -----------------------------------------------------------
> 
> (Updated Aug. 2, 2014, 5:03 p.m.)
> 
> 
> Review request for KDEPIM and Dan Vrátil.
> 
> 
> Repository: akonadi
> 
> 
> Description
> -------
> 
> astyle cannot (yet) distinguite a hythen from a minus character.
> Using a "string" such as X-AKAPPEND in the macro MAKE_CMD_ROW is a problem.
> astyle surounds the - with spaces.
> I propose to use a second macro MAKE_CMD_ROW2 providing the string as "X-AKAPPEND".
> It might be prettier to do this for all the commands.
> 
> The problem could also happens for the commands:
> X-AKLIST and X-AKLSUB
> 
> 
> Diffs
> -----
> 
>   server/tests/unittest/handlertest.cpp 6fc5d66 
> 
> Diff: https://git.reviewboard.kde.org/r/119576/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guy Maurel
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/