Add clang-format style #72

Open
dimmus wants to merge 1 commits from dimmus/enlightenment:devs/dimmus/clang-format into master
First-time contributor

Have a try.
Use it for a long time for personal EFL coding.

Emulates existing coding style as close as possible. Rich comments provided.

ninja -C build clang-format

Have a try. Use it for a long time for personal EFL coding. Emulates existing coding style as close as possible. Rich comments provided. `ninja -C build clang-format`
dimmus added 1 commit 2024-06-04 08:14:27 -07:00
Author
First-time contributor

Tested with clang-format 16 and 17

Tested with clang-format 16 and 17
Owner

actually... that reminds me. a lot of people complained about the 3 space indent etc. as their editor cant do it so i've been playing with a slightly modified formatting in efm2 work.

but that aside .. at least here i don't have a clang-format target that meson generates? is this really really new?

actually... that reminds me. a lot of people complained about the 3 space indent etc. as their editor cant do it so i've been playing with a slightly modified formatting in efm2 work. but that aside .. at least here i don't have a clang-format target that meson generates? is this really really new?
Owner

aaah wait. i have to have .clang-format format when meson generates for it to exist!

aaah wait. i have to have .clang-format format when meson generates for it to exist!
Author
First-time contributor

Probably yes.
.clang-format - since 0.50
.clang-format-include and .clang-format-ignore - since 0.58

Probably yes. .clang-format - since 0.50 .clang-format-include and .clang-format-ignore - since 0.58
raster requested review from Owners 2024-06-05 10:12:45 -07:00
raster reviewed 2024-06-05 10:14:48 -07:00
@ -358,9 +358,11 @@ _e_smart_format_update(E_Smart_Data *sd)
if (sd->format)
{
char buf[256];
// clang-formating off
Owner

actually - it'd be better to reformat the code and not turn off formatting - just have it formatted so an auto-formatter is happy.

actually - it'd be better to reformat the code and not turn off formatting - just have it formatted so an auto-formatter is happy.
raster reviewed 2024-06-05 10:15:47 -07:00
@ -0,0 +1 @@
*.h
Owner

wait.. does this ignore ALL .h files? ... we shouldn't do that. we should just fix the formatting!

wait.. does this ignore ALL .h files? ... we shouldn't do that. we should just fix the formatting!
Author
First-time contributor

Yep.
Had a lot of problems with compilation when headers are turned on (but in EFL :).
Nevertheless, have a try.

Yep. Had a lot of problems with compilation when headers are turned on (but in EFL :). Nevertheless, have a try.
raster reviewed 2024-06-05 10:16:59 -07:00
raster left a comment
Owner

Small things - like .h files being ignored...

when it comes to formatting I think that's a bigger discussion... have a look at efm2 and at the .clang-format there and try that. yes - it'll reformat e code. but it might be easier to edit that kind of formatting?

Small things - like .h files being ignored... when it comes to formatting I think that's a bigger discussion... have a look at efm2 and at the .clang-format there and try that. yes - it'll reformat e code. but it might be easier to edit that kind of formatting?
Author
First-time contributor

Ok

Ok
Author
First-time contributor

So, tried yours:
original:

if (!strncmp(v->mount_point, e_user_dir_get(), strlen(e_user_dir_get())))
     if (unlink(v->mount_point))
       printf("Error unlinking mount point!\n");

became

if (!strncmp(v->mount_point, e_user_dir_get(), strlen(e_user_dir_get())))
    if (unlink(v->mount_point)) printf("Error unlinking mount point!\n");

May be AllowShortBlocksOnASingleLine: Never should be the common standard.

I agree that for short statements single line is better. We can play with penalty option but with strange behaviour in another places.

So, tried yours: original: ``` if (!strncmp(v->mount_point, e_user_dir_get(), strlen(e_user_dir_get()))) if (unlink(v->mount_point)) printf("Error unlinking mount point!\n"); ``` became ``` if (!strncmp(v->mount_point, e_user_dir_get(), strlen(e_user_dir_get()))) if (unlink(v->mount_point)) printf("Error unlinking mount point!\n"); ``` May be `AllowShortBlocksOnASingleLine: Never` should be the common standard. I agree that for short statements single line is better. We can play with `penalty` option but with strange behaviour in another places.
Author
First-time contributor

Original

#ifndef E_SYSTEM_H
# define E_SYSTEM_H 1
# include "config.h"

# ifndef _FILE_OFFSET_BITS
#  define _FILE_OFFSET_BITS 64
# endif

# ifdef STDC_HEADERS
#  include <stdlib.h>
#  include <stddef.h>
# else
...

Formated

#ifndef E_SYSTEM_H
#define E_SYSTEM_H 1
#include "config.h"

#ifndef _FILE_OFFSET_BITS
#define _FILE_OFFSET_BITS 64
#endif

#ifdef STDC_HEADERS
#include <stdlib.h>
#include <stddef.h>
#else
...

To my opinion with spaces is better ;)

Original ``` #ifndef E_SYSTEM_H # define E_SYSTEM_H 1 # include "config.h" # ifndef _FILE_OFFSET_BITS # define _FILE_OFFSET_BITS 64 # endif # ifdef STDC_HEADERS # include <stdlib.h> # include <stddef.h> # else ... ``` Formated ``` #ifndef E_SYSTEM_H #define E_SYSTEM_H 1 #include "config.h" #ifndef _FILE_OFFSET_BITS #define _FILE_OFFSET_BITS 64 #endif #ifdef STDC_HEADERS #include <stdlib.h> #include <stddef.h> #else ... ``` To my opinion with spaces is better ;)
Author
First-time contributor

Do you like

