Remove all empty lines from the start of blocks defined with braces

[aaronfranke]

Continuation of #38738, part of #33027.

To make this PR, I added these lines to the file formatting script:

    if [[ $(uname) == "Linux" ]] && [[ "$f" != *"xml" ]]; then
        # Remove empty lines after the opening brace of indented blocks.
        sed -z -i 's/\x7B\x0A\x0A\x09/\x7B\x0A\x09/g' "$f"
        # Remove empty lines before the closing brace (in some cases).
        sed -z -i 's/\x0A\x0A\x7D/\x0A\x7D/g' "$f"
    fi

These changes weren't made by clang-format for various reasons. Some of them are in comments, or aren't in C++ files. Some of them are enums, and for some reason clang-format doesn't change those. For some others, I don't know why clang-format hasn't changed them.

Unlike other PRs I make, I haven't looked over this line-by-line.

[neikeq]

No empty new line allowed after namespaces either?

[akien-mga]

I have no strong opinion about it, but if we want to keep allowing them, I guess @aaronfranke's script could be improved to leave out cases starting with ^namespace {.

[aaronfranke]

In order for sed to modify newlines, we have to use the -z option, which treats the whole file as a single line, and newline characters as normal characters (technically lines are delimited by null). So it would be tricky to make a script that excludes namespace. One easy idea is to just use the script as-is and manually revert changes to namespace blocks.

[akien-mga]

sed is indeed too limited for multiple line matching, but perl -pi -e works well: 07bc4e2

[akien-mga]

    sed -z -i 's/\x7B\x0A\x0A/\x7B\x0A/' "$f"
    # ...the full script has this command repeated a hundred times.

I guess you just needed to use the g flag:

    sed -z -i 's/\x7B\x0A\x0A/\x7B\x0A/g' "$f"

[akien-mga]

