Skip to content

gui: Do not translate InitWarning messages in debug.log#18922

Merged
jonasschnelli merged 4 commits intobitcoin:masterfrom
hebasto:200509-initwarn
May 13, 2020
Merged

gui: Do not translate InitWarning messages in debug.log#18922
jonasschnelli merged 4 commits intobitcoin:masterfrom
hebasto:200509-initwarn

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented May 9, 2020

This PR forces the bitcoin-qt to write InitWarning() messages to the debug.log file in untranslated form, i.e., in English.

On master (376294c):

$ ./src/qt/bitcoin-qt -lang=nl -debug=vladidation -printtoconsole | grep 'vladi'
Warning: Niet-ondersteunde logcategorie -debug=vladidation.
2020-05-09T12:39:59Z Warning: Niet-ondersteunde logcategorie -debug=vladidation.
2020-05-09T12:40:02Z Command-line arg: debug="vladidation"

With this PR:

$ ./src/qt/bitcoin-qt -lang=nl -debug=vladidation -printtoconsole | grep 'vladi'
Warning: Unsupported logging category -debug=vladidation.
2020-05-09T12:42:04Z Warning: Unsupported logging category -debug=vladidation.
2020-05-09T12:42:35Z Command-line arg: debug="vladidation"

Screenshot from 2020-05-09 15-42-31

Related to #16218.

maflcko
maflcko reviewed May 9, 2020
View reviewed changes
Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

Didn't know that auto return type is possible in C++11, but it seems to compile so why not 🤷‍♂️

@hebasto hebasto force-pushed the 200509-initwarn branch from 4e789b9 to 6249e64 Compare May 9, 2020 15:57
@hebasto
Copy link
Member Author

hebasto commented May 9, 2020

Updated 4e789b9 -> 6249e64 (pr18922.01 -> pr18922.02, diff):

  • reworked following @MarcoFalke's ideas and suggestions

@DrahtBot
Copy link
Contributor

DrahtBot commented May 9, 2020 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot mentioned this pull request May 9, 2020
Merged
maflcko
maflcko reviewed May 9, 2020
View reviewed changes
@hebasto hebasto force-pushed the 200509-initwarn branch from 6249e64 to 3ddfa41 Compare May 10, 2020 04:56
@hebasto
Copy link
Member Author

hebasto commented May 10, 2020 •
edited
Loading

Updated 6249e64 -> 3ddfa41 (pr18922.02 -> pr18922.03, diff):

Please keep the explicit overload for std::string, if you want you can extend it with a comment why it exists.

Also, I like the idea of deriving the return type from the unary operator

@elichai
Copy link
Contributor

elichai commented May 10, 2020

Didn't know that auto return type is possible in C++11, but it seems to compile so why not man_shrugging

That's interesting, I thought so too.
But apparently IIUC C++11 supports auto return type, but requires you to put a trailing return type (-> return_type) from C++14 you can also skip that and the return type will be deduced.

maflcko
maflcko reviewed May 10, 2020
View reviewed changes
@DrahtBot DrahtBot mentioned this pull request May 10, 2020
Merged
@hebasto hebasto force-pushed the 200509-initwarn branch from 3ddfa41 to 4871074 Compare May 10, 2020 13:49
@hebasto
Copy link
Member Author

hebasto commented May 10, 2020

Updated 3ddfa41 -> 4871074 (pr18922.03 -> pr18922.04, diff):

maflcko
maflcko reviewed May 10, 2020
View reviewed changes
@hebasto hebasto force-pushed the 200509-initwarn branch from 4871074 to da16f95 Compare May 10, 2020 15:09
@hebasto
Copy link
Member Author

hebasto commented May 10, 2020

Updated 4871074 -> da16f95 (pr18922.04 -> pr18922.05, diff):

Why is this one needed? (Sorry I missed it in my last review comment)

The current implementation of the Join() allows do not use OpOriginal and OpTranslated unary operators at all.
Is it worth to drop them?

@maflcko
Copy link
Member

maflcko commented May 10, 2020

Is it worth to drop them?

Up to you. I am happy to review either approach, as long as this is done in a new commit.

@hebasto
util: Drop OpOriginal() and OpTranslated()
78be8d9 
The current implementation of the Join() allows do not use OpOriginal()
and OpTranslated() unary operators at all.
@hebasto
Copy link
Member Author

hebasto commented May 10, 2020

Added commit which removes OpOriginal() and OpTranslated() unary operators.

promag
promag reviewed May 13, 2020
View reviewed changes
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

Concept ACK, code looks fine.

@laanwj
Copy link
Member

laanwj commented May 13, 2020

vladiation lol 😂

Looks good to me. Nice cleanup with the *Op functions.

ACK 78be8d9

To have an idea of progress, I would like to know whether an end to this in in sight. Is InitWarning the last step? Or are there more functions after this that need to be converted to bilingual_str?

@jonasschnelli
Copy link
Contributor

jonasschnelli commented May 13, 2020

Have vladidated the code.

utACK 78be8d9

@maflcko
Copy link
Member

maflcko commented May 13, 2020

