-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Preserve white space in text plugin. #6833
Preserve white space in text plugin. #6833
Conversation
@@ -46,6 +46,9 @@ class TfMarkdownView extends LegacyElementMixin(PolymerElement) { | |||
#markdown > p:last-child { | |||
margin-bottom: 0.3em; | |||
} | |||
#markdown p { | |||
white-space: pre; |
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.
This will have the unintended consequence of meaning that actual paragraphs of text will not be wrapped in a sane manner.
For the use case you are investigating, I think we should understand why the markdown renderer is wrapping some text with <pre><code></code></pre>
and other code with just <p>
.
I suspect it has to do with the non alpha-numeric text included in the results but that is just speculation.
Markdown Rendering is here:
https://github.com/tensorflow/tensorboard/blob/master/tensorboard/plugin_util.py
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.
Thanks Brian, that's a good point. I'll look into that thing from the markdown converter until, but also, how about using white-space: break-spaces;
instead? Looks like this pretty much solves the problem by allowing text to wrap, but preserving spaces: https://developer.mozilla.org/en-US/docs/Web/CSS/white-space
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.
That result with the <pre>
tags seems to come from the markdown library. I don't think it's worth trying to figure out how that works. I'm more inclined to just go with the white-space: break-spaces;
solution. If users have an issue with this, we can add a checkbox control later, similar to the "markdown" one, tho I imagine most users would want to see the spaces if they're there.
@@ -46,6 +46,9 @@ class TfMarkdownView extends LegacyElementMixin(PolymerElement) { | |||
#markdown > p:last-child { | |||
margin-bottom: 0.3em; | |||
} | |||
#markdown p { | |||
white-space: break-spaces; |
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.
Mind adding a comment or just a link to the bug (imo its ok that the bug is internal only)?
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.
Done.
Some users require the spaces to be preserved in the text shown in the text dashboard. At the moment I can't think of a reason why not to enable this unconditionally. I imagine anybody interested in seeing text logged would be interested in the value *verbatim*, including spaces. Googlers, see b/335770352
Some users require the spaces to be preserved in the text shown in the text dashboard.
At the moment I can't think of a reason why not to enable this unconditionally. I imagine anybody interested in seeing text logged would be interested in the value verbatim, including spaces.
Googlers, see b/335770352