Skip to content

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Apr 10, 2012

This pull request adds a debug window to the UI (can be opened through Help → Debug window). This includes:

  • Various information (version, number of blocks, number of connections etc)
  • Console for RPC commands (local, not really RPC, also works when not running with -server)

@sipa
Copy link
Member

sipa commented Apr 10, 2012

Wow, nice!

Small request: make responses copy-pastable.

@laanwj
Copy link
Member Author

laanwj commented Apr 10, 2012

They are (though not really intuitively), you can click on the table cell
and press ctrl-c

@Diapolo
Copy link

Diapolo commented Apr 10, 2012

I really like the idea, but would place the function under help and add a seperator line. It simply is no setting or option :). Is there a way export to a text file? Would be consistent to offer such a function, as this is possible "all over the GUI".

@laanwj
Copy link
Member Author

laanwj commented Apr 10, 2012

It's not help either, nor file. We really need a "tools" menu like other
programs. Maybe the settings menu could be renamed "tools", then it makes
more sense.

Exporting the console contents to a file could be useful. I'll keep that in
mind for a future update.

@Diapolo
Copy link

Diapolo commented Apr 10, 2012

Sign message would be a tool, too then ;). So why not create a new tools point, which holds your console and the sign message dialog?

@laanwj
Copy link
Member Author

laanwj commented Apr 11, 2012

Rebased, added build date to info window

@laanwj
Copy link
Member Author

laanwj commented Apr 11, 2012

New commit moved the option to the "Help" menu (until we have something better) and adds explicit "Copy" context menu to console.

@gavinandresen
Copy link
Contributor

Nifty. Tagged for 0.7.

@laanwj
Copy link
Member Author

laanwj commented Apr 22, 2012

Rebased, I've been able to structure the code better because of the recent RPC refactorings:

  • Added a execute() method to CRPCTable.
  • Factored out function RPCConvertValues() to convert parameter values for RPC call from list of strings to command-specific JSON objects.
  • LocalRPC is completely gone (and with it, most changes to the core), no more code additions to bitcoinrpc.cpp. This is now integrated into the Qt code, making use of CRPCTable::execute.

void setClientModel(ClientModel *model);

enum MessageClass {
ERROR,
Copy link
Member

Choose a reason for hiding this comment

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

This (and probably the next one) are #defined somewhere on Windows, breaking the build. Perhaps call it MC_ERROR, if there's really a need for 2 different ERROR values (this and CMD_ERROR). eg, a0031e5

Copy link
Member Author

Choose a reason for hiding this comment

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

Right... Is there a list of the namespace-polluting crap macros that windows dumps into programs somewhere?

And yes, ERROR and REQ_ERROR are different, the first category is meant for general critical errors the other is simply a failed RPC command.

@gmaxwell
Copy link
Contributor

gmaxwell commented May 5, 2012

Pretty awesome. Should this go under a tools menu or should it really be burried under help->debug window->console tab?

Does everyone else really like the nested scrollbars you get? (type help a couple times) I'm concerned that this might confuse users that I'm trying to support "scroll down and you'll see it" "I'm scrolled all the way down!". It's also a bit annoying if you're running a command over and over again to see if something changes to have to scroll internally to the entry to see it. (e.g. run getinfo over and over looking for connections changing)

Also, the fact that you can't copy large chunks of commands is bad from a support perspective. Its helpful if people can say "I ran all these commands and got all these results".

Sending the stop RPC makes the UI lockup for a long time (30 seconds?) before eventually shutting down.

When I first bring up the console focus doesn't move to the text entry box. I have to click it. But it has a blinking cursor even before I do, I think the focus is on the output box.. so it doesn't seem responsive. This only happens the first time after startup. After that it seems to remember.

The green and grey boxes don't seem all that intuitive to me. I'd personally just put [request] [reply]. But I admit this this is mostly taste and you should probably ignore me.

Copy works differently in single element responses vs multi-element responses. E.g. type "getblockhash 5" copy the hash, then "getblock [paste]" and copy the first txn hash. If you do something that has the second behavior first you may believe that it's not possible to copy in the first case, because you don't get any incremental selection. I think ideally incremental selection should always work.

@laanwj
Copy link
Member Author

laanwj commented May 6, 2012