I think the last ones are AbortNode and the gui/rpc warnings #16218 (comment)

@maflcko
Copy link
Member

maflcko commented May 13, 2020

ACK 78be8d9 📢

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 78be8d97d3d750fcfbbe509a72639b7b30b7bd18 📢
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUghsAv/TBQtsX9Zg+DSmubigFhi4U+8zCCTcrY+wH1oBCBcRVZdyDrgKQzWSQH5
f6fg99QfwzQYdhQd3Yykh1+YYTd0uHiUX/+YgRlaXpqNqvMoijMX/JjFgEOOKOCD
rueYNroFEqBChN79zQXj01/SIrVifVtIHzHQTbPsBkcQ6vUInOkQ9eBOpJZvkxro
6WTC/APvUWZWS1F8prsxOR/t6a1YVg8RdjpKqfnQIxUYj/uPIVQfDYK5nhyo4Tao
FRmYA6pDoDsT3MYkDFDOCG/4dXyri8PTdVlqv2W0fyJ5fjGT1YGIyrMp56aILRlj
CtaIniMEj+gQIP2Aqn49hCzlYP7ljMg+IP5BVKBbPu4C0fCjf1QFPYyDMrUT2yP0
w1/7NXqEerOFDpTewkO0C/a96sGN/TpvjDr4uaZ4Svx32kF0pmhiLxy0qtjwL7Wg
6iGt+m+yHDGPsJWzxDAlm+sJXzlUd3s2sQpDlqwv5afHASC5re38Vhc6OaqGt3lz
WrKEK80Y
=Eo5p
-----END PGP SIGNATURE-----

Timestamp of file with hash 69702236ca2cac943236ee37ffe813ced8a7bfccdd4b1792664d9fe1671d2185 -

@maflcko
Copy link
Member

maflcko commented May 13, 2020

@jonasschnelli Ready for merge?!

@jonasschnelli jonasschnelli merged commit 51825ae into bitcoin:master May 13, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 14, 2020
@jonasschnelli
Merge bitcoin#18922: gui: Do not translate InitWarning messages in de…
22afecd 
…bug.log

78be8d9 util: Drop OpOriginal() and OpTranslated() (Hennadii Stepanov)
da16f95 gui: Do not translate InitWarning messages in debug.log (Hennadii Stepanov)
4c9b9a4 util: Enhance Join() (Hennadii Stepanov)
fe05dd0 util: Enhance bilingual_str (Hennadii Stepanov)

Pull request description:

  This PR forces the `bitcoin-qt` to write `InitWarning()` messages to the `debug.log` file in untranslated form, i.e., in English.

  On master (376294c):
  ```
  $ ./src/qt/bitcoin-qt -lang=nl -debug=vladidation -printtoconsole | grep 'vladi'
  Warning: Niet-ondersteunde logcategorie -debug=vladidation.
  2020-05-09T12:39:59Z Warning: Niet-ondersteunde logcategorie -debug=vladidation.
  2020-05-09T12:40:02Z Command-line arg: debug="vladidation"
  ```

  With this PR:
  ```
  $ ./src/qt/bitcoin-qt -lang=nl -debug=vladidation -printtoconsole | grep 'vladi'
  Warning: Unsupported logging category -debug=vladidation.
  2020-05-09T12:42:04Z Warning: Unsupported logging category -debug=vladidation.
  2020-05-09T12:42:35Z Command-line arg: debug="vladidation"
  ```

  ![Screenshot from 2020-05-09 15-42-31](https://user-images.githubusercontent.com/32963518/81474073-c7a50e00-920b-11ea-8775-c41122dacafe.png)

  Related to bitcoin#16218.

ACKs for top commit:
  laanwj:
    ACK 78be8d9
  jonasschnelli:
    utACK 78be8d9
  MarcoFalke:
    ACK 78be8d9 📢

Tree-SHA512: 48e9ecd23c4dd8ec262e3eb94f8e30944bcc9c6c163245fb837b2e0c484d4d0b4f47f7abc638c14edc27d635d340ba3ee4ba4506b062399e9cf59a1564c98755
@hebasto hebasto deleted the 200509-initwarn branch May 14, 2020 04:35
@ryanofsky ryanofsky mentioned this pull request Jun 5, 2020
Closed
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 10, 2020
@hebasto @Fabcien
gui: Do not translate InitWarning messages in debug.log
db8a5bf 
Summary:
```
This PR forces the bitcoin-qt to write InitWarning() messages to the
debug.log file in untranslated form, i.e., in English.
```

Backport of core [[bitcoin/bitcoin#18922 | PR18922]].

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D8647
kwvg added a commit to kwvg/dash that referenced this pull request May 25, 2021
@kwvg
partial bitcoin#18922: Enhance Join()
f154f0d
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request May 28, 2021
@kwvg @UdjinM6
partial merge bitcoin#18922: Enhance Join()
33f09d0
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@promag promag promag left review comments

@maflcko maflcko maflcko left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

GUI Utils/log/libs Wallet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@hebasto @DrahtBot @elichai @maflcko @laanwj @jonasschnelli @promag