GITGUD

neiam /gitgud

Git Gud

public · Issues · Pulls · Labels · Forks · Compare · Actions success · Packages

⭐

Log in to mark this repository.

Let reviewers comment on individual lines of a PR diff

98604fb · Gabriel Morell · 2026-09-09 22:07

3 files +340 -12
Message 
{commit_body(@commit)}

Files changed

modified lib/git_gud_web/components/diff_components.ex
+98 −11
@@ -38,6 +38,18 @@ defmodule GitGudWeb.DiffComponents do
38 38 attr :theme, :string, default: "onedark"
39 39 attr :loaded_idx, :any, default: nil
40 40
41 + attr :comments, :map,
42 + default: %{},
43 + doc: "Line comments keyed by `{path, line}`. Omit on views that don't have any."
44 +
45 + attr :can_comment?, :boolean,
46 + default: false,
47 + doc: "Show the per-line 'add comment' affordance."
48 +
49 + attr :commenting_on, :any,
50 + default: nil,
51 + doc: "`{path, line}` whose inline form is open, or nil."
52 +
41 53 def diff(assigns) do
42 54 loaded = assigns[:loaded_idx] || default_loaded(assigns.files)
43 55 assigns = assign(assigns, :loaded_idx, loaded)
@@ -51,6 +63,9 @@ defmodule GitGudWeb.DiffComponents do
51 63 theme={@theme}
52 64 idx={idx}
53 65 loaded?={MapSet.member?(@loaded_idx, idx)}
66 + comments={@comments}
67 + can_comment?={@can_comment?}
68 + commenting_on={@commenting_on}
54 69 />
55 70 </div>
56 71 """
@@ -98,6 +113,9 @@ defmodule GitGudWeb.DiffComponents do
98 113 attr :theme, :string, required: true
99 114 attr :idx, :integer, required: true
100 115 attr :loaded?, :boolean, default: true
116 + attr :comments, :map, default: %{}
117 + attr :can_comment?, :boolean, default: false
118 + attr :commenting_on, :any, default: nil
101 119
102 120 defp file_diff(assigns) do
103 121 ~H"""
@@ -131,7 +149,13 @@ defmodule GitGudWeb.DiffComponents do
131 149 <% @file.hunks == [] -> %>
132 150 <p class="p-3 text-sm opacity-60">No textual hunks.</p>
133 151 <% true -> %>
134 − <.hunks file={@file} theme={@theme} />
152 + <.hunks
153 + file={@file}
154 + theme={@theme}
155 + comments={@comments}
156 + can_comment?={@can_comment?}
157 + commenting_on={@commenting_on}
158 + />
135 159 <% end %>
136 160 </details>
137 161 """
@@ -139,6 +163,9 @@ defmodule GitGudWeb.DiffComponents do
139 163
140 164 attr :file, :map, required: true
141 165 attr :theme, :string, required: true
166 + attr :comments, :map, default: %{}
167 + attr :can_comment?, :boolean, default: false
168 + attr :commenting_on, :any, default: nil
142 169
143 170 defp hunks(assigns) do
144 171 ~H"""
@@ -149,17 +176,64 @@ defmodule GitGudWeb.DiffComponents do
149 176 </div>
150 177 <table class="w-full border-collapse">
151 178 <tbody>
152 − <tr :for={line <- hunk.lines} class={row_class(line.type)}>
153 − <td class="text-right px-2 py-0.5 opacity-50 select-none w-12 border-r border-base-300">
154 − {line.old_line}
155 − </td>
156 − <td class="text-right px-2 py-0.5 opacity-50 select-none w-12 border-r border-base-300">
157 − {line.new_line}
158 − </td>
159 − <%!-- `whitespace-pre-wrap` renders the template's own
179 + <%= for line <- hunk.lines, anchor = line_anchor(line), true do %>
180 + <tr class={row_class(line.type)}>
181 + <td class="text-right px-2 py-0.5 opacity-50 select-none w-12 border-r border-base-300">
182 + <button
183 + :if={@can_comment? and anchor}
184 + type="button"
185 + phx-click="comment_on_line"
186 + phx-value-path={@file.path}
187 + phx-value-line={anchor}
188 + class="opacity-0 hover:opacity-100 focus:opacity-100 text-primary font-bold"
189 + title={"Comment on line #{anchor}"}
190 + aria-label={"Comment on line #{anchor}"}
191 + >
192 + +
193 + </button>
194 + {line.old_line}
195 + </td>
196 + <td class="text-right px-2 py-0.5 opacity-50 select-none w-12 border-r border-base-300">
197 + {line.new_line}
198 + </td>
199 + <%!-- `whitespace-pre-wrap` renders the template's own
160 200 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 − </tr>
201 + <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>
202 + </tr>
203 + <tr :for={comment <- comments_at(@comments, @file.path, anchor)}>
204 + <td colspan="3" class="px-3 py-2 bg-base-200 border-y border-base-300">
205 + <p class="text-xs opacity-60 mb-1 font-sans">
206 + {comment_author(comment)} commented on line {anchor}
207 + </p>
208 + <p class="whitespace-pre-wrap font-sans text-sm">{comment.body}</p>
209 + </td>
210 + </tr>
211 + <tr :if={@commenting_on == {@file.path, anchor}}>
212 + <td colspan="3" class="px-3 py-2 bg-base-200 border-y border-base-300">
213 + <form phx-submit="save_line_comment" class="space-y-2 font-sans">
214 + <input type="hidden" name="path" value={@file.path} />
215 + <input type="hidden" name="line" value={anchor} />
216 + <textarea
217 + name="body"
218 + rows="3"
219 + required
220 + aria-label={"Comment on line #{anchor}"}
221 + class="textarea textarea-bordered w-full text-sm"
222 + ></textarea>
223 + <div class="flex justify-end gap-2">
224 + <button
225 + type="button"
226 + phx-click="cancel_line_comment"
227 + class="btn btn-xs btn-ghost"
228 + >
229 + Cancel
230 + </button>
231 + <button type="submit" class="btn btn-xs btn-primary">Comment</button>
232 + </div>
233 + </form>
234 + </td>
235 + </tr>
236 + <% end %>
163 237 </tbody>
164 238 </table>
165 239 <% end %>
@@ -167,6 +241,19 @@ defmodule GitGudWeb.DiffComponents do
167 241 """
168 242 end
169 243
244 + # A comment hangs off the new-side line number where there is one —
245 + # that is the line a reader is looking at — and off the old side for
246 + # a deletion, which has no new side.
247 + defp line_anchor(%{new_line: n}) when is_integer(n), do: n
248 + defp line_anchor(%{old_line: o}) when is_integer(o), do: o
249 + defp line_anchor(_), do: nil
250 +
251 + defp comments_at(_comments, _path, nil), do: []
252 + defp comments_at(comments, path, line), do: Map.get(comments, {path, line}, [])
253 +
254 + defp comment_author(%{author: %{handle: h}}) when is_binary(h), do: h
255 + defp comment_author(_), do: "Someone"
256 +
170 257 defp highlight_line("", _path, _theme), do: ""
171 258
172 259 defp highlight_line(text, path, theme) do
modified lib/git_gud_web/live/pr_live/show.ex
+74 −1
@@ -60,6 +60,7 @@ defmodule GitGudWeb.PrLive.Show do
60 60 |> assign(:comment_body, "")
61 61 |> assign(:editing_title?, false)
62 62 |> assign(:editing_comment_id, nil)
63 + |> assign(:commenting_on, nil)
63 64 |> assign(:title_error, nil)
64 65 |> assign(:event_count, Events.count_for(pr))
65 66 |> assign(:page_title, "##{pr.number} #{pr.title}")}
@@ -87,6 +88,22 @@ defmodule GitGudWeb.PrLive.Show do
87 88 Enum.find(socket.assigns.pr.comments, &(&1.id == String.to_integer(id)))
88 89 end
89 90
91 + # A comment with a file and line is a review note anchored in the
92 + # diff; everything else belongs to the conversation below it. Split
93 + # so neither list shows the other's comments twice.
94 + defp line_comment?(%{file_path: p, line: l}) when is_binary(p) and is_integer(l), do: true
95 + defp line_comment?(_), do: false
96 +
97 + defp conversation_comments(pr),
98 + do: Enum.reject(pr.comments, &line_comment?/1)
99 +
100 + defp line_comments(pr) do
101 + pr.comments
102 + |> Enum.filter(&line_comment?/1)
103 + |> Enum.reject(&PrComment.deleted?/1)
104 + |> Enum.group_by(&{&1.file_path, &1.line})
105 + end
106 +
90 107 defp maybe_probe(pr) do
91 108 if pr.state == "open" do
92 109 PullRequests.check_mergeability(pr)
@@ -186,6 +203,7 @@ defmodule GitGudWeb.PrLive.Show do
186 203 socket
187 204 |> assign(:editing_title?, false)
188 205 |> assign(:editing_comment_id, nil)
206 + |> assign(:commenting_on, nil)
189 207 |> assign(:title_error, nil)
190 208 |> assign(:event_count, Events.count_for(pr))
191 209 |> reload()}
@@ -211,6 +229,56 @@ defmodule GitGudWeb.PrLive.Show do
211 229 {:noreply, socket |> assign(:event_count, Events.count_for(pr)) |> reload()}
212 230 end
213 231
232 + def handle_event("comment_on_line", %{"path" => path, "line" => line}, socket) do
233 + if socket.assigns.current_scope && socket.assigns.current_scope.user do
234 + {:noreply, assign(socket, :commenting_on, {path, String.to_integer(line)})}
235 + else
236 + {:noreply, put_flash(socket, :error, "Sign in to comment.")}
237 + end
238 + end
239 +
240 + def handle_event("cancel_line_comment", _params, socket) do
241 + {:noreply, assign(socket, :commenting_on, nil)}
242 + end
243 +
244 + def handle_event("save_line_comment", %{"path" => path, "line" => line, "body" => body}, socket) do
245 + pr = socket.assigns.pr
246 +
247 + case socket.assigns.current_scope && socket.assigns.current_scope.user do
248 + nil ->
249 + {:noreply, put_flash(socket, :error, "Sign in to comment.")}
250 +
251 + user ->
252 + attrs = %{
253 + "body" => String.trim(body),
254 + "file_path" => path,
255 + "line" => String.to_integer(line),
256 + # Pin the comment to the head it was written against, so a
257 + # later push doesn't silently re-anchor it to different code.
258 + "commit_sha" => pr.head_sha
259 + }
260 +
261 + case PullRequests.add_comment(pr, user, attrs) do
262 + {:ok, comment} ->
263 + :ok =
264 + Events.record(pr, "comment_added", user, %{
265 + comment_id: comment.id,
266 + file_path: path,
267 + line: attrs["line"]
268 + })
269 +
270 + {:noreply,
271 + socket
272 + |> assign(:commenting_on, nil)
273 + |> assign(:event_count, Events.count_for(pr))
274 + |> reload()}
275 +
276 + {:error, _cs} ->
277 + {:noreply, put_flash(socket, :error, "Could not save that comment.")}
278 + end
279 + end
280 + end
281 +
214 282 def handle_event("edit_comment", %{"id" => id}, socket) do
215 283 case find_comment(socket, id) do
216 284 nil ->
@@ -250,6 +318,7 @@ defmodule GitGudWeb.PrLive.Show do
250 318 {:noreply,
251 319 socket
252 320 |> assign(:editing_comment_id, nil)
321 + |> assign(:commenting_on, nil)
253 322 |> assign(:event_count, Events.count_for(socket.assigns.pr))
254 323 |> reload()}
255 324
@@ -273,6 +342,7 @@ defmodule GitGudWeb.PrLive.Show do
273 342 {:noreply,
274 343 socket
275 344 |> assign(:editing_comment_id, nil)
345 + |> assign(:commenting_on, nil)
276 346 |> assign(:event_count, Events.count_for(socket.assigns.pr))
277 347 |> reload()}
278 348 else
@@ -451,6 +521,9 @@ defmodule GitGudWeb.PrLive.Show do
451 521 <section>
452 522 <h2 class="font-semibold mb-2">Files changed</h2>
453 523 <GitGudWeb.DiffComponents.diff
524 + comments={line_comments(@pr)}
525 + can_comment?={@current_scope != nil and @current_scope.user != nil}
526 + commenting_on={@commenting_on}
454 527 files={@diff_files}
455 528 theme={@editor_theme}
456 529 loaded_idx={@loaded_diff_idx}
@@ -534,7 +607,7 @@ defmodule GitGudWeb.PrLive.Show do
534 607 <section>
535 608 <h2 class="font-semibold mb-2">Comments</h2>
536 609 <ul class="space-y-3">
537 − <li :for={c <- @pr.comments} class="border border-base-300 rounded p-3">
610 + <li :for={c <- conversation_comments(@pr)} class="border border-base-300 rounded p-3">
538 611 <div class="flex items-baseline justify-between">
539 612 <p class="text-xs opacity-60 mb-1 flex items-center gap-1.5">
540 613 <.avatar name={comment_author(c)} size="xs" alt={comment_author(c)} />
added test/git_gud_web/live/pr_live/line_comment_test.exs
+168 −0
@@ -0,0 +1,168 @@
1 +defmodule GitGudWeb.PrLive.LineCommentTest do
2 + @moduledoc """
3 + Review comments anchored to a file and line in the PR diff.
4 +
5 + `PrComment` already carried `file_path`, `line` and `commit_sha` —
6 + what was missing was any way to write or read one.
7 + """
8 +
9 + use GitGudWeb.ConnCase, async: false
10 +
11 + import Phoenix.LiveViewTest
12 + import GitGud.AccountsFixtures
13 + import GitGud.ForgeFixtures
14 +
15 + alias GitGud.Events
16 + alias GitGud.PullRequests
17 + alias GitGud.Repositories
18 +
19 + defp pr_path(repo, pr) do
20 + handle = Repositories.Storage.repo_handle(GitGud.Repo.preload(repo, [:owner, :organization]))
21 + ~p"/r/#{handle}/#{repo.name}/pulls/#{pr.number}"
22 + end
23 +
24 + defp line_comment!(pr, user, path, line, body) do
25 + {:ok, comment} =
26 + PullRequests.add_comment(pr, user, %{
27 + "body" => body,
28 + "file_path" => path,
29 + "line" => line,
30 + "commit_sha" => pr.head_sha
31 + })
32 +
33 + comment
34 + end
35 +
36 + test "a signed-in viewer gets a per-line comment affordance", %{conn: conn} do
37 + {user, repo} = repository_fixture()
38 + pr = pr_with_branches(repo, user)
39 +
40 + {:ok, lv, _html} = live(log_in_user(conn, user), pr_path(repo, pr))
41 +
42 + assert has_element?(lv, "button[phx-click=comment_on_line]")
43 + end
44 +
45 + test "anonymous visitors get none", %{conn: conn} do
46 + {user, repo} = repository_fixture(%{visibility: "public"})
47 + pr = pr_with_branches(repo, user)
48 +
49 + {:ok, lv, _html} = live(conn, pr_path(repo, pr))
50 +
51 + refute has_element?(lv, "button[phx-click=comment_on_line]")
52 + end
53 +
54 + test "clicking a line opens an inline form on that line only", %{conn: conn} do
55 + {user, repo} = repository_fixture()
56 + pr = pr_with_branches(repo, user)
57 +
58 + {:ok, lv, _html} = live(log_in_user(conn, user), pr_path(repo, pr))
59 +
60 + html = render_hook(lv, "comment_on_line", %{"path" => "feature.txt", "line" => "1"})
61 +
62 + assert html =~ "save_line_comment"
63 + # Exactly one form is open, not one per line.
64 + assert length(String.split(html, "phx-submit=\"save_line_comment\"")) == 2
65 + end
66 +
67 + test "a line comment is saved against its file, line and head sha", %{conn: conn} do
68 + {user, repo} = repository_fixture()
69 + pr = pr_with_branches(repo, user)
70 +
71 + {:ok, lv, _html} = live(log_in_user(conn, user), pr_path(repo, pr))
72 + _ = render_hook(lv, "comment_on_line", %{"path" => "feature.txt", "line" => "1"})
73 +
74 + render_hook(lv, "save_line_comment", %{
75 + "path" => "feature.txt",
76 + "line" => "1",
77 + "body" => "This needs a guard clause"
78 + })
79 +
80 + reloaded = PullRequests.get_pull_request!(repo, pr.number)
81 + comment = Enum.find(reloaded.comments, &(&1.file_path == "feature.txt"))
82 +
83 + assert comment.line == 1
84 + assert comment.body == "This needs a guard clause"
85 + assert comment.commit_sha == pr.head_sha
86 + end
87 +
88 + test "the saved comment renders inline in the diff", %{conn: conn} do
89 + {user, repo} = repository_fixture()
90 + pr = pr_with_branches(repo, user)
91 + line_comment!(pr, user, "feature.txt", 1, "Anchored note")
92 +
93 + {:ok, _lv, html} = live(log_in_user(conn, user), pr_path(repo, pr))
94 +
95 + assert html =~ "Anchored note"
96 + assert html =~ "commented on line 1"
97 + end
98 +
99 + test "line comments stay out of the conversation list", %{conn: conn} do
100 + {user, repo} = repository_fixture()
101 + pr = pr_with_branches(repo, user)
102 + line_comment!(pr, user, "feature.txt", 1, "Inline only")
103 + {:ok, _} = PullRequests.add_comment(pr, user, %{"body" => "Conversation only"})
104 +
105 + {:ok, _lv, html} = live(log_in_user(conn, user), pr_path(repo, pr))
106 +
107 + # Both are present, but the inline one appears once — in the diff.
108 + assert html =~ "Conversation only"
109 + assert length(String.split(html, "Inline only")) == 2
110 + end
111 +
112 + test "a deleted line comment stops rendering in the diff", %{conn: conn} do
113 + {user, repo} = repository_fixture()
114 + pr = pr_with_branches(repo, user)
115 + comment = line_comment!(pr, user, "feature.txt", 1, "Struck out")
116 +
117 + {:ok, _} = PullRequests.delete_comment(comment)
118 +
119 + {:ok, _lv, html} = live(log_in_user(conn, user), pr_path(repo, pr))
120 +
121 + refute html =~ "Struck out"
122 + end
123 +
124 + test "saving records a comment_added event carrying the anchor", %{conn: conn} do
125 + {user, repo} = repository_fixture()
126 + pr = pr_with_branches(repo, user)
127 +
128 + {:ok, lv, _html} = live(log_in_user(conn, user), pr_path(repo, pr))
129 +
130 + render_hook(lv, "save_line_comment", %{
131 + "path" => "feature.txt",
132 + "line" => "2",
133 + "body" => "Recorded"
134 + })
135 +
136 + assert [event] = Events.list_for(pr)
137 + assert event.kind == "comment_added"
138 + assert event.data["file_path"] == "feature.txt"
139 + assert event.data["line"] == 2
140 + end
141 +
142 + test "cancelling closes the form without saving", %{conn: conn} do
143 + {user, repo} = repository_fixture()
144 + pr = pr_with_branches(repo, user)
145 +
146 + {:ok, lv, _html} = live(log_in_user(conn, user), pr_path(repo, pr))
147 + _ = render_hook(lv, "comment_on_line", %{"path" => "feature.txt", "line" => "1"})
148 + html = render_hook(lv, "cancel_line_comment", %{})
149 +
150 + refute html =~ "save_line_comment"
151 + assert PullRequests.get_pull_request!(repo, pr.number).comments == []
152 + end
153 +
154 + test "an anonymous visitor can't save one by sending the event", %{conn: conn} do
155 + {user, repo} = repository_fixture(%{visibility: "public"})
156 + pr = pr_with_branches(repo, user)
157 +
158 + {:ok, lv, _html} = live(conn, pr_path(repo, pr))
159 +
160 + render_hook(lv, "save_line_comment", %{
161 + "path" => "feature.txt",
162 + "line" => "1",
163 + "body" => "Sneaky"
164 + })
165 +
166 + assert PullRequests.get_pull_request!(repo, pr.number).comments == []
167 + end
168 +end

Parents: 6f89f63