-
Notifications
You must be signed in to change notification settings - Fork 38.7k
qt: add network-specific style object #5066
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
Conversation
34a7365 to
b42db95
Compare
src/qt/networkstyle.cpp
Outdated
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.
As is, this removes the possibility to translate testnet or regtest. Is this intended?
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.
Do we really want people to translate the (developer jargon abbreviation) network names? I don't think so.
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.
Hmm well on the other hand "[testnet]" is already a translation message. So there is no extra work in keeping it. And people already know (or should know) about the testnet. Not sure about regtest though, that's just too obscure.
|
Seems like an ACK-worthy idea, appart from the tr() thing and the nits. |
b573d7a to
64b8208
Compare
|
Thanks for the fixes, utACK. |
Mainly cleanups: Gets rid of isTestNet everywhere, by keeping track of network-specific theming in a central place. Also makes GUI no longer dependent on the network ID enumeration, which alleviates concerns about bitcoin#4802.
64b8208 to
99a8143
Compare
src/qt/bitcoin.cpp
Outdated
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 think a NetworkStyle::destroy(networkStyle) or networkStyle->destroy() or so would be a bit more clear here? With just a delete, it's hard to tell where it came from.
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.
Does it matter where it came from? In principle any wrap-up code could be done in the destructor.
I'm fine with adding a NetworkStyle::destroy(obj) function if you think it's more symmetric, but unlike instantiate there is no reason to do anything special based on the network, it would just call delete anyway. I don't like the ->destroy() style as it involves delete self.
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 doesn't really matter, it was just a suggestion for readability.
Out of curiosity though, why not just create a normal scoped object in main and pass around a pointer to it? I suspect I'm missing something obvious.
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.
Здравствуйте друзья!Я не разбираюсь в этом.С уважением Махмет!
2014-10-09 23:07 GMT+04:00 Cory Fields [email protected]:
In src/qt/bitcoin.cpp:
@@ -636,6 +635,8 @@ int main(int argc, char *argv[])
PrintExceptionContinue(NULL, "Runaway exception");
app.handleRunawayException(QString::fromStdString(strMiscWarning));
}
- delete networkStyle;
It doesn't really matter, it was just a suggestion for readability.
Out of curiosity though, why not just create a normal scoped object in
main and pass around a pointer to it? I suspect I'm missing something
obvious.—
Reply to this email directly or view it on GitHub
https://github.com/bitcoin/bitcoin/pull/5066/files#r18666838.
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.
@makhmet Graag alleen Engels hier.
@theuni Well - I initially started from the approach that there was a Style class per network, but that didn't turn out to be needed. The current interface (with a constructor-independent instantiate) leaves some freedom to return to that later if it becomes necessary (ie, if you'd want to paint the background in a network-specific way and offer a method for that). Also the function can return 0 if the network is unknown, unlike a constructor which would have to mess around with exceptions.
Good idea about the RAII though. I'll change the networkStyle in main to a boost::scoped_ptr QScopedPointer.
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.
mirbitcoin21000000 - https://MirBitcoin21000000
https://sites.google.com/site/mirbitcoin21000000/.
google.com/site/mirbitcoin21000000/
|
utACK, much cleaner. |
|
Closing as it's now part of #4802 |

Mainly cleanups: Gets rid of isTestNet everywhere, by keeping track of network-specific theming in a central place.
Also makes GUI no longer dependent on the network ID enumeration, which alleviates concerns about #4802.