Skip to content

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

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.

Sign up for GitHub

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

Closed
rryan wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
18 changes: 18 additions & 0 deletions .clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
Language: Cpp
Standard: Cpp11
BasedOnStyle: Google
IndentWidth: 4
ContinuationIndentWidth: 8
TabWidth: 4
AccessModifierOffset: -2
ConstructorInitializerIndentWidth: 8
ConstructorInitializerAllOnOneLineOrOnePerLine: true
AlignTrailingComments: false
AllowShortFunctionsOnASingleLine: false
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false
AllowShortBlocksOnASingleLine: false
AllowShortCaseLabelsOnASingleLine: false
ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH ]
...
9 changes: 5 additions & 4 deletions src/analyser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@
#include "trackinfoobject.h"

class Analyser {

public:
virtual bool initialise(TrackPointer tio, int sampleRate, int totalSamples) = 0;
public:
virtual bool initialise(TrackPointer tio, int sampleRate,
int totalSamples) = 0;
Copy link
Copy Markdown
Member

@daschuer daschuer Jun 9, 2015

Choose a reason for hiding this comment

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

These type of changes do not help for readability. Is there a way to specify a soft 80 columns limit?
The hard limit should be at 100 or so. Github allows 115.

Copy link
Copy Markdown
Member Author

@rryan rryan Jun 9, 2015

Choose a reason for hiding this comment

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

that it's on two lines now or aligning hanging indents with the opening paren?

virtual bool loadStored(TrackPointer tio) const = 0;
virtual void process(const CSAMPLE* pIn, const int iLen) = 0;
virtual void cleanup(TrackPointer tio) = 0;
virtual void finalise(TrackPointer tio) = 0;
virtual ~Analyser() {}
virtual ~Analyser() {
}
};

#endif
105 changes: 64 additions & 41 deletions src/analyserbeats.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,16 @@ AnalyserBeats::AnalyserBeats(ConfigObject<ConfigValue>* pConfig)
AnalyserBeats::~AnalyserBeats() {
}

bool AnalyserBeats::initialise(TrackPointer tio, int sampleRate, int totalSamples) {
bool AnalyserBeats::initialise(TrackPointer tio, int sampleRate,
int totalSamples) {
if (totalSamples == 0) {
return false;
}

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

@Pegasus-RPG Pegasus-RPG Jun 9, 2015

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

@rryan rryan Jun 9, 2015

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

@Pegasus-RPG Pegasus-RPG Jun 9, 2015

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

@daschuer daschuer Jun 9, 2015

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.

if (!bPreferencesBeatDetectionEnabled) {
qDebug() << "Beat calculation is deactivated";
return false;
Expand All @@ -52,33 +54,45 @@ bool AnalyserBeats::initialise(TrackPointer tio, int sampleRate, int totalSample
return false;
}

bool allow_above = static_cast<bool>(m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_ABOVE_RANGE_ENABLED)).toInt());
bool allow_above = static_cast<bool>(
m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_ABOVE_RANGE_ENABLED))
.toInt());
if (allow_above) {
m_iMinBpm = 0;
m_iMaxBpm = 9999;
} else {
m_iMinBpm = m_pConfig->getValueString(ConfigKey(BPM_CONFIG_KEY, BPM_RANGE_START)).toInt();
m_iMaxBpm = m_pConfig->getValueString(ConfigKey(BPM_CONFIG_KEY, BPM_RANGE_END)).toInt();
m_iMinBpm = m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_RANGE_START))
.toInt();
m_iMaxBpm = m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_RANGE_END))
.toInt();
}

m_bPreferencesFixedTempo = static_cast<bool>(
m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_FIXED_TEMPO_ASSUMPTION)).toInt());
m_pConfig->getValueString(ConfigKey(BPM_CONFIG_KEY,
BPM_FIXED_TEMPO_ASSUMPTION))
.toInt());
m_bPreferencesOffsetCorrection = static_cast<bool>(
m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_FIXED_TEMPO_OFFSET_CORRECTION)).toInt());
m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY,
BPM_FIXED_TEMPO_OFFSET_CORRECTION))
.toInt());
m_bPreferencesReanalyzeOldBpm = static_cast<bool>(
m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_REANALYZE_WHEN_SETTINGS_CHANGE)).toInt());
m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY,
BPM_REANALYZE_WHEN_SETTINGS_CHANGE))
.toInt());
m_bPreferencesFastAnalysis = static_cast<bool>(
m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_FAST_ANALYSIS_ENABLED)).toInt());
m_pConfig->getValueString(ConfigKey(BPM_CONFIG_KEY,
BPM_FAST_ANALYSIS_ENABLED))
.toInt());

