qt: Add warning messages to the debug window #14879

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:20181205-debugwindow-warnings changing 4 files +41 −3
  1. hebasto commented at 11:06 PM on December 5, 2018: member

    Fix: #11016

    This PR adds warning messages to the debug window in -disablewallet mode.

    screenshot from 2018-12-06 01-01-27

  2. fanquake added the label GUI on Dec 5, 2018
  3. in src/qt/rpcconsole.cpp:12 in 436389bf6f outdated 
       8 | @@ -9,25 +9,24 @@
   9 |  #include <qt/rpcconsole.h>
  10 |  #include <qt/forms/ui_debugwindow.h>
  11 |  
  12 | -#include <qt/bantablemodel.h>
    promag commented at 11:35 PM on December 5, 2018:

    No need to touch the headers.

    hebasto commented at 7:22 PM on December 16, 2018:

    Agree.

  4. promag commented at 11:44 PM on December 5, 2018: member

    An alternative approach is to extract the existing alert from the OverviewPage and add that to the QMainWindow. In addition you could try to use addDockWidget(Qt::TopDockWidgetArea, alert_widget).

    Anyway, my point is to avoid duplicate code.

  5. DrahtBot commented at 1:32 AM on December 6, 2018: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  6. hebasto commented at 9:56 AM on December 6, 2018: member

    @promag Thank you for your review.

    An alternative approach is to extract the existing alert from the OverviewPage and add that to the QMainWindow. In addition you could try to use addDockWidget(Qt::TopDockWidgetArea, alert_widget).

    That was my first thought exactly :) In that case warning messages will be exposed not only on Overview tab but on Send, Receive and Transactions tabs too. Can such behaviour be annoying for a user?

    No need to touch the headers.

    I've just moved #include <qt/walletmodel.h> under the #ifdef ENABLE_WALLET guard as all rpcconsole.cpp code supposed. All others headers moves were made by clang-format-diff.py. Let code formatting approach to an ideal step by step :)

  7. in src/qt/rpcconsole.cpp:25 in 436389bf6f outdated 
      25 | +#include <univalue.h>
  26 |  #include <util/system.h>
  27 |  
  28 |  #include <openssl/crypto.h>
  29 |  
  30 | -#include <univalue.h>
    MarcoFalke commented at 6:42 PM on December 6, 2018:

    univalue is a library, so shouldn't move that?

    hebasto commented at 7:39 PM on December 6, 2018:

    @MarcoFalke Sure!

  8. hebasto force-pushed on Dec 6, 2018
  9. promag commented at 6:29 PM on December 9, 2018: member

    Can such behaviour be annoying for a user?

    IMO it'd be preferable

  10. jonasschnelli commented at 8:41 PM on December 9, 2018: contributor

    Concept ACK

  11. DrahtBot added the label Needs rebase on Dec 16, 2018
  12. hebasto force-pushed on Dec 16, 2018
  13. hebasto commented at 7:22 PM on December 16, 2018: member

    Rebased.

  14. DrahtBot removed the label Needs rebase on Dec 16, 2018
  15. hebasto force-pushed on Dec 17, 2018
  16. hebasto commented at 2:37 PM on December 17, 2018: member

    Rebased on top of 3e21b690d1aedb73a7dc2bc5d2ff1b011b52d927.

  17. hebasto commented at 9:07 AM on January 1, 2019: member

    @promag

    ... you could try to use addDockWidget(Qt::TopDockWidgetArea, alert_widget).

    By design, a QDockWidget consists of a title bar and the content area. An alert label can be placed to any of these elements of alert_widget. The other element (without the alert label) cannot be hidden, unfortunately. In addition, such implementation should handle resize events properly that adds some new code and possibly introduces new GUI-related bugs.

  18. hebasto commented at 8:24 AM on January 25, 2019: member

    @Sjors would you mind reviewing this PR?

  19. in src/qt/rpcconsole.cpp:568 in e4fb571969 outdated 
     563 | @@ -563,6 +564,17 @@ bool RPCConsole::eventFilter(QObject* obj, QEvent *event)
 564 |  void RPCConsole::setClientModel(ClientModel *model)
 565 |  {
 566 |      clientModel = model;
 567 | +
 568 | +    auto wallet_enabled = false;
    Sjors commented at 10:00 AM on January 25, 2019:

    nit: bool

  20. in src/qt/rpcconsole.cpp:573 in e4fb571969 outdated 
     568 | +    auto wallet_enabled = false;
 569 | +#ifdef ENABLE_WALLET
 570 | +    wallet_enabled = WalletModel::isWalletEnabled();
 571 | +#endif // ENABLE_WALLET
 572 | +    if (model && !wallet_enabled) {
 573 | +        // Show warning if this is a prerelease version
    Sjors commented at 10:01 AM on January 25, 2019:

    Should be: // Show warning, for example if

  21. Sjors approved
  22. Sjors commented at 10:29 AM on January 25, 2019: member

    tACK e4fb571

    I don't mind the slight duplication of view code.

    In that case warning messages will be exposed not only on Overview tab but on Send, Receive and Transactions tabs too. Can such behaviour be annoying for a user?

    The Overview tab is the first thing you see when you launch Bitcoin Core, so I don't see the point in showing warning in other places. That's especially true the first time you launch the app, so everyone will have seen that it's not a release build.

    The only exception I can think of is when you open a BIP21/70 URI, so maybe some really critical warnings could also be on the send screen, but currently the warnings, e.g. "new version bit" aren't informative enough for that to be useful.

  23. hebasto force-pushed on Jan 25, 2019
  24. hebasto commented at 10:47 AM on January 25, 2019: member

    @Sjors Thank you for your review. All your comments have been addressed.

  25. Sjors commented at 11:28 AM on January 25, 2019: member

    re-tACK c0c9613 assuming Travis is happy (I just restarted two instances, they seem to have been confused by cached code, I noticed that locally too).

  26. Add warning messages to the debug window 593ba696fb
  27. hebasto force-pushed on Jan 25, 2019
  28. hebasto commented at 1:17 PM on January 25, 2019: member

    @Sjors Travis is happy now :)

    After merging #15101 the <qt/walletmodel.h> header gets required even with --disable-wallet.

  29. hebasto commented at 9:39 PM on April 21, 2019: member

    @fanquake would you mind reviewing this PR?

  30. jonasschnelli commented at 4:00 PM on April 23, 2019: contributor

    utACK 593ba696fb32da558091ac02ad87c4893db4ce97

  31. promag commented at 7:12 AM on April 24, 2019: member

    ACK 593ba696fb32da558091ac02ad87c4893db4ce97, agree with @Sjors #14879#pullrequestreview-196433092 above.

  32. hebasto commented at 3:52 PM on June 19, 2019: member

    @ryanofsky @MarcoFalke Would you mind reviewing this PR?

  33. MarcoFalke commented at 3:55 PM on June 19, 2019: member

    Concept ACK

  34. MarcoFalke added this to the milestone 0.19.0 on Jun 19, 2019
  35. in src/qt/rpcconsole.cpp:571 in 593ba696fb 
     562 | @@ -563,6 +563,17 @@ bool RPCConsole::eventFilter(QObject* obj, QEvent *event)
 563 |  void RPCConsole::setClientModel(ClientModel *model)
 564 |  {
 565 |      clientModel = model;
 566 | +
 567 | +    bool wallet_enabled{false};
 568 | +#ifdef ENABLE_WALLET
 569 | +    wallet_enabled = WalletModel::isWalletEnabled();
 570 | +#endif // ENABLE_WALLET
 571 | +    if (model && !wallet_enabled) {
    ryanofsky commented at 5:03 PM on June 19, 2019:

    In commit "Add warning messages to the debug window" (593ba696fb32da558091ac02ad87c4893db4ce97)

    Instead of repeating the wallet check from BitcoinGUI::BitcoinGUI, I think it'd be less fragile and repetitive to add a simpler RPCConsole::enableAlerts() method here:

    void RPCConsole::enableAlerts()
{
    connect(model, &ClientModel::alertsChanged, this, &RPCConsole::updateAlerts);
    updateAlerts(model->getStatusBarWarnings());
}

    and call it from BitcoinGUI::BitcoinGUI where there is already a wallet enabled check being done

    hebasto commented at 10:15 AM on June 23, 2019:

    This approach will not work because the ClientModel object has not yet been created at that moment. So I'd prefer to leave it as is.

  36. ryanofsky approved
  37. ryanofsky commented at 5:06 PM on June 19, 2019: member

    utACK 593ba696fb32da558091ac02ad87c4893db4ce97

  38. MarcoFalke referenced this in commit 93305e6d46 on Aug 28, 2019
  39. MarcoFalke merged this on Aug 28, 2019
  40. MarcoFalke closed this on Aug 28, 2019
  41. sidhujag referenced this in commit ab294f70a1 on Aug 28, 2019
  42. hebasto deleted the branch on Aug 29, 2019
  43. monstrobishi referenced this in commit ba03d4cd54 on Jul 30, 2020
  44. ShengguangXiao referenced this in commit 53f8145406 on Aug 28, 2020
  45. jasonbcox referenced this in commit 9261e048c7 on Oct 15, 2020
  46. ftrader referenced this in commit 1a1ac4e5a0 on Apr 14, 2021
  47. DrahtBot locked this on Dec 16, 2021
Contributors
hebastopromagDrahtBotMarcoFalkejonasschnelliSjorsryanofsky
Labels
GUI

Milestone
0.19.0

Linked (view graph)
#11016 [Qt] Warning messages are not displayed when running bitcoin-qt with -disablewallet
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-21 18:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me