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
markdown: replace new line in usage #33
Conversation
a9a3efb
to
cf2a573
Compare
clidocstool_md.go
Outdated
@@ -227,3 +228,7 @@ func mdCmdOutput(cmd *cobra.Command, old string) (string, error) { | |||
func mdEscapePipe(s string) string { | |||
return strings.ReplaceAll(s, `|`, `\|`) | |||
} | |||
|
|||
func mdReplaceNewline(s string) string { | |||
return regexp.MustCompile(`\r\n?|\n`).ReplaceAllString(s, "<br>") |
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.
Better to put the MustCompile
either in a sync.Once
or as a package var
.
The regex probably should be making the \r
optional, and not the \n
(\r
newlines are AFAIK only occurring on classic macOS);
return regexp.MustCompile(`\r\n?|\n`).ReplaceAllString(s, "<br>") | |
return regexp.MustCompile(`\r?\n`).ReplaceAllString(s, "<br>") |
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.
Oh; is XHTML still a thing nowadays? (I still have the tendency to use <br />
, but perhaps we don't really care about that)
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.
Oh; is XHTML still a thing nowadays? (I still have the tendency to use
<br />
, but perhaps we don't really care about that)
<br>
is enough imo as markdown content is not served as application/xhtml+xml
anyway (considered harmful). Also with HTML5 even if you write <br/>
or <br />
it will eventually be converted to <br>
by the browser.
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.
considered harmful
Heh, yes, a lot of that was written when Internet Explorer was still "the largest browser", and, well, a lot of fun was needed in either case to write HTML that worked both in IE, and in browsers that were actually following standards 😂
But yeah, we don't do anything with XHTML(5), so 😂
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
cf2a573
to
6de4bc9
Compare
Codecov ReportBase: 64.36% // Head: 64.49% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #33 +/- ##
==========================================
+ Coverage 64.36% 64.49% +0.12%
==========================================
Files 4 4
Lines 550 552 +2
==========================================
+ Hits 354 356 +2
Misses 138 138
Partials 58 58
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
LGTM, thanks!
follow-up docker/cli#3924 (comment)
Signed-off-by: CrazyMax crazy-max@users.noreply.github.com