Replace "Hide tray icon" option with positive "Show tray icon" one #115
Conversation
src/qt/optionsmodel.cpp
Outdated
| settings.setValue("fHideTrayIcon", false); | ||
| fHideTrayIcon = settings.value("fHideTrayIcon").toBool(); | ||
| Q_EMIT hideTrayIconChanged(fHideTrayIcon); | ||
| if (!settings.contains("bShowTrayIcon")) { |
There was a problem hiding this comment.
nit, is there a reason to change setting key? No big deal in this case, but ideally "old" key would be migrated - if you wish to rename the key.
There was a problem hiding this comment.
It is not just renaming here. fHideTrayIcon and bShowTrayIcon are different semantically. As the Bitcoin-Qt.conf is written in the human readable format, I'd prefer to introduce a new setting key to avoid any possible confusing.
An old key migration sounds good to me. But is it worth to add more code for such a non-critical key?
There was a problem hiding this comment.
It is not just renaming here.
fHideTrayIconandbShowTrayIconare different semantically.
Of course, you would have to take the !fHideTrayIcon.
An old key migration sounds good to me. But is it worth to add more code for such a non-critical key?
I've started with "nit" 😅
There was a problem hiding this comment.
It is not just renaming here. fHideTrayIcon and bShowTrayIcon are different semantically.
How?
As the Bitcoin-Qt.conf is written in the human readable format, I'd prefer to introduce a new setting key to avoid any possible confusing.
That's not a good reason IMO
There was a problem hiding this comment.
It is not just renaming here. fHideTrayIcon and bShowTrayIcon are different semantically.
How?
The meaning of fHideTrayIcon is opposite to bShowTrayIcon, no?
As the Bitcoin-Qt.conf is written in the human readable format, I'd prefer to introduce a new setting key to avoid any possible confusing.
That's not a good reason IMO
What do you suggest?
There was a problem hiding this comment.
The meaning of fHideTrayIcon is opposite to bShowTrayIcon, no?
It's a strict opposite.
What do you suggest?
Leave fHideTrayIcon alone and just change the UI presentation.
src/qt/optionsmodel.cpp
Outdated
| settings.setValue("fHideTrayIcon", false); | ||
| fHideTrayIcon = settings.value("fHideTrayIcon").toBool(); | ||
| Q_EMIT hideTrayIconChanged(fHideTrayIcon); | ||
| if (!settings.contains("bShowTrayIcon")) { |
There was a problem hiding this comment.
It is not just renaming here. fHideTrayIcon and bShowTrayIcon are different semantically.
How?
As the Bitcoin-Qt.conf is written in the human readable format, I'd prefer to introduce a new setting key to avoid any possible confusing.
That's not a good reason IMO
This change makes easier both (1) using this option, and (2) reasoning about the code.
The removed BitcoinGUI::setTrayIconVisible just duplicates QSystemTrayIcon::setVisible.
This change makes easier both (1) using this option, and (2) reasoning about the code.