Commit c9357581fe
Verified · cmc ci/build: success ci/test: success
Layout: unified · split
internal/httpd/diff.go +31
| @@ -316,6 +316,8 @@ type splitRow struct { | |||
| 316 | Old *diffLine | 316 | Old *diffLine |
| 317 | New *diffLine | 317 | New *diffLine |
| 318 | Threads []diffThread | 318 | Threads []diffThread |
| 319 | OldNote string // "\ No newline" marker belonging to the old side | ||
| 320 | NewNote string // and to the new side | ||
| 319 | Compose *diffLine // the line whose new-thread form opens under this row | 321 | Compose *diffLine // the line whose new-thread form opens under this row |
| 320 | } | 322 | } |
| 321 | 323 | ||
| @@ -338,6 +340,14 @@ func splitFiles(files []diffFile) { | |||
| 338 | ln := &lines[i] | 340 | ln := &lines[i] |
| 339 | switch ln.Class { | 341 | switch ln.Class { |
| 340 | case "hunk", "meta": | 342 | case "hunk", "meta": |
| 343 | if ln.Class == "meta" && ln.Path == "" && len(rows) > 0 && strings.HasPrefix(ln.Text, `\`) { | ||
| 344 | // a marker after a context line: neither side ends in a newline | ||
| 345 | if last := &rows[len(rows)-1]; last.Old != nil && last.Old == last.New { | ||
| 346 | last.OldNote, last.NewNote = ln.Text, ln.Text | ||
| 347 | i++ | ||
| 348 | continue | ||
| 349 | } | ||
| 350 | } | ||
| 341 | rows = append(rows, splitRow{Kind: ln.Class, Text: ln.Text}) | 351 | rows = append(rows, splitRow{Kind: ln.Class, Text: ln.Text}) |
| 342 | i++ | 352 | i++ |
| 343 | case "ctx": | 353 | case "ctx": |
| @@ -349,18 +359,33 @@ func splitFiles(files []diffFile) { | |||
| 349 | i++ | 359 | i++ |
| 350 | default: | 360 | default: |
| 351 | var dels, adds []*diffLine | 361 | var dels, adds []*diffLine |
| 362 | marker := func() string { | ||
| 363 | if i < len(lines) && lines[i].Class == "meta" && strings.HasPrefix(lines[i].Text, `\`) { | ||
| 364 | i++ | ||
| 365 | return lines[i-1].Text | ||
| 366 | } | ||
| 367 | return "" | ||
| 368 | } | ||
| 369 | var oldNote, newNote string | ||
| 352 | for i < len(lines) && lines[i].Class == "del" { | 370 | for i < len(lines) && lines[i].Class == "del" { |
| 353 | dels = append(dels, &lines[i]) | 371 | dels = append(dels, &lines[i]) |
| 354 | i++ | 372 | i++ |
| 355 | } | 373 | } |
| 374 | if len(dels) > 0 { | ||
| 375 | oldNote = marker() | ||
| 376 | } | ||
| 356 | for i < len(lines) && lines[i].Class == "add" { | 377 | for i < len(lines) && lines[i].Class == "add" { |
| 357 | adds = append(adds, &lines[i]) | 378 | adds = append(adds, &lines[i]) |
| 358 | i++ | 379 | i++ |
| 359 | } | 380 | } |
| 381 | if len(adds) > 0 { | ||
| 382 | newNote = marker() | ||
| 383 | } | ||
| 360 | if len(dels)+len(adds) == 0 { | 384 | if len(dels)+len(adds) == 0 { |
| 361 | i++ // an unknown class: skip rather than loop | 385 | i++ // an unknown class: skip rather than loop |
| 362 | continue | 386 | continue |
| 363 | } | 387 | } |
| 388 | first := len(rows) | ||
| 364 | for k := 0; k < len(dels) || k < len(adds); k++ { | 389 | for k := 0; k < len(dels) || k < len(adds); k++ { |
| 365 | r := splitRow{Kind: "pair"} | 390 | r := splitRow{Kind: "pair"} |
| 366 | for _, l := range []*diffLine{pick(dels, k), pick(adds, k)} { | 391 | for _, l := range []*diffLine{pick(dels, k), pick(adds, k)} { |
| @@ -379,6 +404,12 @@ func splitFiles(files []diffFile) { | |||
| 379 | } | 404 | } |
| 380 | rows = append(rows, r) | 405 | rows = append(rows, r) |
| 381 | } | 406 | } |
| 407 | if len(dels) > 0 { | ||
| 408 | rows[first+len(dels)-1].OldNote = oldNote | ||
| 409 | } | ||
| 410 | if len(adds) > 0 { | ||
| 411 | rows[first+len(adds)-1].NewNote = newNote | ||
| 412 | } | ||
| 382 | } | 413 | } |
| 383 | } | 414 | } |
| 384 | files[f].Rows = rows | 415 | files[f].Rows = rows |
internal/httpd/difflayout_test.go +40
| @@ -160,3 +160,43 @@ func TestDiffLayoutForQueryOverridesAccount(t *testing.T) { | |||
| 160 | t.Error("?layout=split ignored for a signed-out reader") | 160 | t.Error("?layout=split ignored for a signed-out reader") |
| 161 | } | 161 | } |
| 162 | } | 162 | } |
| 163 | |||
| 164 | func splitPairs(patch string) []splitRow { | ||
| 165 | files := parseDiff(patch) | ||
| 166 | splitFiles(files) | ||
| 167 | var out []splitRow | ||
| 168 | for _, r := range files[0].Rows { | ||
| 169 | out = append(out, r) | ||
| 170 | } | ||
| 171 | return out | ||
| 172 | } | ||
| 173 | |||
| 174 | const noNL = `\ No newline at end of file` | ||
| 175 | |||
| 176 | func TestSplitNoNewlineMarkerPairs(t *testing.T) { | ||
| 177 | head := "diff --git a/a.txt b/a.txt\n--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n" | ||
| 178 | cases := map[string]struct { | ||
| 179 | patch, oldNote, newNote string | ||
| 180 | }{ | ||
| 181 | "newline added": {head + "-foo\n" + noNL + "\n+foo\n", noNL, ""}, | ||
| 182 | "newline removed": {head + "-foo\n+foo\n" + noNL + "\n", "", noNL}, | ||
| 183 | "both lack": {head + "-foo\n" + noNL + "\n+bar\n" + noNL + "\n", noNL, noNL}, | ||
| 184 | } | ||
| 185 | for name, c := range cases { | ||
| 186 | rows := splitPairs(c.patch) | ||
| 187 | if len(rows) != 2 || rows[1].Kind != "pair" || rows[1].Old == nil || rows[1].New == nil { | ||
| 188 | t.Errorf("%s: want a hunk and one paired row, got %+v", name, rows) | ||
| 189 | continue | ||
| 190 | } | ||
| 191 | if rows[1].OldNote != c.oldNote || rows[1].NewNote != c.newNote { | ||
| 192 | t.Errorf("%s: notes %q %q", name, rows[1].OldNote, rows[1].NewNote) | ||
| 193 | } | ||
| 194 | } | ||
| 195 | } | ||
| 196 | |||
| 197 | func TestSplitEmptyCellsAreHiddenFromAssistiveTech(t *testing.T) { | ||
| 198 | out := renderCommitDiff(t, diffLayout{Split: true, UnifiedURL: "/x", SplitURL: "/y"}, nil) | ||
| 199 | if !strings.Contains(out, `class="ln none" aria-hidden="true"`) || !strings.Contains(out, `class="src none" aria-hidden="true"`) { | ||
| 200 | t.Error("empty cells are not aria-hidden") | ||
| 201 | } | ||
| 202 | } | ||
internal/web/static/style.css +2
| @@ -1411,9 +1411,11 @@ table.difftable.split tr.threadrow td { overflow-wrap: anywhere; } | |||
| 1411 | table.difftable.split td.ln:target, | 1411 | table.difftable.split td.ln:target, |
| 1412 | table.difftable.split td.ln:target + td { background: color-mix(in srgb, var(--mark) 18%, transparent); } | 1412 | table.difftable.split td.ln:target + td { background: color-mix(in srgb, var(--mark) 18%, transparent); } |
| 1413 | table.difftable.split td.ln:target { border-left: 3px solid var(--mark); } | 1413 | table.difftable.split td.ln:target { border-left: 3px solid var(--mark); } |
| 1414 | table.difftable.split .nonl { display: block; color: var(--muted); font-style: italic; } | ||
| 1414 | p.layoutpick { color: var(--muted); font-size: var(--fs-2); margin: 0 0 var(--sp-3); } | 1415 | p.layoutpick { color: var(--muted); font-size: var(--fs-2); margin: 0 0 var(--sp-3); } |
| 1415 | p.layoutpick strong { color: var(--fg); font-weight: 600; } | 1416 | p.layoutpick strong { color: var(--fg); font-weight: 600; } |
| 1416 | @media (max-width: 64rem) { | 1417 | @media (max-width: 64rem) { |
| 1418 | p.layoutpick { display: none; } | ||
| 1417 | table.difftable.split, table.difftable.split tbody { display: block; } | 1419 | table.difftable.split, table.difftable.split tbody { display: block; } |
| 1418 | table.difftable.split colgroup { display: none; } | 1420 | table.difftable.split colgroup { display: none; } |
| 1419 | table.difftable.split tr.pair { display: grid; grid-template-columns: auto 1fr; } | 1421 | table.difftable.split tr.pair { display: grid; grid-template-columns: auto 1fr; } |
internal/web/templates/layout.html +3 −3
| @@ -211,9 +211,9 @@ | |||
| 211 | {{else if $split}}<div class="tablewrap"><table class="difftable split"><colgroup><col class="cln"><col><col class="cln"><col></colgroup> | 211 | {{else if $split}}<div class="tablewrap"><table class="difftable split"><colgroup><col class="cln"><col><col class="cln"><col></colgroup> |
| 212 | {{range .Rows}}{{if eq .Kind "hunk"}}<tr class="hunk"><td class="ln"></td><td class="src" colspan="3">{{.Text}}</td></tr> | 212 | {{range .Rows}}{{if eq .Kind "hunk"}}<tr class="hunk"><td class="ln"></td><td class="src" colspan="3">{{.Text}}</td></tr> |
| 213 | {{else if eq .Kind "meta"}}<tr class="dmeta"><td class="ln"></td><td class="src" colspan="3">{{.Text}}</td></tr> | 213 | {{else if eq .Kind "meta"}}<tr class="dmeta"><td class="ln"></td><td class="src" colspan="3">{{.Text}}</td></tr> |
| 214 | {{else}}<tr class="pair"> | 214 | {{else}}{{$r := .}}<tr class="pair"> |
| 215 | {{with .Old}}{{if eq .Class "del"}}<td class="ln del" id="f{{$fi}}-o{{.OldLine}}">{{template "cmtln" dict "N" .OldLine "Path" .Path "Side" "old" "Base" $base "Viewer" $viewer "Carry" $L.Carry}}</td><td class="src del chroma">{{if .Code}}{{.Code}}{{else}}{{.Content}}{{end}}</td>{{else}}<td class="ln ctx old">{{.OldLine}}</td><td class="src ctx old chroma">{{if .Code}}{{.Code}}{{else}}{{.Content}}{{end}}</td>{{end}}{{else}}<td class="ln none"></td><td class="src none"></td>{{end}} | 215 | {{with .Old}}{{if eq .Class "del"}}<td class="ln del" id="f{{$fi}}-o{{.OldLine}}">{{template "cmtln" dict "N" .OldLine "Path" .Path "Side" "old" "Base" $base "Viewer" $viewer "Carry" $L.Carry}}</td><td class="src del chroma">{{if .Code}}{{.Code}}{{else}}{{.Content}}{{end}}{{with $r.OldNote}}<span class="nonl">{{.}}</span>{{end}}</td>{{else}}<td class="ln ctx old">{{.OldLine}}</td><td class="src ctx old chroma">{{if .Code}}{{.Code}}{{else}}{{.Content}}{{end}}{{with $r.OldNote}}<span class="nonl">{{.}}</span>{{end}}</td>{{end}}{{else}}<td class="ln none" aria-hidden="true"></td><td class="src none" aria-hidden="true"></td>{{end}} |
| 216 | {{with .New}}<td class="ln {{.Class}}" id="f{{$fi}}-n{{.NewLine}}">{{template "cmtln" dict "N" .NewLine "Path" .Path "Side" "new" "Base" $base "Viewer" $viewer "Carry" $L.Carry}}</td><td class="src {{.Class}} chroma">{{if .Code}}{{.Code}}{{else}}{{.Content}}{{end}}</td>{{else}}<td class="ln none"></td><td class="src none"></td>{{end}} | 216 | {{with .New}}<td class="ln {{.Class}}" id="f{{$fi}}-n{{.NewLine}}">{{template "cmtln" dict "N" .NewLine "Path" .Path "Side" "new" "Base" $base "Viewer" $viewer "Carry" $L.Carry}}</td><td class="src {{.Class}} chroma">{{if .Code}}{{.Code}}{{else}}{{.Content}}{{end}}{{with $r.NewNote}}<span class="nonl">{{.}}</span>{{end}}</td>{{else}}<td class="ln none" aria-hidden="true"></td><td class="src none" aria-hidden="true"></td>{{end}} |
| 217 | </tr> | 217 | </tr> |
| 218 | {{with .Compose}}<tr class="threadrow"><td colspan="4"><form id="compose" method="post" action="{{$base}}/diff-comment" class="thread composing"> | 218 | {{with .Compose}}<tr class="threadrow"><td colspan="4"><form id="compose" method="post" action="{{$base}}/diff-comment" class="thread composing"> |
| 219 | <input type="hidden" name="path" value="{{.Path}}"> | 219 | <input type="hidden" name="path" value="{{.Path}}"> |