neiam /gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Packages
⭐
Log in to mark this repository.
Let reviewers suggest a change and commit it from the diff
51deb4e · Gabriel Morell · 2026-09-10 14:36
Message
{commit_body(@commit)}
Files changed
modified
lib/git_gud/events/event.ex
+1
−0
@@ -13,6 +13,7 @@ defmodule GitGud.Events.Event do
| 13 | 13 | labeled unlabeled |
| 14 | 14 | moderated unmoderated |
| 15 | 15 | comment_added comment_edited comment_deleted |
| 16 | + suggestion_applied | |
| 16 | 17 | reviewed review_requested review_request_removed |
| 17 | 18 | ) |
| 18 | 19 |
modified
lib/git_gud/pull_requests.ex
+156
−0
@@ -19,6 +19,7 @@ defmodule GitGud.PullRequests do
| 19 | 19 | alias GitGud.PullRequests.PrComment |
| 20 | 20 | alias GitGud.PullRequests.PrReview |
| 21 | 21 | alias GitGud.PullRequests.PrReviewRequest |
| 22 | + alias GitGud.PullRequests.Suggestion | |
| 22 | 23 | alias GitGud.Repo |
| 23 | 24 | alias GitGud.Repositories |
| 24 | 25 | alias GitGud.Repositories.Numbering |
@@ -688,6 +689,161 @@ defmodule GitGud.PullRequests do
| 688 | 689 | end |
| 689 | 690 | end |
| 690 | 691 | |
| 692 | + # ── suggestions ────────────────────────────────────────────────────── | |
| 693 | + | |
| 694 | + @doc """ | |
| 695 | + Commit a reviewer's suggested replacement onto the PR's source | |
| 696 | + branch. | |
| 697 | + | |
| 698 | + Refuses rather than guesses: | |
| 699 | + | |
| 700 | + * `:not_a_suggestion` — the comment has no ```suggestion block | |
| 701 | + * `:forbidden` — the actor can't write to the source repo | |
| 702 | + * `:stale` — the branch moved since the suggestion was written, so | |
| 703 | + the line it named may no longer be the line it meant | |
| 704 | + * `:out_of_range` — the file no longer has that line | |
| 705 | + * `:closed` — the PR isn't open | |
| 706 | + | |
| 707 | + On success the branch is updated by compare-and-swap against the head | |
| 708 | + the suggestion was written against, so a concurrent push loses the | |
| 709 | + race rather than being silently overwritten. | |
| 710 | + """ | |
| 711 | + def apply_suggestion(%PrComment{} = comment, %User{} = actor) do | |
| 712 | + with {:ok, replacement} <- fetch_suggestion(comment), | |
| 713 | + {:ok, pr} <- fetch_open_pr(comment), | |
| 714 | + source_repo <- Repositories.get_repository!(pr.source_repository_id), | |
| 715 | + :ok <- authorize_write(actor, source_repo), | |
| 716 | + :ok <- ensure_fresh(pr, comment, source_repo), | |
| 717 | + {:ok, content, mode} <- read_file(source_repo, pr.head_sha, comment.file_path), | |
| 718 | + {:ok, updated} <- Suggestion.apply_to(content, comment.line, replacement), | |
| 719 | + {:ok, new_sha} <- | |
| 720 | + commit_replacement(source_repo, pr, comment, actor, updated, mode) do | |
| 721 | + {:ok, new_sha} | |
| 722 | + end | |
| 723 | + end | |
| 724 | + | |
| 725 | + defp fetch_suggestion(%PrComment{body: body, file_path: p, line: l}) | |
| 726 | + when is_binary(p) and is_integer(l) do | |
| 727 | + case Suggestion.parse(body) do | |
| 728 | + nil -> {:error, :not_a_suggestion} | |
| 729 | + text -> {:ok, text} | |
| 730 | + end | |
| 731 | + end | |
| 732 | + | |
| 733 | + defp fetch_suggestion(_), do: {:error, :not_a_suggestion} | |
| 734 | + | |
| 735 | + defp fetch_open_pr(%PrComment{pull_request_id: id}) do | |
| 736 | + case Repo.get(PullRequest, id) do | |
| 737 | + %PullRequest{state: "open"} = pr -> {:ok, pr} | |
| 738 | + %PullRequest{} -> {:error, :closed} | |
| 739 | + nil -> {:error, :closed} | |
| 740 | + end | |
| 741 | + end | |
| 742 | + | |
| 743 | + defp authorize_write(actor, repo) do | |
| 744 | + if GitGud.Organizations.can?(actor, repo, :write), do: :ok, else: {:error, :forbidden} | |
| 745 | + end | |
| 746 | + | |
| 747 | + # The suggestion named a line on a particular commit. If the branch | |
| 748 | + # has moved since, that line number may point somewhere else entirely. | |
| 749 | + defp ensure_fresh(pr, comment, source_repo) do | |
| 750 | + with true <- comment.commit_sha == pr.head_sha, | |
| 751 | + {:ok, current} <- Repositories.resolve(source_repo, pr.source_ref), | |
| 752 | + true <- current == pr.head_sha do | |
| 753 | + :ok | |
| 754 | + else | |
| 755 | + _ -> {:error, :stale} | |
| 756 | + end | |
| 757 | + end | |
| 758 | + | |
| 759 | + defp read_file(repo, commit_sha, path) do | |
| 760 | + with {:ok, commit} <- Repositories.get_commit(repo, commit_sha), | |
| 761 | + {:ok, entry} <- Repositories.get_tree_entry_at(repo, commit.tree_sha, path), | |
| 762 | + {:ok, bytes} <- Repositories.get_blob_bytes(repo, entry.sha) do | |
| 763 | + {:ok, bytes, entry_mode(entry)} | |
| 764 | + else | |
| 765 | + _ -> {:error, :no_such_file} | |
| 766 | + end | |
| 767 | + end | |
| 768 | + | |
| 769 | + # Preserve the executable bit rather than flattening every applied | |
| 770 | + # file to 100644. | |
| 771 | + defp entry_mode(%{mode: mode}) when is_integer(mode) do | |
| 772 | + if Bitwise.band(mode, 0o111) != 0, do: "100755", else: "100644" | |
| 773 | + end | |
| 774 | + | |
| 775 | + defp entry_mode(_), do: "100644" | |
| 776 | + | |
| 777 | + defp commit_replacement(repo, pr, comment, actor, content, mode) do | |
| 778 | + path = repo.disk_path | |
| 779 | + | |
| 780 | + with {:ok, blob_sha} <- write_blob(path, content), | |
| 781 | + {:ok, tree_sha} <- | |
| 782 | + write_tree_with(path, pr.head_sha, comment.file_path, blob_sha, mode), | |
| 783 | + {:ok, commit_sha} <- | |
| 784 | + Git.commit_tree( | |
| 785 | + path, | |
| 786 | + tree_sha, | |
| 787 | + [pr.head_sha], | |
| 788 | + "Apply suggestion to #{comment.file_path}", | |
| 789 | + author: actor_identity(actor) | |
| 790 | + ), | |
| 791 | + :ok <- | |
| 792 | + Git.update_ref(path, "refs/heads/" <> pr.source_ref, commit_sha, pr.head_sha) do | |
| 793 | + # Keep the PR row pointing at the commit we just made, the same | |
| 794 | + # way a push would. | |
| 795 | + _ = pr |> Ecto.Changeset.change(head_sha: commit_sha, mergeable: nil) |> Repo.update() | |
| 796 | + {:ok, commit_sha} | |
| 797 | + end | |
| 798 | + end | |
| 799 | + | |
| 800 | + defp write_blob(path, content) do | |
| 801 | + tmp = Path.join(System.tmp_dir!(), "sugg-#{System.unique_integer([:positive])}") | |
| 802 | + File.write!(tmp, content) | |
| 803 | + | |
| 804 | + try do | |
| 805 | + case System.cmd("git", ["-C", path, "hash-object", "-w", tmp], stderr_to_stdout: true) do | |
| 806 | + {hex, 0} -> Git.from_hex(String.trim(hex)) | |
| 807 | + {out, _} -> {:error, {:hash_object, out}} | |
| 808 | + end | |
| 809 | + after | |
| 810 | + File.rm(tmp) | |
| 811 | + end | |
| 812 | + end | |
| 813 | + | |
| 814 | + # A temp index seeded from the parent tree, so nested paths keep their | |
| 815 | + # surrounding directories instead of being rebuilt by hand. | |
| 816 | + defp write_tree_with(path, parent_sha, file_path, blob_sha, mode) do | |
| 817 | + index = Path.join(System.tmp_dir!(), "sugg-idx-#{System.unique_integer([:positive])}") | |
| 818 | + env = [{"GIT_INDEX_FILE", index}] | |
| 819 | + | |
| 820 | + try do | |
| 821 | + with {:ok, parent} <- Git.commit(path, parent_sha), | |
| 822 | + {_, 0} <- | |
| 823 | + System.cmd("git", ["-C", path, "read-tree", Git.to_hex(parent.tree_sha)], env: env), | |
| 824 | + {_, 0} <- | |
| 825 | + System.cmd( | |
| 826 | + "git", | |
| 827 | + [ | |
| 828 | + "-C", | |
| 829 | + path, | |
| 830 | + "update-index", | |
| 831 | + "--add", | |
| 832 | + "--cacheinfo", | |
| 833 | + mode <> "," <> Git.to_hex(blob_sha) <> "," <> file_path | |
| 834 | + ], | |
| 835 | + env: env | |
| 836 | + ), | |
| 837 | + {hex, 0} <- System.cmd("git", ["-C", path, "write-tree"], env: env) do | |
| 838 | + Git.from_hex(String.trim(hex)) | |
| 839 | + else | |
| 840 | + other -> {:error, {:write_tree, other}} | |
| 841 | + end | |
| 842 | + after | |
| 843 | + File.rm(index) | |
| 844 | + end | |
| 845 | + end | |
| 846 | + | |
| 691 | 847 | # ── review requests ────────────────────────────────────────────────── |
| 692 | 848 | |
| 693 | 849 | @doc """ |
added
lib/git_gud/pull_requests/suggestion.ex
+67
−0
@@ -0,0 +1,67 @@
| 1 | +defmodule GitGud.PullRequests.Suggestion do | |
| 2 | + @moduledoc """ | |
| 3 | + A ```suggestion fenced block inside a line comment: the reviewer's | |
| 4 | + proposed replacement for the line they commented on. | |
| 5 | + | |
| 6 | + Parsing lives here rather than in the Markdown renderer because | |
| 7 | + applying one needs the raw replacement text, not its rendered HTML. | |
| 8 | + """ | |
| 9 | + | |
| 10 | + @fence ~r/^```suggestion[ \t]*\r?\n(?<body>.*?)\r?\n?^```[ \t]*$/ms | |
| 11 | + | |
| 12 | + @doc """ | |
| 13 | + The suggested replacement in `body`, or nil when there isn't one. | |
| 14 | + | |
| 15 | + An empty suggestion block is a real thing — it means "delete this | |
| 16 | + line" — so it returns `""` rather than nil. | |
| 17 | + """ | |
| 18 | + def parse(body) when is_binary(body) do | |
| 19 | + case Regex.named_captures(@fence, body) do | |
| 20 | + %{"body" => text} -> text | |
| 21 | + _ -> nil | |
| 22 | + end | |
| 23 | + end | |
| 24 | + | |
| 25 | + def parse(_), do: nil | |
| 26 | + | |
| 27 | + @doc "True when `body` carries a suggestion block." | |
| 28 | + def suggestion?(body), do: not is_nil(parse(body)) | |
| 29 | + | |
| 30 | + @doc """ | |
| 31 | + The comment text with the suggestion block stripped out — what the | |
| 32 | + reviewer said *about* the change, as opposed to the change itself. | |
| 33 | + """ | |
| 34 | + def prose(body) when is_binary(body) do | |
| 35 | + body | |
| 36 | + |> String.replace(@fence, "") | |
| 37 | + |> String.trim() | |
| 38 | + end | |
| 39 | + | |
| 40 | + def prose(_), do: "" | |
| 41 | + | |
| 42 | + @doc """ | |
| 43 | + Replace 1-indexed `line` of `content` with `replacement`. | |
| 44 | + | |
| 45 | + A replacement spanning several lines expands in place; an empty one | |
| 46 | + removes the line. Returns `{:error, :out_of_range}` when the file no | |
| 47 | + longer has that line, which is how a stale suggestion surfaces. | |
| 48 | + """ | |
| 49 | + def apply_to(content, line, replacement) | |
| 50 | + when is_binary(content) and is_integer(line) and is_binary(replacement) do | |
| 51 | + lines = String.split(content, "\n") | |
| 52 | + | |
| 53 | + if line < 1 or line > length(lines) do | |
| 54 | + {:error, :out_of_range} | |
| 55 | + else | |
| 56 | + idx = line - 1 | |
| 57 | + replacement_lines = if replacement == "", do: [], else: String.split(replacement, "\n") | |
| 58 | + | |
| 59 | + updated = | |
| 60 | + Enum.slice(lines, 0, idx) ++ | |
| 61 | + replacement_lines ++ | |
| 62 | + Enum.slice(lines, (idx + 1)..-1//1) | |
| 63 | + | |
| 64 | + {:ok, Enum.join(updated, "\n")} | |
| 65 | + end | |
| 66 | + end | |
| 67 | +end |
modified
lib/git_gud_web/components/diff_components.ex
+53
−1
@@ -18,6 +18,7 @@ defmodule GitGudWeb.DiffComponents do
| 18 | 18 | use Phoenix.Component |
| 19 | 19 | |
| 20 | 20 | alias GitGud.Git.DiffParser |
| 21 | + alias GitGud.PullRequests.Suggestion | |
| 21 | 22 | alias GitGud.SyntaxHighlight |
| 22 | 23 | |
| 23 | 24 | @doc """ |
@@ -46,6 +47,10 @@ defmodule GitGudWeb.DiffComponents do
| 46 | 47 | default: false, |
| 47 | 48 | doc: "Show the per-line 'add comment' affordance." |
| 48 | 49 | |
| 50 | + attr :can_apply?, :boolean, | |
| 51 | + default: false, | |
| 52 | + doc: "Show Apply on suggestion comments (write access to the source repo)." | |
| 53 | + | |
| 49 | 54 | attr :commenting_on, :any, |
| 50 | 55 | default: nil, |
| 51 | 56 | doc: "`{path, line}` whose inline form is open, or nil." |
@@ -65,6 +70,7 @@ defmodule GitGudWeb.DiffComponents do
| 65 | 70 | loaded?={MapSet.member?(@loaded_idx, idx)} |
| 66 | 71 | comments={@comments} |
| 67 | 72 | can_comment?={@can_comment?} |
| 73 | + can_apply?={@can_apply?} | |
| 68 | 74 | commenting_on={@commenting_on} |
| 69 | 75 | /> |
| 70 | 76 | </div> |
@@ -115,6 +121,7 @@ defmodule GitGudWeb.DiffComponents do
| 115 | 121 | attr :loaded?, :boolean, default: true |
| 116 | 122 | attr :comments, :map, default: %{} |
| 117 | 123 | attr :can_comment?, :boolean, default: false |
| 124 | + attr :can_apply?, :boolean, default: false | |
| 118 | 125 | attr :commenting_on, :any, default: nil |
| 119 | 126 | |
| 120 | 127 | defp file_diff(assigns) do |
@@ -154,6 +161,7 @@ defmodule GitGudWeb.DiffComponents do
| 154 | 161 | theme={@theme} |
| 155 | 162 | comments={@comments} |
| 156 | 163 | can_comment?={@can_comment?} |
| 164 | + can_apply?={@can_apply?} | |
| 157 | 165 | commenting_on={@commenting_on} |
| 158 | 166 | /> |
| 159 | 167 | <% end %> |
@@ -165,6 +173,7 @@ defmodule GitGudWeb.DiffComponents do
| 165 | 173 | attr :theme, :string, required: true |
| 166 | 174 | attr :comments, :map, default: %{} |
| 167 | 175 | attr :can_comment?, :boolean, default: false |
| 176 | + attr :can_apply?, :boolean, default: false | |
| 168 | 177 | attr :commenting_on, :any, default: nil |
| 169 | 178 | |
| 170 | 179 | defp hunks(assigns) do |
@@ -205,7 +214,42 @@ defmodule GitGudWeb.DiffComponents do
| 205 | 214 | <p class="text-xs opacity-60 mb-1 font-sans"> |
| 206 | 215 | {comment_author(comment)} commented on line {anchor} |
| 207 | 216 | </p> |
| 208 | − <p class="whitespace-pre-wrap font-sans text-sm">{comment.body}</p> | |
| 217 | + <%= if Suggestion.suggestion?(comment.body) do %> | |
| 218 | + <p | |
| 219 | + :if={Suggestion.prose(comment.body) != ""} | |
| 220 | + class="whitespace-pre-wrap font-sans text-sm mb-2" | |
| 221 | + > | |
| 222 | + {Suggestion.prose(comment.body)} | |
| 223 | + </p> | |
| 224 | + <div class="rounded border border-base-300 overflow-hidden"> | |
| 225 | + <p class="px-2 py-1 text-xs font-sans bg-base-300/50">Suggested change</p> | |
| 226 | + <div | |
| 227 | + :for={l <- suggestion_lines(comment.body)} | |
| 228 | + class="font-mono whitespace-pre px-2 py-0.5 bg-success/15" | |
| 229 | + > | |
| 230 | + +{l} | |
| 231 | + </div> | |
| 232 | + <p | |
| 233 | + :if={suggestion_lines(comment.body) == []} | |
| 234 | + class="font-mono px-2 py-0.5 bg-error/15 opacity-70" | |
| 235 | + > | |
| 236 | + (remove this line) | |
| 237 | + </p> | |
| 238 | + </div> | |
| 239 | + <div :if={@can_apply?} class="mt-2 font-sans"> | |
| 240 | + <button | |
| 241 | + type="button" | |
| 242 | + phx-click="apply_suggestion" | |
| 243 | + phx-value-id={comment.id} | |
| 244 | + class="btn btn-xs btn-primary" | |
| 245 | + data-confirm="Commit this suggestion to the source branch?" | |
| 246 | + > | |
| 247 | + Apply suggestion | |
| 248 | + </button> | |
| 249 | + </div> | |
| 250 | + <% else %> | |
| 251 | + <p class="whitespace-pre-wrap font-sans text-sm">{comment.body}</p> | |
| 252 | + <% end %> | |
| 209 | 253 | </td> |
| 210 | 254 | </tr> |
| 211 | 255 | <tr :if={@commenting_on == {@file.path, anchor}}> |
@@ -251,6 +295,14 @@ defmodule GitGudWeb.DiffComponents do
| 251 | 295 | defp comments_at(_comments, _path, nil), do: [] |
| 252 | 296 | defp comments_at(comments, path, line), do: Map.get(comments, {path, line}, []) |
| 253 | 297 | |
| 298 | + defp suggestion_lines(body) do | |
| 299 | + case Suggestion.parse(body) do | |
| 300 | + nil -> [] | |
| 301 | + "" -> [] | |
| 302 | + text -> String.split(text, "\n") | |
| 303 | + end | |
| 304 | + end | |
| 305 | + | |
| 254 | 306 | defp comment_author(%{author: %{handle: h}}) when is_binary(h), do: h |
| 255 | 307 | defp comment_author(_), do: "Someone" |
| 256 | 308 |
modified
lib/git_gud_web/components/event_components.ex
+6
−0
@@ -151,6 +151,7 @@ defmodule GitGudWeb.EventComponents do
| 151 | 151 | defp dot_class("description_changed"), do: "bg-base-300" |
| 152 | 152 | defp dot_class(k) when k in ["reviewed", "review_requested"], do: "bg-accent" |
| 153 | 153 | defp dot_class("review_request_removed"), do: "bg-base-300" |
| 154 | + defp dot_class("suggestion_applied"), do: "bg-success" | |
| 154 | 155 | defp dot_class(k) when k in ["comment_added", "comment_edited"], do: "bg-info" |
| 155 | 156 | defp dot_class("comment_deleted"), do: "bg-error" |
| 156 | 157 | defp dot_class("merged"), do: "bg-secondary" |
@@ -235,6 +236,11 @@ defmodule GitGudWeb.EventComponents do
| 235 | 236 | |
| 236 | 237 | defp describe(%{kind: "review_request_removed"}), do: "withdrew a review request." |
| 237 | 238 | |
| 239 | + defp describe(%{kind: "suggestion_applied", data: %{"file_path" => path, "line" => line}}), | |
| 240 | + do: "applied a suggestion to #{path}:#{line}." | |
| 241 | + | |
| 242 | + defp describe(%{kind: "suggestion_applied"}), do: "applied a suggestion." | |
| 243 | + | |
| 238 | 244 | defp describe(%{kind: "comment_added"}), do: "commented." |
| 239 | 245 | defp describe(%{kind: "comment_edited"}), do: "edited a comment." |
| 240 | 246 | defp describe(%{kind: "comment_deleted"}), do: "deleted a comment." |
modified
lib/git_gud_web/live/pr_live/files.ex
+54
−0
@@ -36,6 +36,7 @@ defmodule GitGudWeb.PrLive.Files do
| 36 | 36 | |> assign(:diff_files, diff_files) |
| 37 | 37 | |> assign(:loaded_diff_idx, GitGudWeb.DiffComponents.default_loaded(diff_files)) |
| 38 | 38 | |> assign(:commenting_on, nil) |
| 39 | + |> assign(:can_apply?, can_apply?(socket, pr)) | |
| 39 | 40 | |> assign(:page_title, "Files · ##{pr.number} #{pr.title}")} |
| 40 | 41 | end |
| 41 | 42 |
@@ -53,6 +54,19 @@ defmodule GitGudWeb.PrLive.Files do
| 53 | 54 | |
| 54 | 55 | defp viewer(socket), do: socket.assigns.current_scope && socket.assigns.current_scope.user |
| 55 | 56 | |
| 57 | + # Applying commits to the PR's source branch, so it needs write | |
| 58 | + # access there — which for a fork is a different repo than this one. | |
| 59 | + defp can_apply?(socket, pr) do | |
| 60 | + case socket.assigns.current_scope && socket.assigns.current_scope.user do | |
| 61 | + nil -> | |
| 62 | + false | |
| 63 | + | |
| 64 | + user -> | |
| 65 | + source = Repositories.get_repository!(pr.source_repository_id) | |
| 66 | + GitGud.Organizations.can?(user, source, :write) | |
| 67 | + end | |
| 68 | + end | |
| 69 | + | |
| 56 | 70 | defp line_comment?(%{file_path: p, line: l}) when is_binary(p) and is_integer(l), do: true |
| 57 | 71 | defp line_comment?(_), do: false |
| 58 | 72 |
@@ -77,6 +91,37 @@ defmodule GitGudWeb.PrLive.Files do
| 77 | 91 | end |
| 78 | 92 | end |
| 79 | 93 | |
| 94 | + def handle_event("apply_suggestion", %{"id" => id}, socket) do | |
| 95 | + pr = socket.assigns.pr | |
| 96 | + | |
| 97 | + with user when not is_nil(user) <- viewer(socket), | |
| 98 | + comment when not is_nil(comment) <- | |
| 99 | + Enum.find(pr.comments, &(&1.id == String.to_integer(id))) do | |
| 100 | + case PullRequests.apply_suggestion(comment, user) do | |
| 101 | + {:ok, _sha} -> | |
| 102 | + :ok = | |
| 103 | + Events.record(pr, "suggestion_applied", user, %{ | |
| 104 | + comment_id: comment.id, | |
| 105 | + file_path: comment.file_path, | |
| 106 | + line: comment.line | |
| 107 | + }) | |
| 108 | + | |
| 109 | + {:noreply, | |
| 110 | + socket | |
| 111 | + |> put_flash(:info, "Suggestion applied.") | |
| 112 | + |> push_navigate( | |
| 113 | + to: | |
| 114 | + ~p"/r/#{socket.assigns.handle}/#{socket.assigns.repo.name}/pulls/#{pr.number}/files" | |
| 115 | + )} | |
| 116 | + | |
| 117 | + {:error, reason} -> | |
| 118 | + {:noreply, put_flash(socket, :error, apply_error(reason))} | |
| 119 | + end | |
| 120 | + else | |
| 121 | + _ -> {:noreply, socket} | |
| 122 | + end | |
| 123 | + end | |
| 124 | + | |
| 80 | 125 | def handle_event("cancel_line_comment", _params, socket) do |
| 81 | 126 | {:noreply, assign(socket, :commenting_on, nil)} |
| 82 | 127 | end |
@@ -115,6 +160,14 @@ defmodule GitGudWeb.PrLive.Files do
| 115 | 160 | end |
| 116 | 161 | end |
| 117 | 162 | |
| 163 | + defp apply_error(:forbidden), do: "You don't have write access to the source branch." | |
| 164 | + defp apply_error(:stale), do: "The branch moved since this suggestion was written." | |
| 165 | + defp apply_error(:out_of_range), do: "That line no longer exists in the file." | |
| 166 | + defp apply_error(:closed), do: "This pull request isn't open." | |
| 167 | + defp apply_error(:no_such_file), do: "That file is no longer in the branch." | |
| 168 | + defp apply_error(:not_a_suggestion), do: "That comment isn't a suggestion." | |
| 169 | + defp apply_error(other), do: "Could not apply the suggestion: #{inspect(other)}" | |
| 170 | + | |
| 118 | 171 | @impl true |
| 119 | 172 | def render(assigns) do |
| 120 | 173 | ~H""" |
@@ -162,6 +215,7 @@ defmodule GitGudWeb.PrLive.Files do
| 162 | 215 | <GitGudWeb.DiffComponents.diff |
| 163 | 216 | comments={line_comments(@pr)} |
| 164 | 217 | can_comment?={@current_scope != nil and @current_scope.user != nil} |
| 218 | + can_apply?={@can_apply?} | |
| 165 | 219 | commenting_on={@commenting_on} |
| 166 | 220 | files={@diff_files} |
| 167 | 221 | theme={@editor_theme} |
added
test/git_gud/suggestion_test.exs
+153
−0
@@ -0,0 +1,153 @@
| 1 | +defmodule GitGud.SuggestionTest do | |
| 2 | + use GitGud.DataCase, async: false | |
| 3 | + | |
| 4 | + import GitGud.AccountsFixtures | |
| 5 | + import GitGud.ForgeFixtures | |
| 6 | + | |
| 7 | + alias GitGud.Git | |
| 8 | + alias GitGud.PullRequests | |
| 9 | + alias GitGud.PullRequests.Suggestion | |
| 10 | + alias GitGud.Repositories | |
| 11 | + | |
| 12 | + describe "parsing" do | |
| 13 | + test "pulls the replacement out of a fenced block" do | |
| 14 | + assert Suggestion.parse("looks off\n\n```suggestion\nnew line\n```") == "new line" | |
| 15 | + end | |
| 16 | + | |
| 17 | + test "keeps a multi-line replacement intact" do | |
| 18 | + assert Suggestion.parse("```suggestion\none\ntwo\n```") == "one\ntwo" | |
| 19 | + end | |
| 20 | + | |
| 21 | + test "an empty block means delete the line, not 'no suggestion'" do | |
| 22 | + assert Suggestion.parse("```suggestion\n```") == "" | |
| 23 | + assert Suggestion.suggestion?("```suggestion\n```") | |
| 24 | + end | |
| 25 | + | |
| 26 | + test "a plain comment has none" do | |
| 27 | + refute Suggestion.suggestion?("just a remark") | |
| 28 | + assert Suggestion.parse("just a remark") == nil | |
| 29 | + end | |
| 30 | + | |
| 31 | + test "prose is the comment minus the block" do | |
| 32 | + body = "please rename this\n\n```suggestion\nfoo = 1\n```" | |
| 33 | + assert Suggestion.prose(body) == "please rename this" | |
| 34 | + end | |
| 35 | + end | |
| 36 | + | |
| 37 | + describe "apply_to/3" do | |
| 38 | + test "replaces the named line" do | |
| 39 | + assert {:ok, "a\nX\nc"} = Suggestion.apply_to("a\nb\nc", 2, "X") | |
| 40 | + end | |
| 41 | + | |
| 42 | + test "expands one line into several" do | |
| 43 | + assert {:ok, "a\nX\nY\nc"} = Suggestion.apply_to("a\nb\nc", 2, "X\nY") | |
| 44 | + end | |
| 45 | + | |
| 46 | + test "an empty replacement removes the line" do | |
| 47 | + assert {:ok, "a\nc"} = Suggestion.apply_to("a\nb\nc", 2, "") | |
| 48 | + end | |
| 49 | + | |
| 50 | + test "the first and last lines are reachable" do | |
| 51 | + assert {:ok, "X\nb\nc"} = Suggestion.apply_to("a\nb\nc", 1, "X") | |
| 52 | + assert {:ok, "a\nb\nX"} = Suggestion.apply_to("a\nb\nc", 3, "X") | |
| 53 | + end | |
| 54 | + | |
| 55 | + test "a line past the end is out of range rather than appended" do | |
| 56 | + assert {:error, :out_of_range} = Suggestion.apply_to("a\nb", 5, "X") | |
| 57 | + assert {:error, :out_of_range} = Suggestion.apply_to("a\nb", 0, "X") | |
| 58 | + end | |
| 59 | + end | |
| 60 | + | |
| 61 | + describe "apply_suggestion/2" do | |
| 62 | + setup do | |
| 63 | + {user, repo} = repository_fixture() | |
| 64 | + pr = pr_with_branches(repo, user) | |
| 65 | + {:ok, user: user, repo: repo, pr: pr} | |
| 66 | + end | |
| 67 | + | |
| 68 | + defp suggest!(pr, user, line, body) do | |
| 69 | + {:ok, comment} = | |
| 70 | + PullRequests.add_comment(pr, user, %{ | |
| 71 | + "body" => body, | |
| 72 | + "file_path" => "feature.txt", | |
| 73 | + "line" => line, | |
| 74 | + "commit_sha" => pr.head_sha | |
| 75 | + }) | |
| 76 | + | |
| 77 | + comment | |
| 78 | + end | |
| 79 | + | |
| 80 | + test "commits the replacement to the source branch", %{user: user, repo: repo, pr: pr} do | |
| 81 | + comment = suggest!(pr, user, 1, "```suggestion\nreplaced\n```") | |
| 82 | + | |
| 83 | + assert {:ok, new_sha} = PullRequests.apply_suggestion(comment, user) | |
| 84 | + assert new_sha != pr.head_sha | |
| 85 | + | |
| 86 | + # The branch moved to the new commit... | |
| 87 | + assert {:ok, ^new_sha} = Repositories.resolve(repo, "feature") | |
| 88 | + | |
| 89 | + # ...and the file says what the reviewer suggested. | |
| 90 | + {:ok, commit} = Repositories.get_commit(repo, new_sha) | |
| 91 | + {:ok, entry} = Repositories.get_tree_entry_at(repo, commit.tree_sha, "feature.txt") | |
| 92 | + {:ok, bytes} = Repositories.get_blob_bytes(repo, entry.sha) | |
| 93 | + assert bytes =~ "replaced" | |
| 94 | + end | |
| 95 | + | |
| 96 | + test "the new commit's parent is the old head", %{user: user, repo: repo, pr: pr} do | |
| 97 | + comment = suggest!(pr, user, 1, "```suggestion\nchild\n```") | |
| 98 | + {:ok, new_sha} = PullRequests.apply_suggestion(comment, user) | |
| 99 | + | |
| 100 | + {:ok, commit} = Repositories.get_commit(repo, new_sha) | |
| 101 | + assert Git.to_hex(pr.head_sha) in Enum.map(commit.parent_shas || [], &Git.to_hex/1) | |
| 102 | + end | |
| 103 | + | |
| 104 | + test "the PR row follows the branch", %{user: user, repo: repo, pr: pr} do | |
| 105 | + comment = suggest!(pr, user, 1, "```suggestion\nmoved\n```") | |
| 106 | + {:ok, new_sha} = PullRequests.apply_suggestion(comment, user) | |
| 107 | + | |
| 108 | + assert PullRequests.get_pull_request!(repo, pr.number).head_sha == new_sha | |
| 109 | + end | |
| 110 | + | |
| 111 | + test "someone without write access is refused", %{user: user, pr: pr} do | |
| 112 | + comment = suggest!(pr, user, 1, "```suggestion\nnope\n```") | |
| 113 | + outsider = user_fixture() | |
| 114 | + | |
| 115 | + assert {:error, :forbidden} = PullRequests.apply_suggestion(comment, outsider) | |
| 116 | + end | |
| 117 | + | |
| 118 | + test "a plain comment isn't applyable", %{user: user, pr: pr} do | |
| 119 | + comment = suggest!(pr, user, 1, "just a remark") | |
| 120 | + assert {:error, :not_a_suggestion} = PullRequests.apply_suggestion(comment, user) | |
| 121 | + end | |
| 122 | + | |
| 123 | + test "a suggestion written against an older head is stale", %{user: user, repo: repo, pr: pr} do | |
| 124 | + comment = suggest!(pr, user, 1, "```suggestion\ntoo late\n```") | |
| 125 | + | |
| 126 | + # Someone pushes; the line this named may not be that line now. | |
| 127 | + advance_branch(repo, "feature", user) | |
| 128 | + | |
| 129 | + assert {:error, :stale} = PullRequests.apply_suggestion(comment, user) | |
| 130 | + end | |
| 131 | + | |
| 132 | + test "a line past the end of the file is refused", %{user: user, pr: pr} do | |
| 133 | + comment = suggest!(pr, user, 999, "```suggestion\nnowhere\n```") | |
| 134 | + assert {:error, :out_of_range} = PullRequests.apply_suggestion(comment, user) | |
| 135 | + end | |
| 136 | + | |
| 137 | + test "a closed PR can't take suggestions", %{user: user, pr: pr} do | |
| 138 | + comment = suggest!(pr, user, 1, "```suggestion\nclosed\n```") | |
| 139 | + {:ok, _} = PullRequests.set_state(pr, "closed") | |
| 140 | + | |
| 141 | + assert {:error, :closed} = PullRequests.apply_suggestion(comment, user) | |
| 142 | + end | |
| 143 | + | |
| 144 | + test "applying twice in a row is refused as stale", %{user: user, pr: pr} do | |
| 145 | + one = suggest!(pr, user, 1, "```suggestion\nfirst\n```") | |
| 146 | + two = suggest!(pr, user, 1, "```suggestion\nsecond\n```") | |
| 147 | + | |
| 148 | + assert {:ok, _} = PullRequests.apply_suggestion(one, user) | |
| 149 | + # The branch moved under it, so the second no longer means what it said. | |
| 150 | + assert {:error, :stale} = PullRequests.apply_suggestion(two, user) | |
| 151 | + end | |
| 152 | + end | |
| 153 | +end |
Parents: 7360f1e