Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Oct 9, 2014

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.

@laanwj laanwj force-pushed the 2014_10_gui_network_style branch from 34a7365 to b42db95 Compare October 9, 2014 09:18
Copy link

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?

Copy link
Member Author

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.

Copy link
Member Author

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.

@Diapolo
Copy link

Diapolo commented Oct 9, 2014

Seems like an ACK-worthy idea, appart from the tr() thing and the nits.

@laanwj
Copy link
Member Author

laanwj commented Oct 9, 2014

There you go, translated [testnet] again:
untitled

@laanwj laanwj force-pushed the 2014_10_gui_network_style branch from b573d7a to 64b8208 Compare October 9, 2014 10:38
@Diapolo
Copy link

Diapolo commented Oct 9, 2014

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 laanwj force-pushed the 2014_10_gui_network_style branch from 64b8208 to 99a8143 Compare October 9, 2014 11:59
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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.

Copy link

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.

Copy link
Member Author

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theuni
Copy link
Member

theuni commented Oct 9, 2014

utACK, much cleaner.

@laanwj
Copy link
Member Author

laanwj commented Oct 10, 2014

Closing as it's now part of #4802

@laanwj laanwj closed this Oct 10, 2014
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants