Fix review excerpt

[confusedsushi]

Currently the "File Changed" tab of a PR is somehow broken. This is also true for the current release 1.16.0.

When you are on the "File Changed" tab, and want to look at code excerpt before or after the code changes, the layout breaks. You can test this on try.gitea.io here: https://try.gitea.io/testnotexisting/magic_enum/pulls/2/files

The problem occurs for the unified view and for the split view.

Kind of the same problem was there for commenting a line of code, this was fixed in #18321 and #18403.

For consistency, I changed the solution of #18321, I removed the colspan and instead added a <td>. The goal was to have code similarly with the split view.

Also the separator line in the split view was in the wrong column, this was fixed too.

[lunny]

Fix #18516 and #18517?

[silverwind]

@zeripath could you verify this is the correct fix?

[confusedsushi]

Yes, this is fixing #18516.

#18517 is something different.

[zeripath]

Sorry for another bug here. The diff code is remarkably complex and fiddly.

I've fixed this so that the escaped warning triangle appears on the excerpted lines too.

[zeripath]

@silverwind I think this is now fixed.

I don't understand why the colspan in the js has been switched to three separate columns in the js - because that is not related to these code changes but I guess it might just help with keeping the same rendering so I've left it.

[silverwind]

JS should probably not create DOM elements from scratch but clone existing ones and alter them. That way, a whole class of possible errors is eliminated :)

[confusedsushi]

@silverwind I think this is now fixed.

I don't understand why the colspan in the js has been switched to three separate columns in the js - because that is not related to these code changes but I guess it might just help with keeping the same rendering so I've left it.

I wanted to have the similarity to the table layout of other rows. There you have line-number-old | line-number-new it was before collapsed with the colspan.

With your changes now, there is again one line number column missing.

[zeripath]

Show me I can't see how that could be happening.

This is the patch between my changes and c090808

diff --git a/templates/repo/diff/blob_excerpt.tmpl b/templates/repo/diff/blob_excerpt.tmpl
index c20e2c8b6..9bf0a7f99 100644
--- a/templates/repo/diff/blob_excerpt.tmpl
+++ b/templates/repo/diff/blob_excerpt.tmpl
@@ -21,25 +21,18 @@
 			</td>
 			<td colspan="5" class="lines-code lines-code-old ">{{$inlineDiff := $.section.GetComputedInlineDiffFor $line}}<code {{if $inlineDiff.EscapeStatus.Escaped}}class="code-inner has-escaped" title="{{$.i18n.Tr "repo.line_unicode"}}"{{else}}class="code-inner"{{end}}>{{$inlineDiff.Content}}</code></td>
 		{{else}}
+			{{$inlineDiff := $.section.GetComputedInlineDiffFor $line}}
 			<td class="lines-num lines-num-old" data-line-num="{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}"><span rel="{{if $line.LeftIdx}}diff-{{Sha1 $.fileName}}L{{$line.LeftIdx}}{{end}}"></span></td>
-			<td class="blob-excerpt lines-escape lines-escape-old"></td>
+			<td class="blob-excerpt lines-escape lines-escape-old">{{if and $line.LeftIdx $inlineDiff.EscapeStatus.Escaped}}<a href="" class="toggle-escape-button" title="{{$.i18n.Tr "repo.line_unicode"}}"></a>{{end}}</td>
 			<td class="blob-excerpt lines-type-marker lines-type-marker-old">{{if $line.LeftIdx}}<span class="mono" data-type-marker=""></span>{{end}}</td>
 			<td class="blob-excerpt lines-code lines-code-old halfwidth">{{/*
-				*/}}{{if $line.LeftIdx}}{{/*
-						*/}}{{$inlineDiff := $.section.GetComputedInlineDiffFor $line}}<code {{if $inlineDiff.EscapeStatus.Escaped}}class="code-inner has-escaped" title="{{$.i18n.Tr "repo.line_unicode"}}"{{else}}class="code-inner"{{end}}>{{$inlineDiff.Content}}</code>{{/*
-					*/}}{{else}}{{/*
-						*/}}<code class="code-inner"></code>{{/*
-					*/}}{{end}}{{/*
-				*/}}</td>
+				*/}}<code {{if and $line.LeftIdx $inlineDiff.EscapeStatus.Escaped}}class="code-inner has-escaped" title="{{$.i18n.Tr "repo.line_unicode"}}"{{else}}class="code-inner"{{end}}>{{if $line.LeftIdx}}{{$inlineDiff.Content}}{{end}}</code>{{/*
+			*/}}</td>
 			<td class="lines-num lines-num-new" data-line-num="{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}"><span rel="{{if $line.RightIdx}}diff-{{Sha1 $.fileName}}R{{$line.RightIdx}}{{end}}"></span></td>
