util: change GetWarnings parameter to bool #17750

GetWarnings() changes the format of the output warning string based on a passed-in string argument that can be set to "gui" or "statusbar".

Change the argument to a bool:

• there are only two types of behaviour, so a bool is a more natural argument type
• changing the name to verbose does not set any expectations for the how the calling code will use the returned string (currently, statusbar is used for RPC warnings, not a status bar)
• removes some error-handling code for when the passed-in string is not one of the two strings expected.

Code review ACK 7aab8d1024996c7c422bd34a8226df0117b813f7.

It was always weird to have a string argument here. I think some of the "conditional translation" code could be simplified even more with bilingual_str, but, I don't think it's necessary to do so here.

code review ACK 7aab8d1024996c7c422bd34a8226df0117b813f7

It was always weird to have a string argument here.

Yes, I expect Satoshi was giving himself flexibility in case other formats were needed in future. If that does happen we can always switch this to an enum.

Yea, there was at least "rpc" which was removed in d8e9a2ac74e1071c9a92b9858b7ed3143413ee94. But a enum seemed a more obvious choice (at the time, it doesn't matter now, I don't expect the set of possibilities here to be ever expanded again). There was a previous try at this in #11412, but the concerns there no longer hold as safe mode was removed (which would disable some RPC calls when the function returned a non-empty string!).

ACK 7aab8d1024996c7c422bd34a8226df0117b813f7 -- diff looks correct :)

Not sure if this makes sense. Maybe in the future, an RPC might want to return the "verbose" warning. This would be harder with your changes, as there is no way to get the verbose warning without translation.

As you are changing all call sites anyway, you might want to use bilingual_str, and Untranslated to denote whether a string is translated. This makes it possible for the caller (rpc or gui) to pick out the right string.

ACK 7aab8d1024996c7c422bd34a8226df0117b813f7 otherwise.

Edit: Untranslated might not exist on master, but you can add it with

+/** Mark a bilingual_str as untranslated */
+inline static bilingual_str Untranslated(std::string original) { return {original, original}; }

Maybe in the future, an RPC might want to return the "verbose" warning. This would be harder with your changes, as there is no way to get the verbose warning without translation.

There's no way to get the verbose warning without translation either before or after this PR. I can't see how this is any harder after this change than before (and even if it were, I don't see "we may want to do something else in future" as a good argument against improving the code now).

As you are changing all call sites anyway, you might want to ...

That's not the aim of this PR. Feel free to open a follow-up.

Yes, I understand that it wasn't possible before either. However, at least the parameter was explicitly called "gui", which implies that the string might be translated. With your changes, it appears that an RPC call could just set it to verbose to get more warnings (still untranslated).