if (head.size < 0)
  {
  }
else
  {
  }

instead of

if (head.size < 0)
{
}
else
{
}

?

Personally I like the last one.

Do you like ``` if (head.size < 0) { } else { } ``` instead of ``` if (head.size < 0) { } else { } ``` ? Personally I like the last one.
Author
First-time contributor

Sorry if it looks like a 'taste' discussion, i'm only wondering :)
As you sad earlier "... that's a bigger discussion."

Sorry if it looks like a 'taste' discussion, i'm only wondering :) As you sad earlier "... that's a bigger discussion."
Owner

So, tried yours:
original:

if (!strncmp(v->mount_point, e_user_dir_get(), strlen(e_user_dir_get())))
     if (unlink(v->mount_point))
       printf("Error unlinking mount point!\n");

became

if (!strncmp(v->mount_point, e_user_dir_get(), strlen(e_user_dir_get())))
    if (unlink(v->mount_point)) printf("Error unlinking mount point!\n");

May be AllowShortBlocksOnASingleLine: Never should be the common standard.

I agree that for short statements single line is better. We can play with penalty option but with strange behaviour in another places.

Hmmmn.. but on the other hand things like

if (!strcmp(str, "hello")) return;

is so much nicer as a single line. so short simple if's actually do look nice single-lined. it's compact and easy to read. the above example actually is REALLY poor. imho that should have a {} around the if (unlink).... bit due to it being now less "easy" so read without carefully keeping indents right. so formatting has pointed out imho poorly arranged code to begin with. :)

> So, tried yours: > original: > ``` > if (!strncmp(v->mount_point, e_user_dir_get(), strlen(e_user_dir_get()))) > if (unlink(v->mount_point)) > printf("Error unlinking mount point!\n"); > ``` > became > ``` > if (!strncmp(v->mount_point, e_user_dir_get(), strlen(e_user_dir_get()))) > if (unlink(v->mount_point)) printf("Error unlinking mount point!\n"); > ``` > > May be `AllowShortBlocksOnASingleLine: Never` should be the common standard. > > I agree that for short statements single line is better. We can play with `penalty` option but with strange behaviour in another places. > Hmmmn.. but on the other hand things like if (!strcmp(str, "hello")) return; is so much nicer as a single line. so short simple if's actually do look nice single-lined. it's compact and easy to read. the above example actually is REALLY poor. imho that should have a {} around the if (unlink).... bit due to it being now less "easy" so read without carefully keeping indents right. so formatting has pointed out imho poorly arranged code to begin with. :)
Owner

Original

#ifndef E_SYSTEM_H
# define E_SYSTEM_H 1
# include "config.h"

# ifndef _FILE_OFFSET_BITS
#  define _FILE_OFFSET_BITS 64
# endif

# ifdef STDC_HEADERS
#  include <stdlib.h>
#  include <stddef.h>
# else
...

Formated

#ifndef E_SYSTEM_H
#define E_SYSTEM_H 1
#include "config.h"

#ifndef _FILE_OFFSET_BITS
#define _FILE_OFFSET_BITS 64
#endif

#ifdef STDC_HEADERS
#include <stdlib.h>
#include <stddef.h>
#else
...

To my opinion with spaces is better ;)

I agree - I didn't seem to find out how to tell clang format to do this yet... :) but it should.

> Original > ``` > #ifndef E_SYSTEM_H > # define E_SYSTEM_H 1 > # include "config.h" > > # ifndef _FILE_OFFSET_BITS > # define _FILE_OFFSET_BITS 64 > # endif > > # ifdef STDC_HEADERS > # include <stdlib.h> > # include <stddef.h> > # else > ... > ``` > Formated > ``` > #ifndef E_SYSTEM_H > #define E_SYSTEM_H 1 > #include "config.h" > > #ifndef _FILE_OFFSET_BITS > #define _FILE_OFFSET_BITS 64 > #endif > > #ifdef STDC_HEADERS > #include <stdlib.h> > #include <stddef.h> > #else > ... > ``` > > To my opinion with spaces is better ;) I agree - I didn't seem to find out how to tell clang format to do this yet... :) but it should.
Owner

Do you like

if (head.size < 0)
  {
  }
else
  {
  }

instead of

if (head.size < 0)
{
}
else
{
}

?

Personally I like the last one.

i do perfer the indented one. :)

> Do you like > ``` > if (head.size < 0) > { > } > else > { > } > ``` > instead of > ``` > if (head.size < 0) > { > } > else > { > } > ``` > ? > > Personally I like the last one. i do perfer the indented one. :)
Owner

Sorry if it looks like a 'taste' discussion, i'm only wondering :)
As you sad earlier "... that's a bigger discussion."

there is a lot of taste in formatting, but if we're going to use auto-formatters, it's better to massage one that doesn't need exceptions like excluding.h files and other "disable it here" blocks. so keep it universal and automated, but then make sure the auto formatting works well. like the # spacing + indenting... it seems IndentPPDirectives is the way to go... :)

> Sorry if it looks like a 'taste' discussion, i'm only wondering :) > As you sad earlier "... that's a bigger discussion." there is a lot of taste in formatting, but if we're going to use auto-formatters, it's better to massage one that doesn't need exceptions like excluding.h files and other "disable it here" blocks. so keep it universal and automated, but then make sure the auto formatting works well. like the # spacing + indenting... it seems IndentPPDirectives is the way to go... :)
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b dimmus-devs/dimmus/clang-format master
git pull devs/dimmus/clang-format

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff dimmus-devs/dimmus/clang-format
git push origin master
Sign in to join this conversation.
No reviewers
enlightenment/Owners
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: enlightenment/enlightenment#72
No description provided.