-			<td class="blob-excerpt lines-escape lines-escape-new"></td>
+			<td class="blob-excerpt lines-escape lines-escape-new">{{if and $line.RightIdx $inlineDiff.EscapeStatus.Escaped}}<a href="" class="toggle-escape-button" title="{{$.i18n.Tr "repo.line_unicode"}}"></a>{{end}}</td>
 			<td class="blob-excerpt lines-type-marker lines-type-marker-new">{{if $line.RightIdx}}<span class="mono" data-type-marker=""></span>{{end}}</td>
 			<td class="blob-excerpt lines-code lines-code-new halfwidth">{{/*
-				*/}}{{if $line.RightIdx}}{{/*
-					*/}}{{$inlineDiff := $.section.GetComputedInlineDiffFor $line}}<code {{if $inlineDiff.EscapeStatus.Escaped}}class="code-inner has-escaped" title="{{$.i18n.Tr "repo.line_unicode"}}"{{else}}class="code-inner"{{end}}>{{$inlineDiff.Content}}</code>{{/*
-				*/}}{{else}}{{/*
-					*/}}<code class="code-inner"></code>{{/*
-				*/}}{{end}}{{/*
+				*/}}<code {{if and $line.RightIdx $inlineDiff.EscapeStatus.Escaped}}class="code-inner has-escaped" title="{{$.i18n.Tr "repo.line_unicode"}}"{{else}}class="code-inner"{{end}}>{{if $line.RightIdx}}{{$inlineDiff.Content}}{{end}}</code>{{/*
 			*/}}</td>
 		{{end}}
 	</tr>
@@ -69,9 +62,10 @@
 			<td class="lines-num lines-num-old" data-line-num="{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}"><span rel="{{if $line.LeftIdx}}diff-{{Sha1 $.fileName}}L{{$line.LeftIdx}}{{end}}"></span></td>
 			<td class="lines-num lines-num-new" data-line-num="{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}"><span rel="{{if $line.RightIdx}}diff-{{Sha1 $.fileName}}R{{$line.RightIdx}}{{end}}"></span></td>
 		{{end}}
-		<td class="blob-excerpt lines-escape"></td>
+		{{$inlineDiff := $.section.GetComputedInlineDiffFor $line}}
+		<td class="blob-excerpt lines-escape">{{if $inlineDiff.EscapeStatus.Escaped}}<a href="" class="toggle-escape-button" title="{{$.i18n.Tr "repo.line_unicode"}}"></a>{{end}}</td>
 		<td class="blob-excerpt lines-type-marker"><span class="mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span></td>
-		<td class="blob-excerpt lines-code{{if (not $line.RightIdx)}} lines-code-old{{end}}">{{$inlineDiff := $.section.GetComputedInlineDiffFor $line}}<code {{if $inlineDiff.EscapeStatus.Escaped}}class="code-inner has-escaped" title="{{$.i18n.Tr "repo.line_unicode"}}"{{else}}class="code-inner"{{end}}>{{$inlineDiff.Content}}</code></td>
+		<td class="blob-excerpt lines-code{{if (not $line.RightIdx)}} lines-code-old{{end}}"><code {{if $inlineDiff.EscapeStatus.Escaped}}class="code-inner has-escaped" title="{{$.i18n.Tr "repo.line_unicode"}}"{{else}}class="code-inner"{{end}}>{{$inlineDiff.Content}}</code></td>
 	</tr>
 	{{end}}
 {{end}}

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (main@6f6b849). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main   #18502   +/-   ##
=======================================
  Coverage        ?   46.24%           
=======================================
  Files           ?      842           
  Lines           ?   121192           
  Branches        ?        0           
=======================================
  Hits            ?    56049           
  Misses          ?    58333           
  Partials        ?     6810           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f6b849...f63df6a. Read the comment docs.

[zeripath]

Please send backport