qt: add network-specific style object#5066
Conversation
laanwj
force-pushed
the
2014_10_gui_network_style
branch
from
34a7365 to
b42db95
Compare
src/qt/networkstyle.cpp
Outdated
There was a problem hiding this comment.
As is, this removes the possibility to translate testnet or regtest. Is this intended?
There was a problem hiding this comment.
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.
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. |
|
There you go, translated [testnet] again: |
laanwj
force-pushed
the
2014_10_gui_network_style
branch
from
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.
laanwj
force-pushed
the
2014_10_gui_network_style
branch
from
64b8208 to
99a8143
Compare
src/qt/bitcoin.cpp
Outdated
There was a problem hiding this comment.
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.
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.
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.
Здравствуйте друзья!Я не разбираюсь в этом.С уважением Махмет!
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.
@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.
mirbitcoin21000000 - https://MirBitcoin21000000
https://sites.google.com/site/mirbitcoin21000000/.
google.com/site/mirbitcoin21000000/
|
utACK, much cleaner. |
|
Closing as it's now part of #4802 |
4 participants
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.