QString library = m_pConfig->getValueString(
ConfigKey(VAMP_CONFIG_KEY, VAMP_ANALYSER_BEAT_LIBRARY));
ConfigKey(VAMP_CONFIG_KEY, VAMP_ANALYSER_BEAT_LIBRARY));
QString pluginID = m_pConfig->getValueString(
ConfigKey(VAMP_CONFIG_KEY, VAMP_ANALYSER_BEAT_PLUGIN_ID));
ConfigKey(VAMP_CONFIG_KEY, VAMP_ANALYSER_BEAT_PLUGIN_ID));

m_pluginId = pluginID;
m_iSampleRate = sampleRate;
Expand All @@ -89,8 +103,9 @@ bool AnalyserBeats::initialise(TrackPointer tio, int sampleRate, int totalSample

if (bShouldAnalyze) {
m_pVamp = new VampAnalyser();
bShouldAnalyze = m_pVamp->Init(library, pluginID, m_iSampleRate, totalSamples,
m_bPreferencesFastAnalysis);
bShouldAnalyze =
m_pVamp->Init(library, pluginID, m_iSampleRate, totalSamples,
m_bPreferencesFastAnalysis);
if (!bShouldAnalyze) {
delete m_pVamp;
m_pVamp = NULL;
Expand All @@ -110,14 +125,20 @@ bool AnalyserBeats::loadStored(TrackPointer tio) const {
int iMinBpm;
int iMaxBpm;

bool allow_above = static_cast<bool>(m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_ABOVE_RANGE_ENABLED)).toInt());
bool allow_above = static_cast<bool>(
m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_ABOVE_RANGE_ENABLED))
.toInt());
if (allow_above) {
iMinBpm = 0;
iMaxBpm = 9999;
} else {
iMinBpm = m_pConfig->getValueString(ConfigKey(BPM_CONFIG_KEY, BPM_RANGE_START)).toInt();
iMaxBpm = m_pConfig->getValueString(ConfigKey(BPM_CONFIG_KEY, BPM_RANGE_END)).toInt();
iMinBpm = m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_RANGE_START))
.toInt();
iMaxBpm = m_pConfig->getValueString(
ConfigKey(BPM_CONFIG_KEY, BPM_RANGE_END))
.toInt();
}

bool bpmLock = tio->hasBpmLock();
Expand All @@ -127,9 +148,9 @@ bool AnalyserBeats::loadStored(TrackPointer tio) const {
}

QString library = m_pConfig->getValueString(
ConfigKey(VAMP_CONFIG_KEY, VAMP_ANALYSER_BEAT_LIBRARY));
ConfigKey(VAMP_CONFIG_KEY, VAMP_ANALYSER_BEAT_LIBRARY));
QString pluginID = m_pConfig->getValueString(
ConfigKey(VAMP_CONFIG_KEY, VAMP_ANALYSER_BEAT_PLUGIN_ID));
ConfigKey(VAMP_CONFIG_KEY, VAMP_ANALYSER_BEAT_PLUGIN_ID));

// At first start config for QM and Vamp does not exist --> set default
// TODO(XXX): This is no longer present in initialise. Remove?
Expand All @@ -145,13 +166,13 @@ bool AnalyserBeats::loadStored(TrackPointer tio) const {
QString version = pBeats->getVersion();
QString subVersion = pBeats->getSubVersion();

QHash<QString, QString> extraVersionInfo = getExtraVersionInfo(
pluginID, m_bPreferencesFastAnalysis);
QHash<QString, QString> extraVersionInfo =
getExtraVersionInfo(pluginID, m_bPreferencesFastAnalysis);
QString newVersion = BeatFactory::getPreferredVersion(
m_bPreferencesOffsetCorrection);
m_bPreferencesOffsetCorrection);
QString newSubVersion = BeatFactory::getPreferredSubVersion(
m_bPreferencesFixedTempo, m_bPreferencesOffsetCorrection,
iMinBpm, iMaxBpm, extraVersionInfo);
m_bPreferencesFixedTempo, m_bPreferencesOffsetCorrection,
iMinBpm, iMaxBpm, extraVersionInfo);

