-
-
Notifications
You must be signed in to change notification settings - Fork 449
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
Remove BaseSettings & merge ConcurrentSettings #4775
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
} | ||
|
||
bool ConcurrentSettings::isHighlightedUser(const QString &username) | ||
std::vector<std::weak_ptr<pajlada::Settings::SettingData>> _settings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable '_settings' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
std::vector<std::weak_ptr<pajlada::Settings::SettingData>> _settings;
^
} | ||
|
||
bool ConcurrentSettings::isHighlightedUser(const QString &username) | ||
std::vector<std::weak_ptr<pajlada::Settings::SettingData>> _settings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for global variable '_settings' [readability-identifier-naming]
std::vector<std::weak_ptr<pajlada::Settings::SettingData>> _settings; | |
std::vector<std::weak_ptr<pajlada::Settings::SettingData>> SETTINGS; |
src/singletons/Settings.cpp:46:
- _settings.push_back(std::move(setting));
+ SETTINGS.push_back(std::move(setting));
src/singletons/Settings.cpp:197:
- for (const auto &weakSetting : _settings)
+ for (const auto &weakSetting : SETTINGS)
src/singletons/Settings.cpp:236:
- for (const auto &weakSetting : _settings)
+ for (const auto &weakSetting : SETTINGS)
{ | ||
QString settingsPath = settingsDirectory + "/settings.json"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'settingsPath' of type 'QString' can be declared 'const' [misc-const-correctness]
QString settingsPath = settingsDirectory + "/settings.json"; | |
QString const settingsPath = settingsDirectory + "/settings.json"; |
|
||
void Settings::saveSnapshot() | ||
{ | ||
rapidjson::Document *d = new rapidjson::Document(rapidjson::kObjectType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: use auto when initializing with new to avoid duplicating the type name [modernize-use-auto]
rapidjson::Document *d = new rapidjson::Document(rapidjson::kObjectType); | |
auto *d = new rapidjson::Document(rapidjson::kObjectType); |
} | ||
|
||
rapidjson::Value key(setting->getPath().c_str(), a); | ||
auto curVal = setting->unmarshalJSON(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'auto curVal' can be declared as 'auto *curVal' [llvm-qualified-auto]
auto curVal = setting->unmarshalJSON(); | |
auto *curVal = setting->unmarshalJSON(); |
return; | ||
} | ||
|
||
const auto &snapshot = *(this->snapshot_.get()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: redundant get() call on smart pointer [readability-redundant-smartptr-get]
const auto &snapshot = *(this->snapshot_.get()); | |
const auto &snapshot = *(this->snapshot_); |
|
||
float Settings::getClampedUiScale() const | ||
{ | ||
return clamp<float>(this->uiScale.getValue(), 0.2f, 10); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]
return clamp<float>(this->uiScale.getValue(), 0.2f, 10); | |
return clamp<float>(this->uiScale.getValue(), 0.2F, 10); |
|
||
void Settings::setClampedUiScale(float value) | ||
{ | ||
this->uiScale.setValue(clamp<float>(value, 0.2f, 10)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]
this->uiScale.setValue(clamp<float>(value, 0.2f, 10)); | |
this->uiScale.setValue(clamp<float>(value, 0.2F, 10)); |
}; | ||
|
||
ConcurrentSettings &getCSettings(); | ||
void _actuallyRegisterSetting( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for function '_actuallyRegisterSetting' [readability-identifier-naming]
void _actuallyRegisterSetting( | |
void actuallyRegisterSetting( |
7789e34
to
a0bea33
Compare
Description