Like for namespace {, that's something where we might want to preserve the spacing for readability.

[akien-mga]

There's also empty lines before the closing }; for some enums. It would be great if we managed to fix those with a script too.

[aaronfranke]

This will work for closing braces at the start of lines, but not for indented closing braces.

sed -z -i 's/\x0A\x0A\x7D/\x0A\x7D/g' "$f"

[akien-mga]

The .py files likely need a manual clang-formatting overall, there are lots of style issues.

[akien-mga]

(By manual I mean it should be done by a contributor visually, we can't run clang-format on Python files (one could trick it by copying the content in a .cpp file, format it, and then copy back).

[akien-mga]

clang-format does this weird thing on method definitions which end with an unnecessary semicolon, it's pretty messed up. It would be good to fix all occurrences of this (in a different PR):

$ rg "\(\)\{" -g'!thirdparty'
servers/audio/audio_driver_dummy.cpp
134:AudioDriverDummy::~AudioDriverDummy(){

core/command_queue_mt.h
287:            virtual void post(){};
288:            virtual ~CommandBase(){};

core/os/os.h
521:    virtual void force_process_input(){};

core/os/thread_dummy.h
54:     virtual void lock(){};
55:     virtual void unlock(){};

core/os/os.cpp
96:void OS::debug_break(){
520:void OS::native_video_pause(){
524:void OS::native_video_unpause(){
528:void OS::native_video_stop(){

core/ring_buffer.h
221:    ~RingBuffer<T>(){};

servers/physics/physics_server_sw.cpp
1445:void PhysicsServerSW::sync(){
1586:PhysicsServerSW::~PhysicsServerSW(){

servers/physics_2d/physics_2d_server_sw.cpp
1461:Physics2DServerSW::~Physics2DServerSW(){

core/io/file_access_buffered_fa.h
154:    FileAccessBufferedFA(){

servers/physics/joints/jacobian_entry_sw.h
57:     JacobianEntrySW(){};

drivers/coreaudio/audio_driver_coreaudio.cpp
704:AudioDriverCoreAudio::~AudioDriverCoreAudio(){};

core/vmap.h
205:    _FORCE_INLINE_ VMap(){};

servers/arvr/arvr_interface.cpp
125:ARVRInterface::~ARVRInterface(){};

servers/arvr/arvr_positional_tracker.cpp
235:ARVRPositionalTracker::~ARVRPositionalTracker(){

core/object.h
581:    _FORCE_INLINE_ static void register_custom_data_to_otdb(){};

drivers/dummy/rasterizer_dummy.h
754:    RasterizerStorageDummy(){};
764:    void canvas_begin(){};
765:    void canvas_end(){};

platform/windows/key_mapping_windows.h
42:     KeyMappingWindows(){};

platform/x11/key_mapping_x11.h
44:     KeyMappingX11(){};

platform/uwp/thread_uwp.cpp
67:ThreadUWP::ThreadUWP(){
71:ThreadUWP::~ThreadUWP(){

platform/iphone/ios.mm
84:iOS::iOS(){};

platform/iphone/in_app_store.mm
337:InAppStore::~InAppStore(){};

platform/iphone/icloud.mm
357:ICloud::~ICloud(){};

platform/iphone/game_center.mm
398:GameCenter::~GameCenter(){};

platform/haiku/key_mapping_haiku.h
35:     KeyMappingHaiku(){};

modules/gdscript/gdscript_tokenizer.h
180:    virtual ~GDScriptTokenizer(){};

editor/rename_dialog.h
51:     void _cancel_pressed(){};
115:    ~RenameDialog(){};

modules/camera/camera_win.cpp
56:CameraFeedWindows::CameraFeedWindows(){
78:void CameraFeedWindows::deactivate_feed(){
85:void CameraWindows::add_active_cameras(){
96:CameraWindows::~CameraWindows(){

scene/main/viewport.cpp
177:    TooltipPanel(){};
185:    TooltipLabel(){};

scene/3d/vehicle_body.cpp
47:     btVehicleJacobianEntry(){};

scene/3d/arvr_nodes.cpp
170:ARVRCamera::ARVRCamera(){
174:ARVRCamera::~ARVRCamera(){
389:ARVRController::~ARVRController(){
528:ARVRAnchor::~ARVRAnchor(){
619:ARVROrigin::~ARVROrigin(){

scene/3d/proximity_group.cpp
212:ProximityGroup::~ProximityGroup(){

[Calinou]

By the way, should we also add the missing space between () and {?

[akien-mga]

(The above was from the 3.2 branch. I'm now working on fixing them.)

[akien-mga]

@Calinou That's what I was commenting about yes :)

[akien-mga]

That's another case of weird formatting where the prototype ends with ){, not sure why. The macros also need fixing for the missing braces on single-line statement (commenting here as I see it, but it doesn't have to be handled in this PR).

[akien-mga]

Seems like those happen because the macro call doesn't end with ;. I'll fix that.

[aaronfranke]

@akien-mga When you get a chance to review this again, I re-created the PR and modified the script to only remove newlines from the start of blocks when their contents are indented. This way, blocks for namespaces and extern "C" keep their empty lines at the start.

Also, I tried replacing sed -z -i with perl -i -pe and it did not work correctly for some reason. For now, I just have a check for whether or not the OS is Linux before attempting to run the sed commands, which is the best I could come up with in a short period of time. Another alternative would be to just not have this be in the script, and applied manually occasionally.

[akien-mga]

Needs a rebase (possibly re-run), otherwise seems good.

[akien-mga]

Thanks!

[bruvzg]

New file_format.sh lines do not work on macOS (with BSD flavoured sed), probably should be changed to perl equivalents (see #40706 for similar issues).

sed: illegal option -- z
usage: sed script [-Ealnru] [-i extension] [file ...]
	sed [-Ealnu] [-i extension] [-e script] ... [-f script_file] ... [file ...]

[akien-mga]

There's a [[ $(uname) == "Linux" ]] condition which is meant to prevent using it on macOS, doesn't it work?

[aaronfranke]

@bruvzg That's why there is the if [[ $(uname) == "Linux" ]] check. If anyone knows a Mac-compatible solution, that would be appreciated. But it shouldn't error on macOS due to the Linux check.

[bruvzg]

I did not noticed it. Condition probably works fine. Script did not removed lines from my PRs and I have tested sed commands manually to see what's wrong, script itself do not print any errors.

[bruvzg]

Using gsed instead of sed should word on macOS, if GNU sed is installed (it's not by default).