Formatting: Drop tab usage

[mrjimenez]

I am not entirely against dropping tabs, but I would like a good reason to do that.

On the "pro tabs" side, tabs for indentation avoid the "how many spaces will be used for indentation" wars. I use 8, but I am old school. Javascript folks like 2. You can make both worlds happy with tabs.

On the other hand, you can enforce the number of spaces with clang_format and just fight the "tabs wars" once.

What are your thoughts?

[whyman]

I think the problem is the mix of tabs and spaces in the code. Particularly github seems to not expect 8 wide tabs, which makes things look odd.

My hope here was to unify the two at 8 spaces, so it at least looks consistent.

I dont really care about the indent size (my personal pref is smaller is surely better if we stick to an 80 char line length, perhaps 4 as a tradeoff)

[whyman]

Seems that my local clang format and the one in the action disagree 😁

Will have a look at what is happening there in the morning

[mrjimenez]

Well, I'd rather have a tab as 8 spaces, but hey, it is your call since you are the one doing the job. I agree with whatever you decide, 4 spaces seems like a nice tradeoff.

If clang-format does the job for me, I'm ok with whatever you do. I might even adopt it in other projects,,, :laughing:

[whyman]

Sigh, seems like MSVC dislikes the changes for some reason.

The error messages win the award for least helpful errors by a long way.

[whyman]

Fixed up the windows failure, needed to ensure winsock2.h was included before other things

[mrjimenez]

Great job, looks very nice to me.

I will commit tomorrow, to give some time in case someone has anything to add. This is usually a very sensitive issue, people are very attached to their editor configurations.

[whyman]

@Vollstrecker you mean the preprocessor changes?

The problem is that they are idented seperately to the code itself, so it leads to odd situations with indented #ifdefs and not code

Plus older compilers require preprocessor directives without spaces beforehand.

[Vollstrecker]

ixml/inc/ixmldebug.h for example (first file) they are the same level indented as the code they affect. So it's obvious where they belong. Especially nested ifdef's get really unreadable when on the same level. Maybe an empty line to separate them is better in that case.

For the compilers, I always test on debian stable, which I consider old.How many systems are still out there with older software that can't handle this, but need a up-to-date libupnp?

[whyman]

Everyone needs an up to date libupnp as the old ones are full of security issues :grin:

[mrjimenez]

I think indenting #ifdefs is not a good thing. I have tried it, but the results are not good.

Ian, as promised, I will commit it now, but please do continue if you still have things to add.

[mrjimenez]

Oh, and @whyman , before committing, I was thinking about squashing this PR instead of rebase and merge, since some intermediary commits did not pass the tests, is there a problem with that?

[whyman]

Squashing is of course fine!

[mrjimenez]

I normally don't like to squash because it looses the evolution of the code, not to mention that more granular commits are always easier to understand.