  • Eventually it makes sense to add a tools menu where Sign message and verify message should also go. However, it's hidden in the second tab for a good reason. Users should not stumble too easy on it so it's now a second tab of a dangerous-looking window.
  • I don't like the nested scrollbars either. However, the Qt QTableWidget doesn't like rows that are too tall (they break scrolling, as the y offset can only start at a whole cell). That's why tall cells get them.
  • Selecting and copying multiple cells could be added.
  • Yes it should probably focus the input box when the tab is selected.
  • Typing the "stop" RPC command hangs the UI for as long as it takes to shutdown: seems that the UI is not hidden in this case when it leaves the main loop (could be easily fixed, this problem is not related to the console but happens also when you shut down some other way). (fixed by Hide UI immediately after leaving the main loop #1209)
  • The green and grey boxes are categories, they should go together with filtering. I intend to add buttons at the top with the same boxes to hide/show them (I also want to add normal debug/error messages). I'm open to suggestions as to the colors, or even icons (boxes are placeholders), but I don't want to much discussion about this.

IMO given that there are no critical issues we should merge this as soon as possible. It's better to have something, and it is functionality only a very small part of people will use, so perfecting it is not the foremost priority.

@gmaxwell
Copy link
Contributor

gmaxwell commented May 6, 2012

ha. I didn't mean to imply it shouldn't be pulled because of any of that. I just sat down and ran through it and what I wrote is just a braindump what I found. Considering that its a GUI feature, the fact that I actually tried to break it shows how excited I am to get it in. :)

However, it shouldn't be pulled this second because we may cut a 0.6.2 to take the addrman crash fix.

@laanwj
Copy link
Member Author

laanwj commented May 6, 2012

I agree, thanks for testing!

@luke-jr
Copy link
Member

luke-jr commented May 6, 2012

Object::connect: No such signal ClientModel::numBlocksChanged(int) in src/qt/rpcconsole.cpp:161
Object::connect: (receiver name: 'RPCConsole')

(this is with next-test, so maybe an undetected merge conflict)

@laanwj
Copy link
Member Author

laanwj commented May 6, 2012

Yes, that's an undetected merge conflict. numBlocksChanged changed from one to two arguments in #1205.

@grue0
Copy link

grue0 commented May 7, 2012

DO WANT!

On a side note, does it show all debug messages, or just RPC requests?

@gmaxwell
Copy link
Contributor

gmaxwell commented May 7, 2012

Oh, gruez has a nice point. This needs a debug log viewer tab too. :) Getting random windows users to find the debug log is an amazing PITA.

@Diapolo
Copy link

Diapolo commented May 7, 2012

Windows users are ticking a little different, don't blame them ^^. But ACK to the debug viewer idea, perhaps in a seperate commit, to not bloat this one?

@gmaxwell
Copy link
Contributor

gmaxwell commented May 7, 2012

I don't blame them— I'm very happy they use Bitcoin. But finding files in the file system and viewing them with wordpad isn't something most windows apps ask people to do, so they don't know how to do it. And because I know nothing about windows its very hard for me to talk them through it. A log viewer in bitcoin is something I could bring up locally. :) And sure, if I'm not doing the work I'm in no position to ask about which commits it gets placed in. ;)

@laanwj
Copy link
Member Author

laanwj commented May 7, 2012

I'm not going to add a full log viewer. There's zillions of those. That problem has been solved by grumpy system admins way before my time. Better to just implement logging to the OS log, if you want that, so you can use your operating system's log viewer.

However, it was my intention to show the last N debug and error messages for debugging, eventually (not necessarily in this commit).

@laanwj
Copy link
Member Author

laanwj commented May 7, 2012

Maybe we're thinking too difficult: on windows, we could just add a button to the debug window that opens debug.log in a wordpad?

@gmaxwell
Copy link
Contributor

gmaxwell commented May 7, 2012

::facepalm:: "The Russians Used a Pencil"

gmaxwell added a commit that referenced this pull request May 8, 2012
Add UI RPC console / debug window
@gmaxwell gmaxwell merged commit fa8cc47 into bitcoin:master May 8, 2012
@laanwj laanwj mentioned this pull request May 9, 2012
coblee pushed a commit to litecoin-project/litecoin that referenced this pull request Jul 17, 2012
@laanwj laanwj deleted the 2012_04_consoleui branch April 9, 2014 14:14
msgilligan added a commit to msgilligan/omnicore that referenced this pull request Oct 31, 2014
sanch0panza pushed a commit to sanch0panza/bitcoin that referenced this pull request May 17, 2018
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Dec 25, 2019
…ressbook is empty

fcc9e7d [Model[GUI] getLastUnusedMethod method name refactored up until the full proper functionality gets coded. (furszy)
fe5ce60 [Model][UI] Receive dialog, create an address if the addressbook is empty and/or inform the user and don't popup the dialog if the creation fails. * getLastUnusedAddress, IsValid double check added just to be sure before present it in the UI. (furszy)

Pull request description:

  Receive dialog, create address if addressbook is empty and/or inform the user and don't popup the dialog if the creation fails.

  * getLastUnusedAddress, IsValid double check added just to be sure before present it in the UI.

ACKs for top commit:
  random-zebra:
    utACK fcc9e7d
  Warrows:
    ACK fcc9e7d

Tree-SHA512: 61f09b6cee9163f63a95c387787a0ea925ad0a7bb668e87951ca7dfed795e167d974a7000e8bf63ab3ea36398ab551e5b65bf6ed7af1093dfc2027abec2881ce
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants