Discussion: Coding style

[yan12125]

Current coding styles in QTerminal as well as QTermWidget are quite inconsistent. It would be better to have a universal coding style.

1. Identation. There are several places with mixed spaces and tabs. I prefer spaces everywhere.
2. & and * in reference and pointer types, respectively. There are at least three patterns: Foo& foo, Foo & foo, Foo &foo. I like the first. The third is also fine.
3. Do we need brackets for control flow blocks with only one statement? For example:

if (foo)
   bar();

and

if (foo)
{
    bar();
}

I like the second as it's clearer.

Any thoughts?

[agaida]

re 1. full ack re 2. first or last re 3. we should use brackets - even for one statement

[jleclanche]

The most common style throughout lxqt has been k&r.

if (foo) {
    bar();
}

[yan12125]

Thanks @agaida and @jleclanche.

About K&R C: does that mean the left curly bracket if place immediately after if/for/while/...? That is, not in a new line? Seems the other flavor is more common in some recent LXQt changes: https://github.com/lxde/lxqt-session/commit/2525047c3db688a254107c66d6d8e85c39b2664e (by palinek) https://github.com/lxde/lxqt-panel/commit/36667031dd5a9f71f851426e08a4fad7980ac42a (by tsujan)

I have no preference for either flavors. I just want to make things consistent.

For references and pointer types: apparently almost all LXQt codes use the first variant. Now I know what I should do :)

[jleclanche]

Correct. We should probably just use the clang-format as well. Maybe @yan12125 you can give a shot setting it up for qterminal?

[yan12125]

I was thinking whether there's a good linter or formatter for C++, and a tool from the LLVM team sounds promising :) I'll check how to use it.

[yan12125]

Here's a test result on qtermwidget: https://github.com/lxde/qtermwidget/compare/test-clang-format.

The file .clang-format controls how clang-format reformats codes. There are several templates, and among them I find Webkit is the closest one to existing formats in other LXQt components.

Overall, the result is quite satisfying. The only missing feature in clang-format is that it doesn't indent nested preprocessor directives. For example, it changes the following code:

#ifdef HAVE_FOO
#  ifdef HAVE_BAR
#    include <foo/bar.h>
#  endif
#endif

into

#ifdef HAVE_FOO
#ifdef HAVE_BAR
#include <foo/bar.h>
#endif
#endif

It's not a big deal for me as complicated preprocessor directives are uncommon in qtermwidget.

[pas2k]

Hope it's not too late to join in :-)

1. Spaces. Rationale: the only advantage of tabs disappears on multiline function declarations:

std::auto_ptr<CSomeHugeClassName> *frobnicate(CAnotherClassNameHere arg1, int arg2,
      <Tabs mixed with spaces here>           enum SomeEnum arg3)

2. Foo &foo. Rationale: in C and C++, * and & are (arguably counterintuitively) variable-associated:

int *a = NULL, *b = NULL;

If you write int* a = NULL, b = NULL;, the code will look right, but in fact would be wrong.

3. +1 for brackets everywhere. As for style, I prefer K&R. However, if newline brace is to be accepted, consider Horstman style to make code both readable and compact:

if (foo)
{   bar();
    baz();
}

[yan12125]

Personally I don't really like Horstman style - that breaks the symmetric pattern of the code.

[apjanke]

Is there a consensus here now? Can this be closed? And maybe summarized in a “Code style” section of the CONTRIBUTING.md file?

[yan12125]

I think the Webkit style from clang-format is almost the consensus. Just need to check whether it fits expectations listed in all comments above or not.

[apjanke]

What about spaces inside the parentheses for function calls and if/for statements? I see a mix of styles here.

https://github.com/lxqt/qtermwidget/blob/e292763e6d23fd92ac30cad9ac107709b8c47c71/lib/TerminalDisplay.cpp#L281-L292

[yan12125]

Looks like there are no spaces inside parentheses in other LXQt projects.

[tsujan]

IMHO, whatever style is used by the original author(s) of a program should be followed by other devs that work on it. I use PCMan's style in libfm-qt but 2 different styles in my own apps. The style isn't important; its consistency is.

[apjanke]

Yep. I don't much care what the style is (except I hate Horstman braces, sorry @pas2k), just that it's consistent within a project. It's a small annoyance that I'm reading through the QTerminal codebase and I'm seeing style inconsistency even within a file.

On that note: would you like PRs that just do style clean-ups to match whatever's decided here, or leave that until there's an actual behavioral change in the code?

[tsujan]

It's a small annoyance that I'm reading through the QTerminal codebase and I'm seeing style inconsistency even within a file.

That means some devs didn't respect the original style. Bad? Yes! But, sometimes, it isn't easy to forget one's own style.

would you like PRs that just do style clean-ups to match whatever's decided here

I can only speak for myself: no! My reason is that such changes may introduce bugs -- typos or irresistible urges to change some codes ;) We've seen that in the case of replacing 0 with nullptr.

[apjanke]

Understood. I'll keep the style changes to myself, and just chime in with substantive PRs.

[yan12125]

I don't remember why this was closed in 2019. Now I prefer to keep this open until clang-format or another equivalent tool is integrated.

[juniorohanyere]

Try looking into betty coding style. I'm not sure if it works for c++, but it's a good choice for C codes

[yan12125]

I prefer a coding style that can be integrated with clang-format (or an equivalent tool). Is betty such one? Showing git diff output for qterminal and/ qtermwidget after applying the coding style is even better for discussions.