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

Added clang format #2465

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

AnotherFoxGuy
Copy link
Member

Closes #2463

@only-a-ptr let me know what you think of this

@AnotherFoxGuy AnotherFoxGuy changed the title ✨ Added clang format Added clang format Nov 9, 2019
Copy link
Member

@ohlidalp ohlidalp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I downloaded the .clang-format file alone, applied it to some sources in my VisualStudio and tried some other settings.

Overall, this is an improvement, but a lot of specific places would benefit from //clang-format off. I need to spend a while looking at the modified code.

{
}

Ogre::Camera * mainCamera;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This * placement is just silly - I'd go with "PointerAlignment: Left"

}

void MumbleIntegration::initMumble()
{
#ifdef _WIN32
#ifdef _WIN32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This IndentPPDirectives is neat but then, it has limits - this makes sense to indent because of #ifdef USE_MUMBLE on top, but it's too far away from there so it ends up being just confusing.

I tried putting // clang-format off/on around the #ifdef USE_MUMBLE but it doesn't do the trick - the comment only affects reformatting, not parsing the document.


#include <OgreScriptLoader.h>
#define SOUND_PLAY_ONCE(_ACTOR_, _TRIG_) SoundScriptManager::getSingleton().trigOnce((_ACTOR_), (_TRIG_))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd put //clang-format off around this section. It looks more readable the way it was.


void setEnabled(bool state);

void setCamera(Ogre::Vector3 position, Ogre::Vector3 direction, Ogre::Vector3 up, Ogre::Vector3 velocity);
void setLoadingBaseSounds(bool value) { loading_base = value; };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm quite fond of these short inline getters/setters, but it seems ClangFormat has no support for it - there's just BraceWrapping.AfterFunction.

bool nd_override_mass:1; //!< User defined attr; mass is user-specified rather than calculated (override the calculation)
bool nd_under_water:1; //!< State; GFX hint
bool nd_no_mouse_grab:1; //!< Attr; User-defined
bool nd_cab_node : 1; //!< Attr; This node is part of collision triangle
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go with BitFieldColonSpacing:After, but None would be fine, too.

m_autoselect = NEUTRAL;
if (m_cur_gear > 0) m_autoselect = DRIVE;
if (m_cur_gear < 0) m_autoselect = REAR;
if (m_cur_gear == 0) m_autoselect = NEUTRAL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this turned out nice

@@ -68,20 +68,17 @@ void CharacterFactory::update(float dt)
{
gEnv->player->update(dt);

for (auto& c : m_remote_characters)
for (auto &c : m_remote_characters)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, I'd go with PointerAlingment:Left

if (truck->ar_right_blink_on) gd.ShowLights |= DL_SIGNAL_R;
if (truck->ar_warn_blink_on) gd.ShowLights |= DL_SIGNAL_ANY;
if (truck->tc_mode) gd.ShowLights |= DL_TC;
if (truck->alb_mode) gd.ShowLights |= DL_ABS;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here, I'd like it column-indented:

if (truck->tc_mode)     { gd.ShowLights |= DL_TC; }
if (truck->alb_mode)    { gd.ShowLights |= DL_ABS; }

but then it'd be ruined by the HasStarterContact..IsRunning long condition, so maybe it's best this way.

exit
fi

find .. -regex '.*\.\(h\|cpp\)' -not -path '*/plugins/*' -exec clang-format -style=file -i {} \;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exclusion pattern needs extending, i.e. for DearIMGUI and utf8cpp sources (or should those go under plugins, too?)

public:
invalid_code_point(uint32_t cp) : cp(cp) {}
virtual const char* what() const throw() { return "Invalid code point"; }
uint32_t code_point() const {return cp;}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apparently I'm not the only one who likes inline function bodies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add clang-format config file
2 participants