-
Notifications
You must be signed in to change notification settings - Fork 95
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
Code Style #24
Comments
Thanks so much for caring about this. The reason for some of the inconsistencies is due to “eras” so to speak, e.g. the availability system was overhauled only a few years ago and this is when “Proper” code style issues like
I’ve erred on not changing code style of 3rd-party headers such as the libpackageinfo changes seen in the linked PR, because we want to easily keep them in sync when they change. Changing code style would create merge conflicts, possibly causing bits and pieces of upstream changes to be lost by accident in cleaning up the conflicts. So I’m preferring timeliness on merging updated sets of headers over code style there. I think we could make an exception for Flipswitch here to fix its lack of C++ compatibility. Ideally should send that upstream as well so we’re only patching it this once. GraphicsServices and a few others came from earlier collections of headers such as KennyTM’s, and I wasn’t so sure whether it’s worth cleaning them up or not. I think it is worth it since we know now that the upstream repos are unlikely to be updated again - this repo is probably the canonical place to get these headers from now. So we may as well bring them to our standards. If you’re up for cleaning up code style based on what I’ve discussed here then that would be a big help 🙂 (p.s. @PoomSmart, sorry I completely forgot to review your pull request!) |
Should we set up clang-format so we don’t have to worry about manually modifying headers? I know that brings up the discussion of the specifics of the style we pick but it’d save us from the work of applying it |
I’m supportive of a My recommendation would be to use the WebKit (or another) default, and modify the rules as needed to match the current style |
Per
README
:There are many different styles throughout the headers, even ignoring vendor headers (e.g.
openssl
).I believe there should be a more unified, and explicit style.
Examples of different existing styles
Availability attribution:
NS_AVAILABLE_IOS
, andNS_DEPRECATED_IOS
API_AVAILABLE
(I don't see instances of the equivalentAPI_DEPRECATED
in this repo)External declarations:
__BEGIN_DECLS
#if __cplusplus \\ extern "C" { \\ #endif
idiom (mostly equivalent to__BEGIN_DECLS
)extern
, without any C++ linkage indicator.OBJC_EXTERN
andFOUNDATION_EXTERN
Property attributes (
ObjCSpaceAfterProperty
):@property (attributes) type;
@property(attributes) type
Style discussions are sometimes criticized, because developers have different opinions on how code should be formatted, however I mention these three because I've seen issues arise in due to these in particular.
Availability attributions are important, especially for projects such as this one, where private system APIs are sometimes documented. Deciding on an attribution system allows developers to contribute more easily and effectively.
External declarations must be accurate when using C++, to avoid linking errors. Theos used to create ObjC++ source files by default. A unified
extern
marking system would avoid issues such asFSSwitchState.h
missing a C++ linker hint.Property attributes are very strongly a style choice, but I mention it due to #21 changing these property declarations. It could be bothersome, if someone felt compelled to change them back, or if new changes are made with no property trailing space, and someone else goes to change those.
The text was updated successfully, but these errors were encountered: