Skip to content

[do not merge] Straw-man clang-format example.#616

Closed
rryan wants to merge 1 commit intomixxxdj:masterfrom
rryan:clang-format
Closed

[do not merge] Straw-man clang-format example.#616
rryan wants to merge 1 commit intomixxxdj:masterfrom
rryan:clang-format

Conversation

@rryan
Copy link
Copy Markdown
Member

@rryan rryan commented Jun 9, 2015

No description provided.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Aha, here's an example where it added a space on a code item. But I guess we acknowledged that possibility.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yea, this will happen to all commented code basically -- which isn't great but also isn't the worst thing. Maybe commented code should mostly be deleted...

Comment thread src/analyserbeats.cpp
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we mind how much more room this takes up? I can see the value of having it split out though, like here, it's now hard to miss the .toInt().
(What do the rules do if the ConfigKey() bit would have extended past the defined line length?)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can tweak the costs of each kind of break to try and avoid this. By default clang-format prefers breaking fewer parentheses so it kept ConfigKey together and dropped toInt to the next line instead of breaking ConfigKey and putting toInt at the end of the 2nd line.

Basically, if it's automatic I don't really mind it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given that choice, I agree with its decision because this is clearer than that would've been. (Though the indentation would have probably lined up better.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Eclipse does this:

    bool bPreferencesBeatDetectionEnabled =
            static_cast<bool>(m_pConfig->getValueString(
                    ConfigKey(BPM_CONFIG_KEY, BPM_DETECTION_ENABLED)).toInt());

It uses a simple double indent line brake rule.

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.

3 participants