-
Notifications
You must be signed in to change notification settings - Fork 38.7k
[QT] pretty print (indent) multiline html output #6781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[QT] pretty print (indent) multiline html output #6781
Conversation
|
Looks good. Tested ACK 0daa74b. Maybe the font should be set to monospace as well in this PR. |
|
Thanks @jonasschnelli! Looks good to me, now it has a similar behavior as the console. @MarcoFalke: the font was changed earlier this year, see #5898. Personally I find it pretty readable how it is. |
|
@dexX7 Pretty print without monospace is very limited, though. |
|
Sorry, my bad! Now I see where you were going. On the first glimpse I assumed the help message was just badly formatted, but the messed up indentation is caused by the font. Currently (with this PR): When reverting #5898: |
|
ACK, nice work. |
|
Hmm... we could set the monospace font for json responses only? Or would this look ugly? |
|
No for json only. What about something like diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index f387a3e..396fe54 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -465,6 +465,7 @@ void RPCConsole::clear()
ui->messagesWidget->document()->setDefaultStyleSheet(
"table { }"
"td.time { color: #808080; padding-top: 3px; } "
+ "td.message { font-family: monospace; }"
"td.cmd-request { color: #006060; } "
"td.cmd-error { color: red; } "
"b { color: #006060; } "This will cause the font to be too large in Ubuntu? |
|
Hm.. so as far as I can see a monospaced font is desired, but the issue was that the default on Ubuntu was too big, and Maybe it would be possible to retrieve the default font size and use this, or an adjusted size based on this size, for the stylesheet, assuming the default size equals E.g.: // 1.0 em of default font:
QFont().pointSize()
// 0.8 em of the Monospace font:
QFont("Monospace").pointSize() * 4 / 5(
To compare without adjusted size, but I'm not sure how this plays out for other OS though, and slightly related: Edit: this is what I tested: https://gist.github.com/dexX7/218beb74246a1b5f0a6f |
|
I also like the monospace look in Ubuntu. But I think this is controversial (IIRC there are also HiDPI issues) and would recommend to address it in a different PR. |
|
It is difficult, because this PR migrates the look from bad to less bad. But once finally solved (using proper font), it is required anyway, so ACK. |
|
I think this looks fine. I don't think monospace/fixed is necessary, and I remember some problems with those in the past. |
|
utACK |
|
Does #6864 make this unnecessary? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's slightly unexpected for this to be part of the responsibility of a function called HtmlEscape. Especially to do this only on fMultiLine - there's nothing inherently multiline about replacing spaces.
If this is still necessary with monospace - maybe add a separate flag for this step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related: I suppose that changing the spaces to non-breaking ones cause problems when pasting the output into a json file and trying to parse it with a json parser? E.g. according to RFC4627, %xa0 is not a valid whitespace character:
ws = *(
%x20 / ; Space
%x09 / ; Horizontal tab
%x0A / ; Line feed or New line
%x0D ; Carriage return
)
(not saying this is a huge issue, not many people will likely do this, but it's something to keep in mind when considering presentation versus representation)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we achieve the same, non-collapsing spaces, using css' whitespace property? http://www.w3schools.com/cssref/pr_text_white-space.asp . Or is this not supported by Qt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Closing this one then |




Before:

After:

If the window is to small it will result in line break without indent (as in console):