if (version == newVersion && subVersion == newSubVersion) {
// If the version and settings have not changed then if the world is
Expand All @@ -160,10 +181,12 @@ bool AnalyserBeats::loadStored(TrackPointer tio) const {
} else if (m_bPreferencesReanalyzeOldBpm) {
return false;
} else if (pBeats->getBpm() == 0.0) {
qDebug() << "BPM is 0 for track so re-analyzing despite preference settings.";
qDebug() << "BPM is 0 for track so re-analyzing despite preference "
"settings.";
return false;
} else if (pBeats->findNextBeat(0) <= 0.0) {
qDebug() << "First beat is 0 for grid so analyzing track to find first beat.";
qDebug() << "First beat is 0 for grid so analyzing track to find "
"first beat.";
return false;
} else {
qDebug() << "Beat calculation skips analyzing because the track has"
Expand All @@ -177,7 +200,7 @@ bool AnalyserBeats::loadStored(TrackPointer tio) const {
}
}

void AnalyserBeats::process(const CSAMPLE *pIn, const int iLen) {
void AnalyserBeats::process(const CSAMPLE* pIn, const int iLen) {
Copy link
Copy Markdown
Member

@Pegasus-RPG Pegasus-RPG Jun 9, 2015

Choose a reason for hiding this comment

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

With these types of changes, I thought putting the * on the type helps us know that the variable is a pointer-to-a-thing instead of a thing.

Copy link
Copy Markdown
Member Author

@rryan rryan Jun 9, 2015

Choose a reason for hiding this comment

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

Yep -- that's what it did here though? (unless I'm misunderstanding you)

Copy link
Copy Markdown
Member

@Pegasus-RPG Pegasus-RPG Jun 9, 2015

Choose a reason for hiding this comment

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

Oh, so it did. I must be getting dizzy. Sorry.

if (m_pVamp == NULL)
return;
bool success = m_pVamp->Process(pIn, iLen);
Expand Down Expand Up @@ -212,13 +235,12 @@ void AnalyserBeats::finalise(TrackPointer tio) {
return;
}

QHash<QString, QString> extraVersionInfo = getExtraVersionInfo(
m_pluginId, m_bPreferencesFastAnalysis);
QHash<QString, QString> extraVersionInfo =
getExtraVersionInfo(m_pluginId, m_bPreferencesFastAnalysis);
BeatsPointer pBeats = BeatFactory::makePreferredBeats(
tio, beats, extraVersionInfo,
m_bPreferencesFixedTempo, m_bPreferencesOffsetCorrection,
m_iSampleRate, m_iTotalSamples,
m_iMinBpm, m_iMaxBpm);
tio, beats, extraVersionInfo, m_bPreferencesFixedTempo,
m_bPreferencesOffsetCorrection, m_iSampleRate, m_iTotalSamples,
m_iMinBpm, m_iMaxBpm);

BeatsPointer pCurrentBeats = tio->getBeats();

Expand All @@ -232,7 +254,8 @@ void AnalyserBeats::finalise(TrackPointer tio) {
// If the track received the beat lock while we were analyzing it then we
// abort setting it.
if (tio->hasBpmLock()) {
qDebug() << "Track was BPM-locked as we were analysing it. Aborting analysis.";
qDebug() << "Track was BPM-locked as we were analysing it. Aborting "
"analysis.";
return;
}

Expand All @@ -259,7 +282,7 @@ void AnalyserBeats::finalise(TrackPointer tio) {

// static
QHash<QString, QString> AnalyserBeats::getExtraVersionInfo(
QString pluginId, bool bPreferencesFastAnalysis) {
QString pluginId, bool bPreferencesFastAnalysis) {
QHash<QString, QString> extraVersionInfo;
extraVersionInfo["vamp_plugin_id"] = pluginId;
if (bPreferencesFastAnalysis) {
Expand Down
6 changes: 3 additions & 3 deletions src/analyserbeats.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@
#include "configobject.h"
#include "vamp/vampanalyser.h"

class AnalyserBeats: public Analyser {
class AnalyserBeats : public Analyser {
public:
AnalyserBeats(ConfigObject<ConfigValue>* pConfig);
virtual ~AnalyserBeats();
bool initialise(TrackPointer tio, int sampleRate, int totalSamples);
bool loadStored(TrackPointer tio) const;
void process(const CSAMPLE *pIn, const int iLen);
void process(const CSAMPLE* pIn, const int iLen);
void cleanup(TrackPointer tio);
void finalise(TrackPointer tio);

private:
static QHash<QString, QString> getExtraVersionInfo(
QString pluginId, bool bPreferencesFastAnalysis);
QString pluginId, bool bPreferencesFastAnalysis);
QVector<double> correctedBeats(QVector<double> rawbeats);

ConfigObject<ConfigValue>* m_pConfig;
Expand Down
Loading