gui: rename debug window #17096

pull Zero-1729 wants to merge 1 commits into bitcoin:master from Zero-1729:rename-debug-window changing 2 files +3 −3
  1. Zero-1729 commented at 1:32 PM on October 10, 2019: contributor

    Edit: I have now limited the change in this PR to only renaming the window title from Debug Window to Node Window. Check this comment for more details.

    This PR is in response to #17082, which aims to rename the Debug window title to a more user friendly term; Node window.

    Closes #17082

  2. fanquake added the label GUI on Oct 10, 2019
  3. hebasto commented at 1:40 PM on October 10, 2019: member

    Concept ACK.

  4. hebasto commented at 1:42 PM on October 10, 2019: member

    Could scripted diff be used for renaming commits?

  5. in src/qt/rpcconsole.cpp:808 in 6cdb9560dc outdated
     804 | @@ -805,7 +805,7 @@ void RPCConsole::clear(bool clearHistory)
     805 |      QString clsKey = "Ctrl-L";
     806 |  #endif
     807 |  
     808 | -    message(CMD_REPLY, (tr("Welcome to the %1 RPC console.").arg(PACKAGE_NAME) + "<br>" +
     809 | +    message(CMD_REPLY, (tr("Welcome to the %1 Node console.").arg(PACKAGE_NAME) + "<br>" +
    


    MarcoFalke commented at 2:06 PM on October 10, 2019:

    in commit 6cdb9560dc8d301fb96cdb90c2b7c3d6fd0e4399:

    I think the console is still all about RPC, so could keep the previous name?


    Zero-1729 commented at 2:16 PM on October 10, 2019:

    I assumed changing the message from 'RPC console' to 'Node console' would make the new window name ('Node window') consistent with the welcome message.


    promag commented at 2:20 PM on October 10, 2019:

    The "Node window" has 4 tabs, the RPC console is one of those.


    Zero-1729 commented at 2:23 PM on October 10, 2019:

    Let me change that


    Zero-1729 commented at 3:22 PM on October 10, 2019:

    Fixed.

  6. in src/qt/rpcconsole.cpp:808 in 6cdb9560dc outdated
     804 | @@ -805,7 +805,7 @@ void RPCConsole::clear(bool clearHistory)
     805 |      QString clsKey = "Ctrl-L";
     806 |  #endif
     807 |  
     808 | -    message(CMD_REPLY, (tr("Welcome to the %1 RPC console.").arg(PACKAGE_NAME) + "<br>" +
    


    promag commented at 2:09 PM on October 10, 2019:

    This is actually RPC.

  7. promag commented at 2:09 PM on October 10, 2019: member

    Concept ACK, agree with the scripted diff.

  8. Zero-1729 commented at 2:21 PM on October 10, 2019: contributor

    Sorry, the ci tests failed because I seemed to have left debugwindow.ui instead of changing it to nodewindow.ui in src/Makefile.qt.include.

  9. Zero-1729 commented at 3:07 PM on October 10, 2019: contributor

    Are there any more changes/edits I need to make?

  10. DrahtBot commented at 4:11 PM on October 10, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  11. fanquake requested review from jonasschnelli on Oct 10, 2019
  12. laanwj commented at 5:50 PM on October 10, 2019: member

    I think it makes sense to separate the change in user facing name (in a separate PR) from the internal refactoring to change the class name. These can be discussed / bike shedded seperately.

  13. promag commented at 5:55 PM on October 10, 2019: member

    Agree with @laanwj. Also, maybe we can make the current tab the window title and so we don't have to decide for a new name?

  14. Zero-1729 commented at 6:24 PM on October 10, 2019: contributor

    I think it makes sense to separate the change in user facing name (in a separate PR) from the internal refactoring to change the class name. These can be discussed / bike shedded seperately.

    Fair enough, I guess I could squash the commits into a single commit (gui: rename debug window); with only the initial commit, along with the revert from this commit, making this PR only the user facing change.

    Then maybe open a separate PR later, with the internal class change and file renames, assuming this PR is merged.

  15. Zero-1729 commented at 7:20 PM on October 10, 2019: contributor

    Agree with @laanwj. Also, maybe we can make the current tab the window title and so we don't have to decide for a new name? @promag would that be changing the window title of NodeWidget (or RPCConsole) whenever a tab is set? Explicitly setting the window title to tab text, so {tab_name} Window, i.e: Information Window, Console Window, etc.

    E.g:

    Console Tab: Sample Window 1

    Is this what you mean? Or am I missing something?

  16. DrahtBot added the label Needs rebase on Oct 15, 2019
  17. Zero-1729 force-pushed on Oct 16, 2019
  18. DrahtBot removed the label Needs rebase on Oct 16, 2019
  19. Zero-1729 commented at 6:41 PM on October 16, 2019: contributor

    The changes made in this PR are too much, so I have squashed them all to only include the UI facing changes (i.e., the original gui: rename debug window commit), as per @laanwj's suggestion. Limiting this PR to the user facing change(s) only (i.e. the renaming of the window title from Debug window to Node window). That way this would remain a simple PR, instead of a super PR.

    Also, with respect to @promag's suggestion, about making the window title the current tab name, I have created a separate branch with the changes included, alongside the internal class change from RPCConsole to NodeWidget, file renames, etc. That way, assuming those changes are requested, a new PR can easily be opened.

  20. jonasschnelli approved
  21. jonasschnelli commented at 12:00 AM on November 16, 2019: contributor

    utACK b5d6aceaae878cf1b0abaf039ceb82c0e65ec246 - trivial change. Only user facing name change.

  22. Zero-1729 commented at 6:00 PM on November 16, 2019: contributor

    @jonasschnelli I just cloned the branch locally and built it from source, all the tests in src/test/test_bitcoin-qt were successful. 👍

    The window title properly displays "Node window".

  23. MarkLTZ referenced this in commit 91021bf551 on Nov 17, 2019
  24. Zero-1729 commented at 12:21 AM on November 22, 2019: contributor

    Are there any other checks or changes that need to be done?

  25. in src/qt/bitcoingui.cpp:321 in b5d6aceaae outdated
     317 | @@ -318,8 +318,8 @@ void BitcoinGUI::createActions()
     318 |      verifyMessageAction = new QAction(tr("&Verify message..."), this);
     319 |      verifyMessageAction->setStatusTip(tr("Verify messages to ensure they were signed with specified Bitcoin addresses"));
     320 |  
     321 | -    openRPCConsoleAction = new QAction(tr("&Debug window"), this);
     322 | -    openRPCConsoleAction->setStatusTip(tr("Open debugging and diagnostic console"));
     323 | +    openRPCConsoleAction = new QAction(tr("&Node window"), this);
    


    promag commented at 12:31 AM on November 22, 2019:

    This needs to be updated: https://github.com/bitcoin/bitcoin/blob/69a6f1ad1f7ca52b2524ab8322bfa89a9b0ee61b/src/qt/bitcoingui.cpp#L415

    But I'm not sure about this shortcut, there's now a shortcut for each tab so maybe drop these?


    promag commented at 12:33 AM on November 22, 2019:

    Also &N mnemonic is associated with "Network Traffic" which is a tab in the node window.


    Zero-1729 commented at 2:43 AM on November 22, 2019:

    This needs to be updated: https://github.com/bitcoin/bitcoin/blob/69a6f1ad1f7ca52b2524ab8322bfa89a9b0ee61b/src/qt/bitcoingui.cpp#L415

    But I'm not sure about this shortcut, there's now a shortcut for each tab so maybe drop these?

    So taking out the entire line since these commits make the shortcut unnecessary?


    Zero-1729 commented at 2:47 AM on November 22, 2019:

    Also &N mnemonic is associated with "Network Traffic" which is a tab in the node window.

    Do you have a suggestion as to what mnemonic to replace it with? Or would you rather it be removed completely:

    openRPCConsoleAction = new QAction(tr("Node window"), this);
    

    promag commented at 4:59 PM on November 22, 2019:

    I'd drop it, not sure about others though.


    Zero-1729 commented at 12:57 AM on November 23, 2019:

    Got it :+1:

  26. promag commented at 12:36 AM on November 22, 2019: member

    Also, with respect to @promag's suggestion, about making the window title the current tab name, I have created a separate branch with the changes included, alongside the internal class change from RPCConsole to NodeWidget, file renames, etc. That way, assuming those changes are requested, a new PR can easily be opened.

    Oh thanks for the consideration!

  27. Zero-1729 requested review from promag on Nov 23, 2019
  28. Zero-1729 commented at 9:06 PM on November 23, 2019: contributor

    @promag are there any more updates?

  29. fanquake commented at 11:01 PM on November 23, 2019: member
  30. Zero-1729 force-pushed on Nov 24, 2019
  31. Zero-1729 commented at 12:19 AM on November 24, 2019: contributor

    @fanquake squashed :+1:

  32. Zero-1729 commented at 10:19 AM on December 19, 2019: contributor

    Are there any more updates I need to make to the current changes?

  33. in src/qt/bitcoingui.cpp:415 in 5c4a9a947c outdated
     411 | @@ -412,7 +412,6 @@ void BitcoinGUI::createActions()
     412 |  #endif // ENABLE_WALLET
     413 |  
     414 |      connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_C), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindowActivateConsole);
     415 | -    connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_D), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindow);
    


    hebasto commented at 10:44 PM on January 5, 2020:

    Removing this shortcut is not mentioned in the PR description. Moreover, I'd prefer to keep it as is.

    Refs:


    Zero-1729 commented at 7:35 PM on January 6, 2020:

    Oh, my bad, the change was in response to an earlier review by @promag.

    Just read the refs and reverted the change :+1:

  34. gui: renamed 'debug window' to 'node window'
    - Renamed 'Debug window' to a more user friendly term - 'Node window' - in the window title and tray menu.
    
    fixes #17082
    44f15cfdcf
  35. Zero-1729 force-pushed on Jan 6, 2020
  36. hebasto approved
  37. hebasto commented at 9:52 AM on January 7, 2020: member

    ACK 44f15cfdcfc64d5a0c36fded39e4aef49415b11b, tested on Linux Mint 19.3:

    DeepinScreenshot_select-area_20200107114953

    DeepinScreenshot_select-area_20200107115040

  38. Zero-1729 commented at 6:10 PM on January 13, 2020: contributor

    I now realize I can't cancel the review request I mistakenly made to @promag a while back, as I lack the permissions. He should just ignore the request so I don't waste his time, especially since @hebasto has ACKed the changes.

  39. theStack approved
  40. theStack commented at 1:40 PM on January 22, 2020: member
  41. jonasschnelli referenced this in commit 3253b5dcf4 on Jan 27, 2020
  42. jonasschnelli merged this on Jan 27, 2020
  43. jonasschnelli closed this on Jan 27, 2020

  44. sidhujag referenced this in commit 67ff110a92 on Jan 27, 2020
  45. MarkLTZ referenced this in commit 136a27c84d on Feb 3, 2020
  46. promag commented at 9:26 PM on September 20, 2020: member

    @Zero-1729 sorry missed your review request.

  47. Zero-1729 commented at 1:52 PM on September 23, 2020: contributor

    @promag no worries :+1:.

  48. sidhujag referenced this in commit 65ea373f78 on Nov 10, 2020
  49. jasonbcox referenced this in commit 661586a033 on Nov 25, 2020
  50. ftrader referenced this in commit cf5a10a0ce on Apr 14, 2021
  51. DrahtBot locked this on Feb 15, 2022

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-14 21:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me