src/net.cpp: Fail to listen alert text break fix #23568

RandyMcMillan commented at 3:14 AM on November 22, 2021: contributor

No description provided.

RandyMcMillan commented at 3:17 AM on November 22, 2021: contributor

Original gui PR: https://github.com/bitcoin-core/gui/pull/476

https://github.com/bitcoin-core/gui/pull/476#issuecomment-974583912

When another instance of bitcoin is running:

Before:

After:

DrahtBot added the label P2P on Nov 22, 2021

katesalazar commented at 7:27 AM on November 22, 2021: contributor

Concept ACK. Wouldn't merge a597dcf81613d43 before having a look at how it looks in other deployments.

src/net.cpp: Fail to listen alert text break fix ae1e47430e

in src/net.cpp:2535 in a597dcf816 outdated

2531 | @@ -2532,7 +2532,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
2532 |      if (fListen && !InitBinds(connOptions)) {
2533 |          if (m_client_interface) {
2534 |              m_client_interface->ThreadSafeMessageBox(
2535 | -                _("Failed to listen on any port. Use -listen=0 if you want this."),
2536 | +                _("Failed to listen on any port.\nUse -listen=0 if you want this."),

laanwj commented at 3:43 PM on November 26, 2021:

I think we're generally trying to avoid having newlines in translation messages? @hebasto

hebasto commented at 4:16 PM on November 27, 2021:

Yes. As I mentioned in https://github.com/bitcoin-core/gui/pull/476#issuecomment-974891494, we cannot be sure that the chosen location of the hardcoded \n is optimal for translated strings.

If the problem is the breaking -listen=0, why do not use a nonbreaking hyphen:

                _("Failed to listen on any port. Use \u2011listen=0 if you want this."),

?

katesalazar commented at 4:40 PM on November 27, 2021:

Breaking text at the end of the sentence would be cutest. (at least at that macOS build). Breaking text before -listen=0 is good enough as a first step.

RandyMcMillan force-pushed on Nov 27, 2021

RandyMcMillan commented at 2:15 AM on November 28, 2021: contributor

Update: commit ae1e474

Screen Shot 2021-11-27 at 9 14 15 PM

in src/net.cpp:2535 in ae1e47430e

2531 | @@ -2532,7 +2532,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
2532 |      if (fListen && !InitBinds(connOptions)) {
2533 |          if (m_client_interface) {
2534 |              m_client_interface->ThreadSafeMessageBox(
2535 | -                _("Failed to listen on any port. Use -listen=0 if you want this."),
2536 | +                _("Failed to listen on any port. Use \u2011listen=0 if you want this."),

luke-jr commented at 9:09 PM on November 30, 2021:

Approach NACK: \u2011 is not a valid character on the commandline, so this will break if someone copy/pastes it.

katesalazar commented at 9:42 PM on November 30, 2021:

Aha... so replacing " -([a-z]+)=..." with " \u2011{1}={2}" somewhere in ThreadSafeMessageBox would be OK?

luke-jr commented at 9:56 PM on November 30, 2021:

No? Users may wish to copy out of GUI dialogs too.

katesalazar commented at 7:42 PM on December 1, 2021:

Aye, I actually thought about that other case, but wasn't sure about it.

flack commented at 8:31 PM on December 1, 2021:

not sure how that would work in Qt, but couldn't you split the text into two strings, and then show each sentence on a new line?

_("Failed to listen on any port.")
// insert line break
_("Use -listen=0 if you want this.")

That's how the screenshot in the OP looks, and it also makes sense semantically

katesalazar commented at 7:31 PM on December 12, 2021:

not sure how that would work in Qt, but couldn't you split the text into two strings, and then show each sentence on a new line?

That's how the screenshot in the OP looks, and it also makes sense semantically

Neat for the window in macOS, but is there any possibility that this is going to end up introducing an unnecessary line break in any console output?

In other words, is it somewhere else that actually has to sort out the passed text better because isn't currently good enough?

Ideally, shouldn't be a responsibility of Bitcoin source code, but the widget toolkit, or the desktop environment.

luke-jr changes_requested

MarcoFalke commented at 7:43 AM on December 13, 2021: member

What is the goal of this pull? Tend to -0

RandyMcMillan commented at 2:05 PM on December 13, 2021: contributor

This should've been a trivial fix - since other commits have been merged with '\n' in them.

RandyMcMillan closed this on Dec 13, 2021

flack commented at 2:35 PM on December 13, 2021: contributor

@MarcoFalke the goal is (or was) to convey to the user that they should use -listen=0 to get around the error. However, due due unfortunate text flowing, it might look like - listen=0, which won't work

MarcoFalke commented at 3:16 PM on December 13, 2021: member

Oh, I didn't realize this because the pull description was emtpy

flack commented at 3:29 PM on December 13, 2021: contributor

yeah, the info is in the first comment, not sure why

katesalazar commented at 7:36 PM on December 13, 2021: contributor

On Mon, Dec 13, 2021 at 3:05 PM @RandyMcMillan @.***> wrote:

This should've been a trivial fix - since other commits have been merged with '\n' in them.

then tell that, link the git revs, instead of rage closing, let maintainers the decision to close PRs 🙁

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/23568#issuecomment-992513415, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMRS4W7VYBLADB5C5MLG5QDUQX4SXANCNFSM5IP6QABQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

luke-jr commented at 10:20 PM on December 13, 2021: member

\u2011 is not \n

DrahtBot locked this on Dec 13, 2022