Pass clang-format on various cpp/header files #5559

nerzhul · 2017-04-10T19:52:10Z

I ran clang-format on all my header files and choose an interesting compliant subset. Please review and we merge, those headers are not heavily modified in PR.

nerzhul · 2017-04-10T19:54:06Z

Please tell me if some files are not good and why, and i will remove them from the changeset permitting others to merge

nerzhul · 2017-04-10T20:07:42Z

I passed it on various cpp trivial files to fix too. This permits to cover ~12% of our files with clang-format LINT check

SmallJoker · 2017-04-10T20:18:17Z

src/chat.h

- name(a_name),
- text(a_text)
+ ChatLine(EnrichedString a_name, EnrichedString a_text)
+ : age(0.0), name(a_name), text(a_text)


Spaces? Nope.

SmallJoker · 2017-04-10T20:18:51Z

src/cavegen.h

+ CavesRandomWalk(INodeDefManager *ndef, GenerateNotifier *gennotify = NULL,
+ s32 seed = 0, int water_level = 1,
+ content_t water_source = CONTENT_IGNORE,
+ content_t lava_source = CONTENT_IGNORE);


1-tab is fine according to our code style guidelines.

2-tab too?
The problem here is that we have to tell the formatter one of them...

SmallJoker · 2017-04-10T20:22:02Z

The other code looks good. Feels like this is going to be a long way until everything's styled correctly.

nerzhul · 2017-04-10T20:29:59Z

@SmallJoker yeah i ran clang-format on all files, 60k modifications if i remember. I remove chat.h from this changeset

nerzhul · 2017-04-10T20:40:37Z

i squashed the commits to have a full build with the current changeset

nerzhul · 2017-04-10T21:08:47Z

@SmallJoker i rebased & squashed, please tell me if it's okay for you, i will merge when build was successful

SmallJoker · 2017-04-10T21:47:21Z

src/httpfetch.h

-void httpfetch_sync(const HTTPFetchRequest &fetch_request,
- HTTPFetchResult &fetch_result);
-
+void httpfetch_sync(const HTTPFetchRequest &fetch_request, HTTPFetchResult &fetch_result);


Length: 91, over the limits.

strange to see how clang-format count, because it is limited to 90 chars

using python interpreter it's good

>>> len("void httpfetch_sync(const HTTPFetchRequest &fetch_request, HTTPFetchResult &fetch_result);") 90

90 is the absolute maximum. Whatever, looks good.

nerzhul · 2017-04-11T05:55:00Z

@sfan5 can you look at this please ?

Note: with this commit 83 files are checked by clang-format over a total of 465 => ~17%

sfan5 · 2017-04-12T10:59:51Z

src/drawscene.h

+void draw_scene(video::IVideoDriver *driver, scene::ISceneManager *smgr, Camera &camera,
+ Client &client, LocalPlayer *player, Hud &hud, Minimap &mapper,
+ gui::IGUIEnvironment *guienv, const v2u32 &screensize,
+ const video::SColor &skycolor, bool show_hud, bool show_minimap);


while it may be useful to compact this, i would prefer if it didn't do this...

both are compact, one respect 80 chars other 90 chars are you sure you want me to revert this change ?

sfan5 · 2017-04-12T11:00:25Z

src/event_manager.h

 {
 static void receiverReceive(MtEvent *e, void *data)
 {
- MtEventReceiver *r = (MtEventReceiver*)data;
+ MtEventReceiver *r = (MtEventReceiver *)data;


why did it add a space here?

because the rule for pointers is to have * at the right, then casts are affected

sfan5 · 2017-04-12T11:01:31Z

src/keycode.h

@@ -34,16 +34,16 @@ class KeyPress
 KeyPress();
 KeyPress(const char *name);

- KeyPress(const irr::SEvent::SKeyInput &in, bool prefer_character=false);
+ KeyPress(const irr::SEvent::SKeyInput &in, bool prefer_character = false);


is this part of our guidelines?

no, it's a choice of LLVM to have readable code when using equal, and i'm not afraid, i prefer having it it permit to view more easily default values

sfan5 · 2017-04-12T11:02:39Z

src/unittest/test_nodedef.cpp

@@ -55,7 +56,7 @@ void TestNodeDef::testContentFeaturesSerialization()

 std::ostringstream os(std::ios::binary);
 f.serialize(os, LATEST_PROTOCOL_VERSION);
- //verbosestream<<"Test ContentFeatures size: "<<os.str().size()<<std::endl;
+ // verbosestream<<"Test ContentFeatures size: "<<os.str().size()<<std::endl;


I understand why its doing this but this is quite stupid nonetheless

it's a tool, it's not an IA, the change it just for respect the coding style

nerzhul · 2017-04-13T07:34:51Z

@sfan5 waiting for your update :)

nerzhul · 2017-04-22T06:50:59Z

rebased and i removed some files from PR as they are not very convenient, i think i will merge it tomorrow

nerzhul added the Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements label Apr 10, 2017

nerzhul requested review from sfan5 and SmallJoker April 10, 2017 19:52

nerzhul force-pushed the clang_format_header_4 branch from 887773f to 21d135f Compare April 10, 2017 20:00

SmallJoker reviewed Apr 10, 2017

View reviewed changes

nerzhul force-pushed the clang_format_header_4 branch from ac79c34 to e35fdaa Compare April 10, 2017 20:37

nerzhul changed the title ~~Pass clang-format on various header files~~ Pass clang-format on various cpp/header files Apr 10, 2017

nerzhul closed this Apr 10, 2017

nerzhul reopened this Apr 10, 2017

SmallJoker reviewed Apr 10, 2017

View reviewed changes

nerzhul added this to the 0.4.16 milestone Apr 11, 2017

sfan5 reviewed Apr 12, 2017

View reviewed changes

Pass clang-format on various cpp/header files

8f555d3

nerzhul force-pushed the clang_format_header_4 branch from e35fdaa to 8f555d3 Compare April 22, 2017 06:48

nerzhul merged commit 91a9382 into minetest:master Apr 23, 2017

nerzhul added this to Done in Minetest 0.4.16 Apr 29, 2017

nerzhul deleted the clang_format_header_4 branch January 6, 2018 10:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass clang-format on various cpp/header files #5559

Pass clang-format on various cpp/header files #5559

nerzhul commented Apr 10, 2017

nerzhul commented Apr 10, 2017

nerzhul commented Apr 10, 2017 •

edited

Loading

SmallJoker Apr 10, 2017

SmallJoker Apr 10, 2017

sfan5 Apr 12, 2017

SmallJoker commented Apr 10, 2017

nerzhul commented Apr 10, 2017

nerzhul commented Apr 10, 2017

nerzhul commented Apr 10, 2017

SmallJoker Apr 10, 2017

nerzhul Apr 10, 2017 •

edited

Loading

nerzhul Apr 10, 2017

SmallJoker Apr 10, 2017

nerzhul commented Apr 11, 2017 •

edited

Loading

sfan5 Apr 12, 2017

nerzhul Apr 12, 2017

sfan5 Apr 12, 2017

nerzhul Apr 12, 2017

sfan5 Apr 12, 2017

nerzhul Apr 12, 2017

sfan5 Apr 12, 2017

nerzhul Apr 12, 2017

nerzhul commented Apr 13, 2017

nerzhul commented Apr 22, 2017

Pass clang-format on various cpp/header files #5559

Pass clang-format on various cpp/header files #5559

Conversation

nerzhul commented Apr 10, 2017

nerzhul commented Apr 10, 2017

nerzhul commented Apr 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SmallJoker commented Apr 10, 2017

nerzhul commented Apr 10, 2017

nerzhul commented Apr 10, 2017

nerzhul commented Apr 10, 2017

Choose a reason for hiding this comment

nerzhul Apr 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerzhul commented Apr 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerzhul commented Apr 13, 2017

nerzhul commented Apr 22, 2017

nerzhul commented Apr 10, 2017 •

edited

Loading

nerzhul Apr 10, 2017 •

edited

Loading

nerzhul commented Apr 11, 2017 •

edited

Loading