search

pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
463 stars 133 forks source link

[DO NOT MERGE] Clang format styles #267

Open LecrisUT opened 12 months ago

LecrisUT commented 12 months ago

@pboettch I just want to get your opinion on different clang-format styles. Check json-schema.hpp in the commits for the output differences

Option A: 4bff849b250a66a03076f80d6d37dfd1e34e54b6

This is a minimal adaptation of the current format, just changing the namespace indentation to make it more readable.

Option B: e19283787ad94222a4cd7260cb6563067546a5ad

Complete overhaul based off of clion defaults. This is what I usually use, and it mostly resolves some nitpicks I have with bracing position, indicator for reference/pointer etc.

Do you have any preference or different opinions on these?

pboettch commented 12 months ago

No and no.

Option 1: I don't like indentation for namespaces. And I like indentation for pre-processor-directives (if they are complex)

Option 2: Unacceptable for me. There are so many things wrong in my eyes. Especially the & and * position in arguments and variable declarations. The & and * belongs to the variable not to the type:

Type* var1, var2;

What's the type of var2? How do you write it as a pointer? Type* var1, * var2 ?

But thanks for the proposal, thanks to clang-format, I hope it didn't take you too much time.

I'm however open for real improvement. E.g. pre-processor indents, why not scrap them, if it's doing too much non-sense. For the moment, I think it is OK.

LecrisUT commented 12 months ago

No problem, option B are my personal nitpicks where I believe you should define values and pointers separately anyway. But yeah those personal preferences so no problem having different opinions.

But let's discuss option A a bit more. The indentation of the preprocessor is kept the same, it mostly changes namespace indentation. In that file for example it was hard to navigate to where nlohmann::json_schema namespace began. That is not an issue with nlohmann/json because there only one namespace is used (excluding the macro defined one), but in this file both parent and child namespaces are used which creates the navigation problem.

Alternatively though if only one namespace is used (with aliases on the parent namespace if needed) that would also solve the readability issue.