Investigate styling of else blocks with/without `{`

[ann0see]

Throughout the Code we sometimes omit { in if/else blocks which might be confusing for developers and clang format. In general think this is a somewhat questionable style which led to confusion. We should overthink when omitting { is really ok.

Related to a comment by pljones:

So this is the one I think is questionable... I think. It should be same level as after the if on line 920.

Originally posted by @pljones in #3020 (comment)

[pljones]

Note that the reference comment was not about omitted {. It's about the insertion of comments between a keyword and its {. clang_format does the right thing with the { block, as far as I could tell: #3020 (comment)

According to the project coding standards, omitting { ... } around a block is never permitted, as I understand them.

[ann0see]

Then comments should also be inside the blocks...

[pljones]

Actually... now I look again, I think it's styling is not as I'd like it... but maybe not "wrong" by the clang-format rules...

        else
            //### TODO: END ###//
            // if "directorytype" itself is set, use it (note "AT_NONE", "AT_DEFAULT" and "AT_CUSTOM" are min/max directory type here)
            //### TODO: BEGIN ###//
            // compatibility to old version < 3.8.2
            if ( GetNumericIniSet ( IniXMLDocument,

The { and } before the else are correctly nested at the same level as the else. The comment causing a break in the middle of the else if statement is what throws things. The { and } for the last line above are nested below the if rather than the else, as they (questionably) should be. We use the TODO: BEGIN/TODO: END in several places... The problem here is the statement breaks - it's going to cause weird styling and I think we have to live with it.

if (a == b)
{
  // do stuff
}
else
  if (c == d)
// ... code block here

It would put the brackets under the if rather than the else even without the comment, I'd imagine.

[ann0see]

Hmm. Now I seem to get it. Yeah.

else // something 
  if (...) 
  {
    ...
  }

Should probably never be allowed.

Always write else and if on the same line

} else if (something) 
{

Or use (not recommending that though)

else
{
  if ( something ) 
  {
  }
}

[pljones]

However, we've adopted guidance that deprecated code should be bracketed by comments that enclose only the deprecated code (such that the BEGIN to END lines can simply be deleted and leave the code compilable).

That means if an if (..) { .. } else block prefixes an alternative flow (introduced by its own if), the code to be deleted is the first if to the else but not the second if - which forms else if in normal layout. We end up with

// TODO: BEGIN
if (..)
{
  ..
} else
// TODO: END

which can be removed, followed by

if (..)
{
  ..
}

... but clang-format doesn't think the second if (or the preceding comments or following { .. } block) should be at the same level as the first, but indented.

[pljones]

I have a feeling that - as most of this is about problems caused by "TODO - remove this sometime" blocks - we'll ignore this issue in the same way we ignore the "TODO - remove this sometime" instruction.

[ann0see]

Closing. Don't think it's worth having this open anymore.