Big style cleanup

[humdingerb]

Had a bit of trouble resolving merge conflicts before, but after doing that at least 3 times, it seems to stick...

[humdingerb]

This is a rather large PR, though the actual changes a code-style-minimal...

[korli]

The CI has found that the Marble addon fails to build:

Marble.cpp: In member function 'int32 MarbleManipulator::thread_function(int32)': Marble.cpp:182:44: error: 'sy' was not declared in this scope; did you mean 'y'? 182 | } else if (((int32)sy % update_interval) == 0) | ^~ | y`

[humdingerb]

The CI has found that the Marble addon fails to build:

Marble.cpp: In member function 'int32 MarbleManipulator::thread_function(int32)': Marble.cpp:182:44: error: 'sy' was not declared in this scope; did you mean 'y'? 182 | } else if (((int32)sy % update_interval) == 0) | ^~ | y`

Yeah, I fixed this and other stuff, too. I stupidly forgot to "build.sh addons" and so only checked the main app...

[humdingerb]

@korli, sorry I may have overwritten your changes when I pushed my fixes just now before seeing your last 2 comments.

[humdingerb]

@korli, seeing you're reviewing things right now, I'll wait with any fixes... Thanks for the review!

[korli]

@korli, seeing you're reviewing things right now, I'll wait with any fixes... Thanks for the review!

I'm not sure what style is correct for extern "C" { and } break;

[humdingerb]

@korli, seeing you're reviewing things right now, I'll wait with any fixes... Thanks for the review!

I'm not sure what style is correct for extern "C" { and } break;

The `extern "C" comes straight from 'haiku-format's. The '{ break;' in case statements is, I think, @dsizzle-style. I like it, because it's a bit more compact then adding more linebreaks, and the 'break' stands out a bit more than when it's indented the same as the lines above.

[korli]

The `extern "C" comes straight from 'haiku-format's. The '{ break;' in case statements is, I think, @dsizzle-style. I like it, because it's a bit more compact then adding more linebreaks, and the 'break' stands out a bit more than when it's indented the same as the lines above.

With extern "C" { on one line, one knows it's not a function.

[dsizzle]

I’m fine with erring towards Haiku style. That was my intent originally but I got some things wrong and haven’t gone back to fix them. As long as it’s clean and consistent I’m happy.

I do want to review this in detail though, especially the conflicts.

But overall I really really appreciate this!

[humdingerb]

Do take your time. I think I have fixed those conflicts already. I'll have another look tomorrow and will address korli's comments then as well.

[humdingerb]

BTW, I haven't touched any header files yet. Mainly because haiku-format wreaks havoc with the indentation there. Maybe I'll just do some pointer* cleanup there. Or, if @dsizzle has an example for a 'perfect' header, I'll try to replicate what's there. It's mainly the indentation that sooner or later starts to confuse me. There always see to be some exception to the rule when it comes to long types, for example.

[humdingerb]

Outing myself as a bloddy dabbler again... :) As soon as I re-order the #includes, things fail to compile. For example, I changed the includes of AntiDitherer.cpp to:

#include "AntiDitherer.h"

#include "AddOns.h"
#include "ManipulatorInformer.h"
#include "Selection.h"

#include <Bitmap.h>
#include <Catalog.h>
#include <CheckBox.h>
#include <ClassInfo.h>
#include <LayoutBuilder.h>
#include <Spinner.h>
#include <StatusBar.h>
#include <Window.h>

#include <string.h>

resulting in errors:

In file included from AntiDitherer.cpp:11:
AntiDitherer.h:107:73: error: field 'target' has incomplete type 'BMessenger'
  107 |                 BMessenger                                              target;
      |                                                                         ^~~~~~
In file included from /boot/system/develop/headers/os/support/Archivable.h:10,
                 from /boot/system/develop/headers/os/app/Handler.h:12,
                 from /boot/system/develop/headers/os/interface/View.h:12,
                 from ../../../artpaint/viewmanipulators/WindowGUIManipulator.h:17,
                 from AntiDitherer.h:13:
/boot/system/develop/headers/os/app/Message.h:29:7: note: forward declaration of 'class BMessenger'
   29 | class BMessenger;
      |       ^~~~~~~~~~
AntiDitherer.h:111:17: error: 'BSpinner' does not name a type
  111 |                 BSpinner                                                *block_size_control;
      |                 ^~~~~~~~
AntiDitherer.h:112:17: error: 'BCheckBox' does not name a type
  112 |                 BCheckBox                                               *reduce_resolution_box;
      |                 ^~~~~~~~~
AntiDitherer.cpp: In constructor 'AntiDithererManipulatorView::AntiDithererManipulatorView(AntiDithererManipulator*, const BMessenger&)':
AntiDitherer.cpp:288:9: error: 'block_size_control' was not declared in this scope
  288 |         block_size_control
      |         ^~~~~~~~~~~~~~~~~~
AntiDitherer.cpp:293:9: error: 'reduce_resolution_box' was not declared in this scope
  293 |         reduce_resolution_box = new BCheckBox("reduce_resolution", B_TRANSLATE("Reduce resolution"),

etc. I'm already convinced I won't touch those messy .h files any more than absolutely necessary... :)

[dsizzle]

It is true that rearranging include files can upset the Dance of the Compiler Fairies, so let's leave that out of the scope of this review. :smile:

[humdingerb]

I'm done with the code style changes for now. I'll make any additional changes you guys find, otherwise feel free to merge.

[humdingerb]

Fixed the last typos. Feel free to merge (squash a few things first?)

[dsizzle]

merged! :crossed_fingers: