Deskbar Style Changes and Refactoring

[jscipione]

This might be a small abuse of the trac system but I created this ticket so I'd have a place to put patches that relate to style changes and refactoring. None of these changes should have any effect on Deskbar from a user's perspective but they either make the code style nicer, or eliminate code duplication, or give a variable a better name.

[jscipione]

Please review this patch, if you approve the changes, apply the patch and close this ticket, if you don't tell me what is wrong. Thanks

[jscipione]

Does some refactoring on WindowMenuBar.cpp and WindowMenuBar.h that I noticed while fixing more things

[tqh]

I guess I'll try to get my hands dirty with a style review tomorrow. This will probably end well ;)

[tqh]

While I think your changes looks ok from a quick view, I find there is no need for the silliness done in TBarApp using be_app to call BarView(). I can't really see why TBarApp can't call BarView() directly. If I'm not mistaken I think that call would be better than introducing a member for fBarView.

Why use this source:haiku/trunk/src/apps/deskbar/BarApp.cpp#L426 when this source:haiku/trunk/src/apps/deskbar/BarApp.h#L135 can be called directly?

[jscipione]

Thank you for reviewing my patch. I grabbed a pointer to the barView in the constructor because the code to grab the barView was being called many times. This way, the pointer to the barView is gotten just once, and is used several times rather than having to grab the pointer over and over again. DRY

[jscipione]

I see what you mean about calling BarView() directly though, I didn't see that function, apparently neither did the original author. I can rework the patch to use that function instead.

[tqh]

A suggestion from DeadYak:

looking at the context it's called in, I'd honestly say it might be better to update the settings and then resend the message to BarWindow to process

So you might want to try and see if the old code can't be written even better. You can ofc just stick to style improvement if you don't want to work on that. Any work is appreciated.

[jscipione]

I created this particular ticket strictly for style changes only. Any functional changes would go somewhere else. Since I really want the other style changes in the above patches committed (gets em' out of my patches) I'll just remove the controversial bits for now.

[jscipione]

Call BarView() function to get the barView pointer instead of creating an fBarView pointer

[jscipione]

Deskbar Style Super Mega Patch. Delete evil spaces at the beginning and end of lines. No your comments are not so special that they deserve and extra indent. No your comments are not so special that they get a tab after instead of a space. No you cannot use a tab instead of a space if the tab happens to indent by 1 space. Spaces around binary operators. Other stuff.

[jscipione]

This patch is big vim style checker found even more style issues including that the Open Tracker license broke the 80 char limit in almost every file! yeah! Also more evil spaces deleted, more 80 char limit fixes, more spaces around binary operators. Single line comments get indented *after* the line they affect as per style guide.

[tqh]

Nice. The patches is beginning to be large, so further changes should probably be divided in to several patches. This way one patch at a time can be reviewed and applied easily.

There is a branch for Deskbar/Tracker rewrite that might be affected if to big changes are done. I do not know what the state of that branch is, so I'm a bit worried of how to handle this. It probably need more devs though, as you have seen the code can be improved and refactored.

[aldeck]

Replying to tqh:

There is a branch for Deskbar/Tracker rewrite that might be affected if to big changes are done. I do not know what the state of that branch is, so I'm a bit worried of how to handle this. It probably need more devs though, as you have seen the code can be improved and refactored.

Don't worry, the tracker_layout branch only touches src/kits/Tracker. I'm taking a Haiku break at the moment but i'm still around :)

[jscipione]

The patches is beginning to be large, so further changes should probably be divided in to several patches. This way one patch at a time can be reviewed and applied easily.

Yes it is large, there are a lot of style problems :) The only style problems I see that are controversial are (line numbers from Deskbar Style Changes and Refactoring 4.diff)

    StatusView.cpp line 1044
    Switcher.cpp line 1470
    Switcher.cpp line 2066

Those comments maybe should be indented. Perhaps I got a little bit too zealous with the unindenting. Perhaps some style expert can comment here.

[jscipione]

I really wish there was a way to upload multiple patches at once... anyway I split my patch into individual diff files so they can be reviewed and applied separately. Because they are style changes _only_ you should be able to apply each file separately without breaking anything. I also accidentally uploaded some unrelated patches so please ignore Deskbar Autoexpand Teams 1.diff, Deskbar Disable Expand Options 1.diff, and Deskbar Resize Team Icons 5.diff. Those are patches to other things, not style related. I marked them as obsolete to reduce the confusion. Sorry I have way too many patches floating around. Anyway, if you like patches you'll love this ticket. Please review my patches and apply them if they are good, fix them if they are not. Thanks!

[tqh]

• Deskbar_ExpandoMenuBar_cpp_style_fix.diff
• Deskbar_ExpandoMenuBar_h_style_fix.diff
• Deskbar_LICENSE_style_fix.diff

applied in hrev40744.

[tqh]

Deskbar_BarApp patches applied in hrev40814.

[tqh]

• Deskbar_BarMenuBar_cpp_style_fix.diff
• Deskbar_BarMenuBar_h_style_fix.diff
• Deskbar_BarMenuTitle_cpp_style_fix.diff
• Deskbar_BarMenuTitle_h_style_fix.diff
• Deskbar_BarView_cpp_style_fix.diff
• Deskbar_BarView_h_style_fix.diff

Applied in hrev40815.

[tqh]

• Deskbar_BarWindow_cpp_style_fix.diff
• Deskbar_BarWindow_h_style_fix.diff
• Deskbar_BeMenu_cpp_style_fix.diff
• Deskbar_CalendarMenuWindow_cpp_style_fix.diff
• Deskbar_BeMenu_h_style_fix.diff
• Deskbar_CalendarMenuWindow_h_style_fix.diff
• Deskbar_DeskBarUtils_cpp_style_fix.diff
• Deskbar_DeskBarUtils_h_style_fix.diff

Applied in hrev40817 and hrev40818.

[tqh]

• Deskbar_PreferencesWindow_cpp_style_fix.diff
• Deskbar_PreferencesWindow_h_style_fix.diff
• Deskbar_ResourceSet_cpp_style_fix.diff
• Deskbar_ResourceSet_h_style_fix.diff
• Deskbar_ShowHideMenuItem_cpp_style_fix.diff
• Deskbar_ShowHideMenuItem_h_style_fix.diff
• Deskbar_StatusView_cpp_style_fix.diff
• Deskbar_StatusView_h_style_fix.diff
• Deskbar_StatusViewShelf_cpp_style_fix.diff
• Deskbar_StatusViewShelf_h_style_fix.diff

Applied in hrev40847.

[tqh]

• Deskbar_Switcher_cpp_style_fix.diff
• Deskbar_Switcher_h_style_fix.diff
• Deskbar_TeamMenu_cpp_style_fix.diff
• Deskbar_TeamMenu_h_style_fix.diff
• Deskbar_TeamMenuItem_cpp_style_fix.diff
• Deskbar_TeamMenuItem_h_style_fix.diff
• Deskbar_TimeView_cpp_style_fix.diff
• Deskbar_TimeView_h_style_fix.diff
• Deskbar_WindowMenu_cpp_style_fix.diff
• Deskbar_WindowMenu_h_style_fix.diff
• Deskbar_WindowMenuItem_cpp_style_fix.diff
• Deskbar_WindowMenuItem_h_style_fix.diff

Applied in hrev40849. Hopefully I didn't miss anything.