Unity-Technologies / qstat

New official qstat repository
Artistic License 2.0
121 stars 33 forks source link

[Do Not Merge] Uncrustify #29

Closed illwieckz closed 7 years ago

illwieckz commented 9 years ago

Hi @stevenh, this is an effort to clean the source (spaces, indentation, braces, etc.) with an uncrustify profile I generated trying to follow https://www.freebsd.org/cgi/man.cgi?query=style&sektion=9 .

There is two choices I made.

There is only one thing I've not managed to get is a nice #define value alignment. All the other things are nice.

There is an uncrustify.cfg file added, this is the profile to clean the source.

What do you think about it and how it looks? :-)

This is the changes: https://github.com/illwieckz/qstat/compare/qstat-help...illwieckz:uncrustify

But Github will not really allows you to see the change, because there is too many files changed and the online diff tool sucks. Using a local tool will help you so much to review the change.

For example, use gitk with 0 lines of context, and you can also tick the “Ignore space changes” option to only review the line changes (like the function definition, block bracing etc.).

This PR can't be merged before #27, but currently I just ask some review. :-)

stevenh commented 9 years ago

Seems good.

illwieckz commented 9 years ago

I will replay this change after other PR will be merged.

stevenh commented 7 years ago

will get this done soon

illwieckz commented 7 years ago

Since I used that uncrustify profile on some other projects I work on, I fixed some stuff in it when I faced issues. I updated my branch with the latest version so you can get that changes too.

I also added an uncrustify target in Makefile.am to just have to type make uncrustify to beautify the code.

Since the last time, indenting is done with tab, alignment is done with space (produced files were not consistent when alignment was done with tab), still assuming 4 spaces tab length. By the way, when using tab of indenting and space for alignment, most of the alignment issues does not appear even if people use it's own in-editor tab-length.

Uncrustify have a problem with that BSD rule: “Closing and opening braces go on the same line as the else” and adds extra spaces on opening braces when closing braces is on the same line of the opening one, so, to get a consistent appearance, the closing brace is on its own line before an else.

So, this profile follows BSD rule except for closing brace before else and except for indenting: it does not ident with "8-char tab for first level, then 4-space for sublevels", it's just 4-char tab indentation all the way down. Alignment is made with spaces since it's the safest options with ascii-art-like comments. For convenience, people still can align comments with tab then run uncrustify.

You can see the latest uncrustify-related commits here to see how it's done.

illwieckz commented 7 years ago

Hmm, I re-read your statement asking explicitely for } else { trailing braces (there), so I will re-enable the "remove new line after closing else brace" rule, and will look why uncrustify adds useless spaces before the next opening brace on this kind of line…

-nl_brace_else=add
+nl_brace_else=remove

That is the spoted uncrustify bug:

        }
-       else if(get_player_info && !status->have_player)
-       {
-           char buf[sizeof(A2S_PLAYER)-1+4] = A2S_PLAYER;
-           memcpy( buf + sizeof(A2S_PLAYER)-1, &status->challenge, 4 );
+       } else if (get_player_info && !status->have_player)    {
+           char buf[sizeof(A2S_PLAYER) - 1 + 4] = A2S_PLAYER;
+           memcpy(buf + sizeof(A2S_PLAYER) - 1, &status->challenge, 4);

Do you see the four spaces (instead of one) in:

have_player)    {

?

illwieckz commented 7 years ago

well, running uncrustify twice with the same profile does the job:

illwieckz commented 7 years ago

so, I fixed the uncrustify stuff (and uncrustify is run twice, that workaround is an extra commit that can be reverted when uncrustify will be fixed upstream).

So, the uncrustify profile on my branch follows the BSD recommendation except: