-
Notifications
You must be signed in to change notification settings - Fork 9
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
Improve app config #237
Improve app config #237
Conversation
inline static const auto gnomeSS = QStringLiteral("/usr/bin/gnome-screenshot"); | ||
inline static const auto gnomeWindowCommand = QStringLiteral("gnome-screenshot -w -f %file"); | ||
inline static const auto gnomeAreaCommand = QStringLiteral("gnome-screenshot -a -f %file"); |
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.
I'm not sure if this is accurate anymore. Gnome changed their screenshot app to pop up a UI. I don't know if the CLI works the same anymore (or, really, at all)
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.
It might not be current but it will work for those with gnome-screenshot installed,
Ill see if there is another newer tool for gnome to check for, Oh its now a part of the shell not a standalone application. perhaps we can recommend a tool for those users. and ill add it to the list.
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.
Actually, yes, that's true. I thought they replaced the binary but it looks like it's not called gnome-screenshot anymore.
Edit: The new screenshotting functionality is apparently built-in to the gnome shell. It doesn't appear that there's a way to invoke it from the terminal.
The changes look fine to me. A couple of thoughts:
if (appConfig->value(CONFIG::SHORTCUT_SCREENSHOT).isNull())
appConfig->setValue(CONFIG::SHORTCUT_SCREENSHOT, defaultValue(CONFIG::SHORTCUT_SCREENSHOT));
//why not:
auto setDefaultIfNull = [appConfig](QString field){ // is it a QString? This is how lazy I am: I can't be bothered to look right now.
if (appConfig->value(field).isNull())
appConfig->setValue(field, defaultValue(field));
}
setDefaultIfNull(CONFIG::SHORTCUT_SCREENSHOT);
setDefaultIfNull(CONFIG::I_FORGET_THE_OTHER_FIELDS); |
a4317d7
to
310d62b
Compare
Fixed it we don't even need to see the config if we try to read and it's empty we just get the default due to the change in the value method . Good call :D |
310d62b
to
07a0471
Compare
defaultValue
method to AppConfig and set several defauts