From a706a04e9a673cd487e9b6d72f4fdd8a98b43d8a Mon Sep 17 00:00:00 2001 From: Tamas Kalman Date: Sun, 5 Oct 2025 12:47:22 -0700 Subject: [PATCH 1/2] Add inline code comments on commits This feature enables users to add inline code comments on individual commits, similar to the existing PR code review functionality. Comments can be added, edited, and deleted on any line of a commit's diff. Changes: - Add service layer functions for commit comment CRUD operations - Add router handlers and API routes for commit comments - Add model queries to find commit comments - Update diff templates to show comment UI on commit pages - Load and display commit comments in the Diff() handler - Reuse existing PR comment templates and JavaScript Implements: https://github.com/go-gitea/gitea/issues/4898 --- models/issues/comment.go | 23 ++++ routers/web/repo/commit.go | 154 +++++++++++++++++++++++ routers/web/web.go | 10 ++ services/repository/commit_comment.go | 122 ++++++++++++++++++ templates/repo/diff/box.tmpl | 2 +- templates/repo/diff/section_split.tmpl | 8 +- templates/repo/diff/section_unified.tmpl | 2 +- 7 files changed, 315 insertions(+), 6 deletions(-) create mode 100644 services/repository/commit_comment.go diff --git a/models/issues/comment.go b/models/issues/comment.go index 3a4049700de1a..e17095c82d4ca 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -1076,6 +1076,29 @@ func CountComments(ctx context.Context, opts *FindCommentsOptions) (int64, error return sess.Count(&Comment{}) } +// FindCommitComments finds all code comments for a specific commit +func FindCommitComments(ctx context.Context, repoID int64, commitSHA string) (CommentList, error) { + comments := make([]*Comment, 0, 10) + return comments, db.GetEngine(ctx). + Where("commit_sha = ?", commitSHA). + And("type = ?", CommentTypeCode). + Asc("created_unix"). + Asc("id"). + Find(&comments) +} + +// FindCommitLineComments finds code comments for a specific file and line in a commit +func FindCommitLineComments(ctx context.Context, commitSHA, treePath string) (CommentList, error) { + comments := make([]*Comment, 0, 10) + return comments, db.GetEngine(ctx). + Where("commit_sha = ?", commitSHA). + And("tree_path = ?", treePath). + And("type = ?", CommentTypeCode). + Asc("created_unix"). + Asc("id"). + Find(&comments) +} + // UpdateCommentInvalidate updates comment invalidated column func UpdateCommentInvalidate(ctx context.Context, c *Comment) error { _, err := db.GetEngine(ctx).ID(c.ID).Cols("invalidated").Update(c) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 1a86a62fae172..b2d3eda9f1338 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -30,8 +30,11 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/modules/web" asymkey_service "code.gitea.io/gitea/services/asymkey" "code.gitea.io/gitea/services/context" + "code.gitea.io/gitea/services/context/upload" + "code.gitea.io/gitea/services/forms" git_service "code.gitea.io/gitea/services/git" "code.gitea.io/gitea/services/gitdiff" repo_service "code.gitea.io/gitea/services/repository" @@ -417,6 +420,25 @@ func Diff(ctx *context.Context) { ctx.Data["MergedPRIssueNumber"] = pr.Index } + // Load commit comments for inline display + comments, err := issues_model.FindCommitComments(ctx, ctx.Repo.Repository.ID, commitID) + if err != nil { + log.Error("FindCommitComments: %v", err) + } else { + if err := comments.LoadPosters(ctx); err != nil { + log.Error("LoadPosters: %v", err) + } + if err := comments.LoadAttachments(ctx); err != nil { + log.Error("LoadAttachments: %v", err) + } + ctx.Data["CommitComments"] = comments + } + + // Mark this as a commit page to enable comment UI + ctx.Data["PageIsCommit"] = true + ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled + upload.AddUploadContext(ctx, "comment") + ctx.HTML(http.StatusOK, tplCommitPage) } @@ -469,3 +491,135 @@ func processGitCommits(ctx *context.Context, gitCommits []*git.Commit) ([]*git_m } return commits, nil } + +// RenderNewCommitCodeCommentForm renders the form for creating a new commit code comment +func RenderNewCommitCodeCommentForm(ctx *context.Context) { + ctx.Data["PageIsCommit"] = true + ctx.Data["AfterCommitID"] = ctx.PathParam("sha") + ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled + upload.AddUploadContext(ctx, "comment") + // Use the same template as PR new comments (defined in pull_review.go) + ctx.HTML(http.StatusOK, "repo/diff/new_comment") +} + +// CreateCommitCodeComment creates an inline comment on a commit +func CreateCommitCodeComment(ctx *context.Context) { + form := web.GetForm(ctx).(*forms.CodeCommentForm) + commitSHA := ctx.PathParam("sha") + + if ctx.Written() { + return + } + + if ctx.HasError() { + ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) + ctx.Redirect(fmt.Sprintf("%s/commit/%s", ctx.Repo.RepoLink, commitSHA)) + return + } + + // Convert line to signed line (negative for previous side) + signedLine := form.Line + if form.Side == "previous" { + signedLine *= -1 + } + + var attachments []string + if setting.Attachment.Enabled { + attachments = form.Files + } + + // Create the comment using the service layer + comment, err := repo_service.CreateCommitCodeComment( + ctx, + ctx.Doer, + ctx.Repo.Repository, + ctx.Repo.GitRepo, + commitSHA, + signedLine, + form.Content, + form.TreePath, + attachments, + ) + if err != nil { + ctx.ServerError("CreateCommitCodeComment", err) + return + } + + log.Trace("Commit comment created: %d for commit %s in %-v", comment.ID, commitSHA, ctx.Repo.Repository) + + // Render the comment + ctx.Data["Comment"] = comment + ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled + upload.AddUploadContext(ctx, "comment") + + ctx.JSON(http.StatusOK, map[string]any{ + "ok": true, + "comment": comment, + }) +} + +// UpdateCommitCodeComment updates an existing commit inline comment +func UpdateCommitCodeComment(ctx *context.Context) { + form := web.GetForm(ctx).(*forms.CodeCommentForm) + commentID := ctx.PathParamInt64(":id") + + comment, err := issues_model.GetCommentByID(ctx, commentID) + if err != nil { + ctx.ServerError("GetCommentByID", err) + return + } + + // Verify this is a commit comment + if comment.Type != issues_model.CommentTypeCode || comment.CommitSHA == "" { + ctx.NotFound(errors.New("not a commit code comment")) + return + } + + // Verify the comment belongs to this repository + if comment.PosterID != ctx.Doer.ID { + ctx.HTTPError(http.StatusForbidden) + return + } + + var attachments []string + if setting.Attachment.Enabled { + attachments = form.Files + } + + // Update the comment + if err := repo_service.UpdateCommitCodeComment(ctx, ctx.Doer, comment, form.Content, attachments); err != nil { + ctx.ServerError("UpdateCommitCodeComment", err) + return + } + + ctx.JSON(http.StatusOK, map[string]any{ + "ok": true, + }) +} + +// DeleteCommitCodeComment deletes a commit inline comment +func DeleteCommitCodeComment(ctx *context.Context) { + commentID := ctx.PathParamInt64(":id") + + comment, err := issues_model.GetCommentByID(ctx, commentID) + if err != nil { + ctx.ServerError("GetCommentByID", err) + return + } + + // Verify this is a commit comment + if comment.Type != issues_model.CommentTypeCode || comment.CommitSHA == "" { + ctx.NotFound(errors.New("not a commit code comment")) + return + } + + // Delete the comment + if err := repo_service.DeleteCommitCodeComment(ctx, ctx.Doer, comment); err != nil { + ctx.ServerError("DeleteCommitCodeComment", err) + return + } + + ctx.JSON(http.StatusOK, map[string]any{ + "ok": true, + }) +} diff --git a/routers/web/web.go b/routers/web/web.go index 5ee211b576a0c..98d3d6cc0bd9c 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1616,6 +1616,16 @@ func registerWebRoutes(m *web.Router) { m.Get("/commit/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff) m.Get("/commit/{sha:([a-f0-9]{7,64})$}/load-branches-and-tags", repo.LoadBranchesAndTags) + // Commit inline code comments + m.Group("/commit/{sha:([a-f0-9]{7,64})$}/comments", func() { + m.Get("/new", reqSignIn, repo.RenderNewCommitCodeCommentForm) + m.Post("", web.Bind(forms.CodeCommentForm{}), reqSignIn, repo.CreateCommitCodeComment) + m.Group("/{id}", func() { + m.Post("", web.Bind(forms.CodeCommentForm{}), reqSignIn, repo.UpdateCommitCodeComment) + m.Delete("", reqSignIn, repo.DeleteCommitCodeComment) + }) + }) + // FIXME: this route `/cherry-pick/{sha}` doesn't seem useful or right, the new code always uses `/_cherrypick/` which could handle branch name correctly m.Get("/cherry-pick/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, context.RepoRefByDefaultBranch(), repo.CherryPick) }, repo.MustBeNotEmpty) diff --git a/services/repository/commit_comment.go b/services/repository/commit_comment.go new file mode 100644 index 0000000000000..39769c15f7eeb --- /dev/null +++ b/services/repository/commit_comment.go @@ -0,0 +1,122 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repository + +import ( + "context" + + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" + notify_service "code.gitea.io/gitea/services/notify" +) + +// CreateCommitCodeComment creates an inline comment on a specific line of a commit +func CreateCommitCodeComment( + ctx context.Context, + doer *user_model.User, + repo *repo_model.Repository, + gitRepo *git.Repository, + commitSHA string, + line int64, + content string, + treePath string, + attachments []string, +) (*issues_model.Comment, error) { + // Validate that the commit exists + commit, err := gitRepo.GetCommit(commitSHA) + if err != nil { + log.Error("GetCommit failed: %v", err) + return nil, err + } + + // Create the comment using CreateCommentOptions + comment, err := issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{ + Type: issues_model.CommentTypeCode, + Doer: doer, + Repo: repo, + Content: content, + LineNum: line, + TreePath: treePath, + CommitSHA: commit.ID.String(), + Attachments: attachments, + }) + if err != nil { + log.Error("CreateComment failed: %v", err) + return nil, err + } + + // Load the poster for the comment + if err = comment.LoadPoster(ctx); err != nil { + log.Error("LoadPoster failed: %v", err) + return nil, err + } + + // Load attachments + if err = comment.LoadAttachments(ctx); err != nil { + log.Error("LoadAttachments failed: %v", err) + return nil, err + } + + // Send notifications for mentions (pass nil for issue since this is a commit comment) + mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, nil, doer, comment.Content) + if err != nil { + log.Error("FindAndUpdateIssueMentions failed: %v", err) + } + + // Notify about the new commit comment using CreateIssueComment + // (commit comments use the same notification path as issue comments) + notify_service.CreateIssueComment(ctx, doer, repo, nil, comment, mentions) + + return comment, nil +} + +// UpdateCommitCodeComment updates an existing commit inline comment +func UpdateCommitCodeComment( + ctx context.Context, + doer *user_model.User, + comment *issues_model.Comment, + content string, + attachments []string, +) error { + // Verify the user has permission to edit + if comment.PosterID != doer.ID { + return util.ErrPermissionDenied + } + + // Update content + oldContent := comment.Content + comment.Content = content + + if err := issues_model.UpdateComment(ctx, comment, comment.ContentVersion, doer); err != nil { + comment.Content = oldContent + return err + } + + // Update attachments if provided + if len(attachments) > 0 { + if err := issues_model.UpdateCommentAttachments(ctx, comment, attachments); err != nil { + return err + } + } + + return nil +} + +// DeleteCommitCodeComment deletes a commit inline comment +func DeleteCommitCodeComment( + ctx context.Context, + doer *user_model.User, + comment *issues_model.Comment, +) error { + // Verify the user has permission to delete + if comment.PosterID != doer.ID { + return util.ErrPermissionDenied + } + + return issues_model.DeleteComment(ctx, comment) +} diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 7eb96e1ddc5bb..3b590d9406ddb 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -185,7 +185,7 @@ {{end}} {{else}} - +
{{if $.IsSplitStyle}} {{template "repo/diff/section_split" dict "file" . "root" $}} {{else}} diff --git a/templates/repo/diff/section_split.tmpl b/templates/repo/diff/section_split.tmpl index 9953db5eb234c..95eeec236dcc8 100644 --- a/templates/repo/diff/section_split.tmpl +++ b/templates/repo/diff/section_split.tmpl @@ -46,7 +46,7 @@ {{else}}
{{if $line.LeftIdx}}{{if $leftDiff.EscapeStatus.Escaped}}{{end}}{{end}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsCommit) -}} @@ -61,7 +61,7 @@ {{if $match.RightIdx}}{{if $rightDiff.EscapeStatus.Escaped}}{{end}}{{end}} {{if $match.RightIdx}}{{end}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsCommit) -}} @@ -78,7 +78,7 @@ {{if $line.LeftIdx}}{{if $inlineDiff.EscapeStatus.Escaped}}{{end}}{{end}} {{if $line.LeftIdx}}{{end}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 2)) -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsCommit) (not (eq .GetType 2)) -}} @@ -93,7 +93,7 @@ {{if $line.RightIdx}}{{if $inlineDiff.EscapeStatus.Escaped}}{{end}}{{end}} {{if $line.RightIdx}}{{end}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 3)) -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsCommit) (not (eq .GetType 3)) -}} diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl index cb612bc27c4e8..565b205ddb9c8 100644 --- a/templates/repo/diff/section_unified.tmpl +++ b/templates/repo/diff/section_unified.tmpl @@ -53,7 +53,7 @@ {{template "repo/diff/section_code" dict "diff" $inlineDiff}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsCommit) -}} From 7ae6e20d91feb74ef8954624814e007df768f2c1 Mon Sep 17 00:00:00 2001 From: Tamas Kalman Date: Sun, 5 Oct 2025 17:53:27 -0700 Subject: [PATCH 2/2] Use separate CommentTypeCommitCode for commit inline comments Instead of reusing CommentTypeCode (which is for PR code comments), introduce a new CommentTypeCommitCode type specifically for inline comments on commits. This provides better separation of concerns and makes the codebase more maintainable. Changes: - Add CommentTypeCommitCode constant and string representation - Update comment type methods to support the new type - Update all commit comment functions to use CommentTypeCommitCode - Handle commit comments separately in updateCommentInfos --- models/issues/comment.go | 28 +++++++++++++++++++-------- routers/web/repo/commit.go | 4 ++-- services/repository/commit_comment.go | 2 +- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index e17095c82d4ca..3fef10d9c59c2 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -115,6 +115,8 @@ const ( CommentTypeUnpin // 37 unpin Issue/PullRequest CommentTypeChangeTimeEstimate // 38 Change time estimate + + CommentTypeCommitCode // 39 Comment a line of code in a commit (not part of a pull request) ) var commentStrings = []string{ @@ -157,6 +159,7 @@ var commentStrings = []string{ "pin", "unpin", "change_time_estimate", + "commit_code", } func (t CommentType) String() string { @@ -174,7 +177,7 @@ func AsCommentType(typeName string) CommentType { func (t CommentType) HasContentSupport() bool { switch t { - case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeDismissReview: + case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeDismissReview, CommentTypeCommitCode: return true } return false @@ -182,7 +185,7 @@ func (t CommentType) HasContentSupport() bool { func (t CommentType) HasAttachmentSupport() bool { switch t { - case CommentTypeComment, CommentTypeCode, CommentTypeReview: + case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeCommitCode: return true } return false @@ -190,7 +193,7 @@ func (t CommentType) HasAttachmentSupport() bool { func (t CommentType) HasMailReplySupport() bool { switch t { - case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeDismissReview, CommentTypeReopen, CommentTypeClose, CommentTypeMergePull, CommentTypeAssignees: + case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeDismissReview, CommentTypeReopen, CommentTypeClose, CommentTypeMergePull, CommentTypeAssignees, CommentTypeCommitCode: return true } return false @@ -447,6 +450,9 @@ func (c *Comment) hashLink(ctx context.Context) string { return "/files#" + c.HashTag() } } + if c.Type == CommentTypeCommitCode { + return "/files#" + c.HashTag() + } return "#" + c.HashTag() } @@ -657,9 +663,9 @@ func (c *Comment) LoadAssigneeUserAndTeam(ctx context.Context) error { return nil } -// LoadResolveDoer if comment.Type is CommentTypeCode and ResolveDoerID not zero, then load resolveDoer +// LoadResolveDoer if comment.Type is CommentTypeCode or CommentTypeCommitCode and ResolveDoerID not zero, then load resolveDoer func (c *Comment) LoadResolveDoer(ctx context.Context) (err error) { - if c.ResolveDoerID == 0 || c.Type != CommentTypeCode { + if c.ResolveDoerID == 0 || (c.Type != CommentTypeCode && c.Type != CommentTypeCommitCode) { return nil } c.ResolveDoer, err = user_model.GetUserByID(ctx, c.ResolveDoerID) @@ -674,7 +680,7 @@ func (c *Comment) LoadResolveDoer(ctx context.Context) (err error) { // IsResolved check if an code comment is resolved func (c *Comment) IsResolved() bool { - return c.ResolveDoerID != 0 && c.Type == CommentTypeCode + return c.ResolveDoerID != 0 && (c.Type == CommentTypeCode || c.Type == CommentTypeCommitCode) } // LoadDepIssueDetails loads Dependent Issue Details @@ -862,6 +868,12 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment if err = UpdateCommentAttachments(ctx, comment, opts.Attachments); err != nil { return err } + case CommentTypeCommitCode: + if err = UpdateCommentAttachments(ctx, comment, opts.Attachments); err != nil { + return err + } + // Commit comments don't have an associated issue, so just return here + return nil case CommentTypeReopen, CommentTypeClose: if err = repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, true); err != nil { return err @@ -1081,7 +1093,7 @@ func FindCommitComments(ctx context.Context, repoID int64, commitSHA string) (Co comments := make([]*Comment, 0, 10) return comments, db.GetEngine(ctx). Where("commit_sha = ?", commitSHA). - And("type = ?", CommentTypeCode). + And("type = ?", CommentTypeCommitCode). Asc("created_unix"). Asc("id"). Find(&comments) @@ -1093,7 +1105,7 @@ func FindCommitLineComments(ctx context.Context, commitSHA, treePath string) (Co return comments, db.GetEngine(ctx). Where("commit_sha = ?", commitSHA). And("tree_path = ?", treePath). - And("type = ?", CommentTypeCode). + And("type = ?", CommentTypeCommitCode). Asc("created_unix"). Asc("id"). Find(&comments) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index b2d3eda9f1338..8b0feece491f1 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -570,7 +570,7 @@ func UpdateCommitCodeComment(ctx *context.Context) { } // Verify this is a commit comment - if comment.Type != issues_model.CommentTypeCode || comment.CommitSHA == "" { + if comment.Type != issues_model.CommentTypeCommitCode || comment.CommitSHA == "" { ctx.NotFound(errors.New("not a commit code comment")) return } @@ -608,7 +608,7 @@ func DeleteCommitCodeComment(ctx *context.Context) { } // Verify this is a commit comment - if comment.Type != issues_model.CommentTypeCode || comment.CommitSHA == "" { + if comment.Type != issues_model.CommentTypeCommitCode || comment.CommitSHA == "" { ctx.NotFound(errors.New("not a commit code comment")) return } diff --git a/services/repository/commit_comment.go b/services/repository/commit_comment.go index 39769c15f7eeb..ca388dad7f478 100644 --- a/services/repository/commit_comment.go +++ b/services/repository/commit_comment.go @@ -36,7 +36,7 @@ func CreateCommitCodeComment( // Create the comment using CreateCommentOptions comment, err := issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{ - Type: issues_model.CommentTypeCode, + Type: issues_model.CommentTypeCommitCode, Doer: doer, Repo: repo, Content: content,