Fix blank lines and indentation in every diff row

eb84db3 · Gabriel Morell · 2026-09-09 20:43

2 files +64 -8
Message
{commit_body(@commit)}

Files changed

modified lib/git_gud_web/components/diff_components.ex
+9 −6
@@ -156,9 +156,9 @@ defmodule GitGudWeb.DiffComponents do
156 156 <td class="text-right px-2 py-0.5 opacity-50 select-none w-12 border-r border-base-300">
157 157 {line.new_line}
158 158 </td>
159 <td class="px-2 py-0.5 whitespace-pre-wrap break-all">
160 <span class="opacity-60 select-none mr-1">{prefix_char(line.type)}</span>{highlight_line(line.text, @file.path, @theme)}
161 </td>
159 + <%!-- `whitespace-pre-wrap` renders the template's own
160 + indentation, so the cell's contents must hug the tags. --%>
161 + <td class="px-2 py-0.5 whitespace-pre-wrap break-all" phx-no-format><span class="opacity-60 select-none mr-1">{prefix_char(line.type)}</span>{highlight_line(line.text, @file.path, @theme)}</td>
162 162 </tr>
163 163 </tbody>
164 164 </table>
@@ -176,9 +176,11 @@ defmodule GitGudWeb.DiffComponents do
176 176 |> Phoenix.HTML.raw()
177 177 end
178 178
179 # Lumis wraps the output in `<pre class="lumis"><code>...</code></pre>`
180 # — strip both so the result drops inline into our diff cell without
181 # double-pre styling.
179 + # Lumis wraps the output in
180 + # `<pre class="lumis"><code><div class="line">...\n</div></code></pre>`
181 + # — strip all three so the result drops inline into our diff cell. The
182 + # trailing newline has to go too: the cell is `whitespace-pre-wrap`, so
183 + # it would render as a blank second line in every row.
182 184 defp strip_pre_wrapper(html) when is_binary(html) do
183 185 html
184 186 |> String.replace(~r/^<pre[^>]*>/, "")
@@ -187,6 +189,7 @@ defmodule GitGudWeb.DiffComponents do
187 189 |> String.replace(~r/<\/code>$/, "")
188 190 |> String.replace(~r/^<div[^>]*>/, "")
189 191 |> String.replace(~r/<\/div>\s*$/, "")
192 + |> String.replace(~r/\n+$/, "")
190 193 end
191 194
192 195 defp row_class(:add),
modified test/git_gud_web/components/diff_components_test.exs
+55 −2
@@ -22,6 +22,30 @@ defmodule GitGudWeb.DiffComponentsTest do
22 22 }
23 23 end
24 24
25 + defp render_lines(lines) do
26 + file = %{
27 + path: ".gitignore",
28 + old_path: nil,
29 + status: :modified,
30 + binary?: false,
31 + insertions: 1,
32 + deletions: 0,
33 + hunks: [%{header: "@@ -36,3 +36,6 @@", lines: lines}]
34 + }
35 +
36 + render_component(&DiffComponents.diff/1, files: [file], theme: "onedark")
37 + end
38 +
39 + defp cell_texts(html) do
40 + html
41 + |> LazyHTML.from_fragment()
42 + |> LazyHTML.query("td.whitespace-pre-wrap")
43 + |> Enum.map(&LazyHTML.text/1)
44 + end
45 +
46 + defp line(type, text, old, new),
47 + do: %{type: type, text: text, old_line: old, new_line: new}
48 +
25 49 describe "default_loaded/1" do
26 50 test "small diff preloads everything" do
27 51 assert MapSet.equal?(DiffComponents.default_loaded([file(0), file(1)]), MapSet.new([0, 1]))
@@ -44,13 +68,19 @@ defmodule GitGudWeb.DiffComponentsTest do
44 68
45 69 describe "expand/2" do
46 70 test "adds the clicked index" do
47 socket = %Phoenix.LiveView.Socket{assigns: %{loaded_diff_idx: MapSet.new([0]), __changed__: %{}}}
71 + socket = %Phoenix.LiveView.Socket{
72 + assigns: %{loaded_diff_idx: MapSet.new([0]), __changed__: %{}}
73 + }
74 +
48 75 socket = DiffComponents.expand(socket, %{"idx" => "3"})
49 76 assert MapSet.member?(socket.assigns.loaded_diff_idx, 3)
50 77 end
51 78
52 79 test "garbage idx is a no-op" do
53 socket = %Phoenix.LiveView.Socket{assigns: %{loaded_diff_idx: MapSet.new([0]), __changed__: %{}}}
80 + socket = %Phoenix.LiveView.Socket{
81 + assigns: %{loaded_diff_idx: MapSet.new([0]), __changed__: %{}}
82 + }
83 +
54 84 socket2 = DiffComponents.expand(socket, %{"idx" => "not-a-number"})
55 85 assert socket2.assigns.loaded_diff_idx == MapSet.new([0])
56 86 end
@@ -99,4 +129,27 @@ defmodule GitGudWeb.DiffComponentsTest do
99 129 assert html =~ "No changes"
100 130 end
101 131 end
132 +
133 + describe "line cells" do
134 + test "a row holds exactly one line of content" do
135 + text = render_lines([line(:add, "/priv/uploads/", nil, 39)]) |> cell_texts() |> hd()
136 +
137 + # The cell is `whitespace-pre-wrap`: any stray newline — from the
138 + # template's own indentation, or the per-line wrapper the
139 + # highlighter emits — becomes a blank line in every single row.
140 + refute text =~ "\n"
141 + assert text == "+/priv/uploads/"
142 + end
143 +
144 + test "context and deletion rows carry their marker with no padding" do
145 + html = render_lines([line(:ctx, "plans", 38, 38), line(:del, "old thing", 40, nil)])
146 +
147 + # The delete marker is a real minus sign (U+2212), not a hyphen.
148 + assert cell_texts(html) == [" plans", "−old thing"]
149 + end
150 +
151 + test "an empty added line renders no content at all" do
152 + assert render_lines([line(:add, "", nil, 39)]) |> cell_texts() == ["+"]
153 + end
154 + end
102 155 end

Parents: 4b717b0