Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn the style guide into a clang-format file #784

Merged
merged 3 commits into from
Nov 30, 2017

Conversation

LemonBoy
Copy link
Member

Note that his has been written with clang-format 3.8 in mind since that's what Debian ships, see here for the docs.

I'm not too fond of the tabs + 100 columns combo since my laptop has a tiny screen, but that's not much of a problem.

So, I hereby declare the style-related bikeshed season open!

@ailin-nemui
Copy link
Contributor

I tried clang-format on all files but it does the wrong thing in several cases, for example odd column style initialisation of arrays and structs, enum {\n wrapping cannot be configured and it misdetects some casts like (SIGNAL_FUNC) (func) (removes space)

@ailin-nemui
Copy link
Contributor

I suggest we merge this, but do not auto-run clang-format. It does screw up several structures like struct initializers and enum lists. We could selectively run it and revert those erroneous changes, and ponder whether we want to litter the code with /* clang-format off */ at those places

@LemonBoy
Copy link
Member Author

It does screw up several structures like struct initializers and enum lists. We could selectively run it and revert those erroneous changes

We should double-check if those problems go away using a newer clang-format or more options

do not indent case deeper
@ailin-nemui
Copy link
Contributor

ailin-nemui commented Nov 16, 2017

outstanding issues and changes that would happen:

- == original irssi
+ == clang-format

a. spaces around boolean fields:

-unsigned int chanop:1;
+unsigned int chanop : 1;

b. collapsation of #define functions:

-#define alias_runstack_push(alias) \
-       alias_runstack = g_slist_append(alias_runstack, alias)
+#define alias_runstack_push(alias) alias_runstack = g_slist_append(alias_runstack, alias)

c. odd folding of enum types:

-enum {
-       CMDERR_OPTION_UNKNOWN = -3,
+enum { CMDERR_OPTION_UNKNOWN = -3,
c.2. alternative here could be to use break on enum, which would look like this:
-    enum {
-           CMDERR_OPTION_UNKNOWN = -3,
+    enum
+    {
+          CMDERR_OPTION_UNKNOWN = -3,

(I would prefer the original style...)

d. unindent of preprocessor codes:

 #ifdef HAVE_SYS_RESOURCE_H
-#  include <sys/resource.h>
+#include <sys/resource.h>
 #endif

e. odd initializer wrapping:

-       static GOptionEntry options[] = {
-               { "config", 0, 0, G_OPTION_ARG_STRING, &irssi_config_file, "Configuration file location (~/.irssi/config)", "PATH" },
-               { "home", 0, 0, G_OPTION_ARG_STRING, &irssi_dir, "Irssi home dir location (~/.irssi)", "PATH" },
-               { NULL }
-       };
+       static GOptionEntry options[] = { { "config", 0, 0, G_OPTION_ARG_STRING, &irssi_config_file,
+                                           "Configuration file location (~/.irssi/config)", "PATH" },
+                                         { "home", 0, 0, G_OPTION_ARG_STRING, &irssi_dir,
+                                           "Irssi home dir location (~/.irssi)", "PATH" },
+                                         { NULL } };

f. non-helpful column wrapping of initializers:

-static const char *levels[] = {
-       "CRAP",
...
-       "HILIGHTS",
-       "NOHILIGHT",
-       "NO_ACT",
-       NULL
-};
+static const char *levels[] = { "CRAP",         "MSGS",     "PUBLICS", "NOTICES",       "SNOTES",
+                               "CTCPS",        "ACTIONS",  "JOINS",   "PARTS",         "QUITS",
+                               "KICKS",        "MODES",    "TOPICS",  "WALLOPS",       "INVITES",
+                               "NICKS",        "DCC",      "DCCMSGS", "CLIENTNOTICES", "CLIENTCRAP",
+                               "CLIENTERRORS", "HILIGHTS",
+                               "NOHILIGHT",    "NO_ACT",   NULL };

g. non-helpful un-wrapping of initializers:

-int mirc_colors[] = { 15, 0, 1, 2, 12, 4, 5, 6, 14, 10, 3, 11, 9, 13, 8, 7,
-        /* 16-27 */  52,  94, 100,  58,  22,  29,  23,  24,  17,  54,  53,  89,
...
+int mirc_colors[] = { 15,
+                     0,
+                     1,
+                     2,
...
+                     /* 16-27 */ 52,
+                     94,
+                     100,
...

h. non-helpful re-wrapping of initializers:

 int term_color256map[] = {
-        0, 4, 2, 6, 1, 5, 3, 7, 8,12,10,14, 9,13,11,15,
-        0, 0, 1, 1, 1, 1, 0, 0, 3, 1, 1, 9, 2, 2, 3, 3, 3, 3,
...
+       0, 4, 2, 6,  1,  5,  3, 7, 8,  12, 10, 14, 9,  13, 11, 15, 0, 0, 1,  1,  1,  1,  0, 0,  3,  1,
+       1, 9, 2, 2,  3,  3,  3, 3, 2,  2,  3,  3,  3,  3,  2,  2,  3, 3, 3,  11, 10, 10, 3, 3,  11, 11,
...

j. its own opinion about empty for loop parts:

-       for (ptr = str; ; str++) {
+       for (ptr = str;; str++) {

altogether, committing the clang-format changes would touch 14000 lines in 285 files, of which are 5000 lines of incorrect tab/space indentation fixes

@ailin-nemui ailin-nemui added the auto-merge This PR is scheduled for merge if no further comments are opened label Nov 27, 2017
@ailin-nemui
Copy link
Contributor

@irssi/developers

@ailin-nemui ailin-nemui merged commit 117f666 into irssi:master Nov 30, 2017
@ailin-nemui ailin-nemui added this to the 1.1.0 milestone Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge This PR is scheduled for merge if no further comments are opened
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants