Fix suggestion includes fenced code blocks #1017
Conversation
service/github/github.go
Outdated
| backticks := countBackticks(txt) + 1 | ||
| if backticks < 3 { | ||
| // At least three backticks are required. | ||
| // https://github.github.com/gfm/#fenced-code-blocks | ||
| // > A code fence is a sequence of at least three consecutive backtick characters (`) or tildes (~). (Tildes and backticks cannot be mixed.) | ||
| backticks = 3 | ||
| } |
There was a problem hiding this comment.
Can you create a helper function to get # of backticks for suggestions considering there are duplicated code?
You can move the comment to that function as well.
service/github/github.go
Outdated
| sb.WriteString("```suggestion\n") | ||
| if txt := s.GetText(); txt != "" { | ||
| sb.Grow(backticks + len("suggestion\n") + len(txt) + len("\n") + backticks) | ||
| for i := 0; i < backticks; i++ { |
There was a problem hiding this comment.
How about creating a short helper function to write n backtics?
Co-authored-by: haya14busa <haya14busa@gmail.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Thank you for your review.
I created it in the commentutil package. |
| sb.WriteString("```suggestion\n") | ||
| if txt := s.GetText(); txt != "" { | ||
| sb.Grow(backticks + len("suggestion\n") + len(txt) + len("\n") + backticks) | ||
| commentutil.WriteCodeFence(&sb, backticks) |
There was a problem hiding this comment.
I think this warning is false positive.
We don't need to check return values because the methods of strings.Builder always return nil error.
https://pkg.go.dev/strings#Builder.Write
Write appends the contents of p to b's buffer. Write always returns len(p), nil.
https://pkg.go.dev/strings#Builder.WriteByte
WriteByte appends the byte c to b's buffer. The returned error is always nil.
The return values exist only to satisfy well-known interfaces such as io.Writer, io.ByteWriter, etc.
Should I suppress the warning?
| commentutil.WriteCodeFence(&sb, backticks) | |
| _ = commentutil.WriteCodeFence(&sb, backticks) |
There was a problem hiding this comment.
ok, sounds good. You can leave it as is.
| sb.WriteString(txt) | ||
| sb.WriteString("\n") | ||
| } | ||
| sb.WriteString("```") | ||
| commentutil.WriteCodeFence(&sb, backticks) |
| sb.WriteString(endLineContent[max(end.GetColumn()-1, 0):]) | ||
| sb.WriteString("\n```") | ||
| sb.Grow(backticks + len("suggestion\n") + len(txt) + len("\n") + backticks) | ||
| commentutil.WriteCodeFence(&sb, backticks) |
| sb.WriteString("```suggestion\n") | ||
| if txt := s.GetText(); txt != "" { | ||
| sb.Grow(backticks + len("suggestion\n") + len(txt) + len("\n") + backticks) | ||
| commentutil.WriteCodeFence(&sb, backticks) |
There was a problem hiding this comment.
ok, sounds good. You can leave it as is.
When reviewdog suggests the following code
modified as
the suggestion comment should be
but actually, reviewdog suggests the following comment:
This is because the markdown parser creates wrong pairs of code fences.
The document of GitHub markdown says that it is needed to wrap them inside quadruple backticks in this case.
https://docs.github.com/en/github/writing-on-github/working-with-advanced-formatting/creating-and-highlighting-code-blocks#fenced-code-blocks
This pull request checks whether the suggestion includes code fences, and use quadruple backticks code fences if it is necessary.
Fixes #999