Cleanup and code maintenance

[guitorri]

• [ ] normalize line ending in all files, set all to LF
• [ ] add a .gitattribute to remind us of the line ending
• [ ] trim trailing whitespaces in all files
• [ ] remove indentation tabs in favor of spaces
• [ ] check header include guards (#268)
• [x] reduce a the QT3_SUPPORT deprecation warnings (#662)
• [ ] reduce and remove header inclusion, favor forward declaration

[in3otd]

maybe also remove tabs used for indentation in several files where mainly spaces are used. Most of the times it seems that a tab there was set to 2 spaces, but it will need to be checked for each case. (and surely I too added some stray tabs here and there :neutral_face: )

[guitorri]

Added tabs to the list.

[nvdl]

Is there any tool available out-of-box to achieve this? What about code formatting?

if () {
    a;
}

or

if ()
{
    a;
}

[felix-salfelder]

@nvdl there's indent.

[nvdl]

Gave it a quick look. Looks like it can do most (or probably all) of the needed stuff. Have to come up with the formatting rules template. Or do we have any?

[felix-salfelder]

let's not overdo it. e.g. indent -linux -i2 -nut outputs something useful for us (and is pretty well defined).

NB1: -gnu is default, but it's plain ugly (sorry). NB2: anyway, there's nothing reasonable about -i2 and -nut, but i anticipate that you will insist :(

[felix-salfelder]

just noticed that this issue is much harder than it looks. indent expectedly and consistently (and obviously) screws up custom indentations.

we are going to have a lot of fun with these (or imply nonsensical restrictions).

int bar(int a,
        int b)
{
  foo();
}

[nvdl]

Yes, I saw the same thing on my side with indentation. Will give it a better look later but it seems we need a formatting template that translates to the "so many" switches it supports. I like one liner like:

int bar(int a, int b) {
  do do;
{

It does something interesting to commented out code; it strips away the indentation.

[felix-salfelder]

I like one liner like:

sure, the example was made up. e.g. what if the one liner does not fit into one line?

[in3otd]

I resisted mentioning code formatting up to now, but since the discussion is started...

in the past we (briefly) discussed this, it was suggested to use clang-format, IMHO it handles things a little better than indent. Of course it can be configured to format every little detail as desired, but to avoid long discussions about where to put which braces when, I suggest to use just one predefined style (Personally I like clang-format -style=LLVM). It will be in any case better than the plethora of odd styles we have in the code.

Another general point, if we agree to re-format the code, is when to do it. On all the code base at once? Just when someone modifies a file? Or even done automatically when committing by a git hook?

[felix-salfelder]

I resisted mentioning code formatting up to now [..]

It's a can of worms indeed. i would be happy to just agree on a style and then apply it to additions and changes. then, most code will have to be changed sooner or later... question is, how to check this automatically?

clang-format -style=LLVM

a newline after a function header truly improves readability.

try (LLVM)

void foo(int X){
  statement1(X);
  statement2(X);
  statement3(X);
}

vs indent -linux -i2 -nut

void foo(int X)
{
  statement1(X);
  statement2(X);
  statement3(X);
}

YMMV (i don't insist). just pointing out flaws :D

[guitorri]

+1 for clang-format. For any automated solution really.

@felix-salfelder if it is really needed we can start with a style (say LLVM) and fine tune particular things in a local style file, .clang-format I think.

Developers should be required to run automatically clang-format as a git hook. Not sure we can expect PR contributors to do that.

Every so often (after/before release ?) the codebase is reformatted to fix eventual style issues brought in with PRs. I don't think we should be spending (wasting IMHO) much time telling people on PRs that the style is a little off.

I think we should do the formatting in one go (one commit) first qucs then qucs-core. I just don't know when would be the best time.

I am not sure how the diffs will look like after the formatting. Are there any potential issues like conflicts when merging old branches with the formatted code? We might need to merge and reformat afterwards. It might be work look around for hindsight in other projects that went thru this.

[nvdl]

We have to think about one style and apply that once as a milestone or something.

Diffs are going to be affected. I had an experience with diff in a PR where the code was formatted mostly with small changes. The whole PR looked "green".

The diffs are going to be affected but that should not prevent someone from seeing the "real" changes easily if the old source is also formatted prior to diff.

I think putting this requirement on PR people will sway them away. Better to have weekly, monthly or milestone-ly formatting session.

The formatting is also somewhat related to translation fixes (#256) but I have not followed that for a while. Are those strings fixed?

sure, the example was made up. e.g. what if the one liner does not fit into one line?

I will still keep that at one line but that is me!

[felix-salfelder]

We have to think about one style and apply that once as a milestone or something.

let me propose to do that just before the 1.0.0 release. the benefit (suboptimal code vs. well formatted suboptimal code) is not worth the rebasing trouble (yet). making stuff modular, will touch/move a lot of the code. it is already pretty hard to keep track of changes.

a CI check, only skimming through added lines, will perfectly shape up our code, without extra hassle to anyone.

I think putting this requirement on PR people will sway them away.

just drop all requirements for /contrib stuff (that's where i suggested to stash 3rd party contributions, until the interfaces are stable). also it is fairly easy to fix whitespace before a merge. the more so, once the CI provides a patch as a build artifact...

[nvdl]

suboptimal code vs. well formatted suboptimal code

:laughing: Well, at least it will look elegant.

This optimization is going to take more time than formatting anyway.

[felix-salfelder]

This optimization is going to take more time than formatting anyway.

to put it straight: rebasing previous work is going to be hell after the reformatting.

[nvdl]

to put it straight: rebasing previous work is going to be hell after the reformatting.

I do not know if it is possible to format the whole commits history. Another branch?

[in3otd]

About cleanup, I think there are around some debug messages in GUI code that use std::cout/std::cerr instead of Qdebug() which means these messages are printed even for release versions, not configured with --enable-debug. IIRC, that was mainly in some tools.