Hi all, There are some errors in the coding style page I believe. Two big issues:
namespace Inkscape { namespace UI { Sandwich make_sandwich(int x); int make_a_mess(float y); } }
instead of
namespace Inkscape { namespace UI { Sandwich make_sandwich(int x); int make_a_mess(float y); } }
For these kind of forward declarations, I think it is much clearer to indent. Not for class definitions etc.
And
switch (a) { case 1: do_one(); break;
instead of
switch (a) { case 1: { do_one(); break; } ... default: break; }
Thanks for the changes, Johan
(I look forward to the release of the new site!)
On Tue, Sep 17, 2013 at 2:27 AM, Johan Engelen <jbc.engelen@...2592...> wrote:
Hi all, There are some errors in the coding style page I believe. Two big issues:
namespace Inkscape { namespace UI { Sandwich make_sandwich(int x); int make_a_mess(float y); } }
instead of
namespace Inkscape { namespace UI { Sandwich make_sandwich(int x); int make_a_mess(float y); } }
Changed this.
For these kind of forward declarations, I think it is much clearer to indent. Not for class definitions etc.
And
switch (a) { case 1: do_one(); break;
instead of
switch (a) { case 1: { do_one(); break; } ... default: break; }
Because the code already has unindented code after the "case:", I assume you meant the opposite. Is this correct?
i.e
case 1: foo(); bar();
instead of:
case 1: foo(); bar();
2013/9/16 Johan Engelen <jbc.engelen@...2592...>:
Hi all, There are some errors in the coding style page I believe. Two big issues:
namespace Inkscape { namespace UI { Sandwich make_sandwich(int x); int make_a_mess(float y); } }
instead of
namespace Inkscape { namespace UI { Sandwich make_sandwich(int x); int make_a_mess(float y); } }
This is totally inconsistent with how other namespaces are indented.
For these kind of forward declarations, I think it is much clearer to indent. Not for class definitions etc.
And
switch (a) { case 1: do_one(); break;
instead of
switch (a) { case 1: { do_one(); break; } ... default: break; }
This leads to excessive indent, especially with nested switches, as seen in event context code.
Switch labels should not be indented at all, like private/public labels in classes, so your example would look like:
switch (a) { case 1: do_one(); break; case 2: do_two(); break; }
Regards, Krzysztof
On 17-9-2013 16:04, Krzysztof Kosiński wrote:
2013/9/16 Johan Engelen <jbc.engelen@...2592...>:
Hi all, There are some errors in the coding style page I believe. Two big issues:
namespace Inkscape { namespace UI { Sandwich make_sandwich(int x); int make_a_mess(float y); } }
instead of
namespace Inkscape { namespace UI { Sandwich make_sandwich(int x); int make_a_mess(float y); } }
This is totally inconsistent with how other namespaces are indented.
It is, and it looks much clearer in that inconsistent way.
For these kind of forward declarations, I think it is much clearer to indent. Not for class definitions etc.
And
switch (a) { case 1: do_one(); break;
instead of
switch (a) { case 1: { do_one(); break; } ... default: break; }
This leads to excessive indent, especially with nested switches, as seen in event context code.
I think people can exercise their own judgement in the special case of large switch statements like the event code. For the example given, it looks clearer the way I wrote in last email.
Switch labels should not be indented at all, like private/public labels in classes, so your example would look like:
switch (a) { case 1: do_one(); break; case 2: do_two(); break; }
My original post was not meant as a discussion item. It is the way a large part of Inkscape's code looks right now and/or as described here: http://inkscape.org/doc/coding_style.php
Thanks, Johan
participants (4)
-
Johan Engelen
-
Krzysztof Kosiński
-
Martin Owens
-
Samuel Chase