fix

Author: wxiaoguang

routers/web/repo/compare.go

@@ -640,6 +640,11 @@ func PrepareCompareDiff(
 	}
 	ctx.Data["DiffShortStat"] = diffShortStat
 	ctx.Data["Diff"] = diff
+	ctx.Data["DiffBlobExcerptData"] = &gitdiff.DiffBlobExcerptData{
+		BaseLink:      ctx.Repo.RepoLink + "/blob_excerpt",
+		DiffStyle:     ctx.FormString("style"),
+		AfterCommitID: headCommitID,
+	}
 	ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0
 
 	if !fileOnly {
@@ -979,32 +984,29 @@ func ExcerptBlob(ctx *context.Context) {
 			section.Lines = append(section.Lines, lineSection)
 		}
 	}
-	if ctx.Doer != nil {
-		ctx.Data["SignedUserID"] = ctx.Doer.ID
-	}
-	isPull := ctx.FormBool("is_pull")
-	ctx.Data["PageIsPullFiles"] = isPull
 
-	if isPull {
+	diffBlobExcerptData.PullIssueIndex = ctx.FormInt64("pull_issue_index")
+	if diffBlobExcerptData.PullIssueIndex > 0 {
 		if !ctx.Repo.CanRead(unit.TypePullRequests) {
 			ctx.NotFound(nil)
 			return
 		}
-		issueIndex := ctx.FormInt64("issue_index")
-		diffBlobExcerptData.PullIndex = issueIndex
-		ctx.Data["IssueIndex"] = issueIndex
-		if issueIndex > 0 {
-			if issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, issueIndex); err == nil && issue.IsPull {
-				ctx.Data["Issue"] = issue
-				ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool {
-					return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee)
-				}
 
-				if allComments, err := issues_model.FetchCodeComments(ctx, issue, ctx.Doer, ctx.FormBool("show_outdated")); err == nil {
-					if lineComments, ok := allComments[filePath]; ok {
-						attachCommentsToLines(section, lineComments)
-						attachHiddenCommentIDs(section, lineComments)
-					}
+		issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, diffBlobExcerptData.PullIssueIndex)
+		if err != nil {
+			log.Error("GetIssueByIndex error: %v", err)
+		} else if issue.IsPull {
+			ctx.Data["Issue"] = issue
+			ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool {
+				return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee)
+			}
+			allComments, err := issues_model.FetchCodeComments(ctx, issue, ctx.Doer, ctx.FormBool("show_outdated"))
+			if err != nil {
+				log.Error("FetchCodeComments error: %v", err)
+			} else {
+				if lineComments, ok := allComments[filePath]; ok {
+					attachCommentsToLines(section, lineComments)
+					attachHiddenCommentIDs(section, lineComments)
 				}
 			}
 		}

routers/web/repo/compare_test.go

@@ -43,9 +43,9 @@ func TestAttachHiddenCommentIDs(t *testing.T) {
 
 	attachHiddenCommentIDs(section, lineComments)
 
-	assert.Equal(t, []int64{100}, section.Lines[0].HiddenCommentIDs)
-	assert.Nil(t, section.Lines[1].HiddenCommentIDs)
-	assert.Equal(t, []int64{200}, section.Lines[2].HiddenCommentIDs)
+	assert.Equal(t, []int64{100}, section.Lines[0].SectionInfo.HiddenCommentIDs)
+	assert.Nil(t, section.Lines[1].SectionInfo.HiddenCommentIDs)
+	assert.Equal(t, []int64{200}, section.Lines[2].SectionInfo.HiddenCommentIDs)
 }
 
 func TestAttachCommentsToLines(t *testing.T) {

routers/web/repo/pull.go

@@ -828,10 +828,10 @@ func viewPullFiles(ctx *context.Context, beforeCommitID, afterCommitID string) {
 
 	ctx.Data["Diff"] = diff
 	ctx.Data["DiffBlobExcerptData"] = &gitdiff.DiffBlobExcerptData{
-		BaseLink:      ctx.Repo.RepoLink + "/blob_excerpt",
-		PullIndex:     pull.Index,
-		DiffStyle:     ctx.FormString("style"),
-		AfterCommitID: afterCommitID,
+		BaseLink:       ctx.Repo.RepoLink + "/blob_excerpt",
+		PullIssueIndex: pull.Index,
+		DiffStyle:      ctx.FormString("style"),
+		AfterCommitID:  afterCommitID,
 	}
 	ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0
 

services/gitdiff/gitdiff.go

@@ -91,7 +91,6 @@ type DiffLineSectionInfo struct {
 	RightHunkSize int
 
 	HiddenCommentIDs []int64 // IDs of hidden comments in this section
-
 }
 
 // DiffHTMLOperation is the HTML version of diffmatchpatch.Diff
@@ -174,11 +173,11 @@ func (d *DiffLine) getExpandDirection() string {
 }
 
 type DiffBlobExcerptData struct {
-	BaseLink      string
-	IsWikiRepo    bool
-	PullIndex     int64
-	DiffStyle     string
-	AfterCommitID string
+	BaseLink       string
+	IsWikiRepo     bool
+	PullIssueIndex int64
+	DiffStyle      string
+	AfterCommitID  string
 }
 
 func (d *DiffLine) RenderBlobExcerptButtons(fileNameHash string, data *DiffBlobExcerptData) template.HTML {
@@ -187,16 +186,13 @@ func (d *DiffLine) RenderBlobExcerptButtons(fileNameHash string, data *DiffBlobE
 
 	makeButton := func(direction, svgName string) template.HTML {
 		style := util.IfZero(data.DiffStyle, "unified")
-		link := data.BaseLink + "/" + data.AfterCommitID + fmt.Sprintf("?style=%s&direction=%s&anchor=%s", url.QueryEscape(style), direction, url.QueryEscape(anchor))
-		if data.PullIndex > 0 {
-			link += fmt.Sprintf("&is_pull=1&issue_index=%d", data.PullIndex)
+		link := data.BaseLink + "/" + data.AfterCommitID + fmt.Sprintf("?style=%s&direction=%s&anchor=%s", url.QueryEscape(style), direction, url.QueryEscape(anchor)) + "&" + d.getBlobExcerptQuery()
+		if data.PullIssueIndex > 0 {
+			link += fmt.Sprintf("&pull_issue_index=%d", data.PullIssueIndex)
 		}
-		link += "&" + d.getBlobExcerptQuery()
-
-		svgContent := svg.RenderHTML(svgName)
 		return htmlutil.HTMLFormat(
 			`<button class="code-expander-button" hx-target="closest tr" hx-get="%s"  data-hidden-comment-ids="%s">%s</button>`,
-			link, dataHiddenCommentIDs, svgContent,
+			link, dataHiddenCommentIDs, svg.RenderHTML(svgName),
 		)
 	}
 	var content template.HTML

services/pull/review.go

@@ -286,11 +286,11 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
 		}
 
 		// If patch is still empty (unchanged line), generate code context
-		if len(patch) == 0 && len(commitID) > 0 {
+		if patch == "" && commitID != "" {
 			patch, err = gitdiff.GeneratePatchForUnchangedLine(gitRepo, commitID, treePath, line, setting.UI.CodeCommentLines)
 			if err != nil {
 				// Log the error but don't fail comment creation
-				log.Warn("Unable to generate patch for unchanged line (file=%s, line=%d, commit=%s): %v", treePath, line, commitID, err)
+				log.Debug("Unable to generate patch for unchanged line (file=%s, line=%d, commit=%s): %v", treePath, line, commitID, err)
 			}
 		}
 	}

templates/repo/diff/blob_excerpt.tmpl

@@ -70,7 +70,7 @@
 		<td class="lines-escape">{{if $inlineDiff.EscapeStatus.Escaped}}<button class="toggle-escape-button btn interact-bg" title="{{template "repo/diff/escape_title" dict "diff" $inlineDiff}}"></button>{{end}}</td>
 		<td class="lines-type-marker"><span class="tw-font-mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span></td>
 		<td class="lines-code{{if (not $line.RightIdx)}} lines-code-old{{end}}">
-			{{- if and $.SignedUserID $.PageIsPullFiles -}}
+			{{- if and ctx.RootData.SignedUserID $diffBlobExcerptData.PullIssueIndex -}}
 				<button type="button" aria-label="{{ctx.Locale.Tr "repo.diff.comment.add_line_comment"}}" class="ui primary button add-code-comment add-code-comment-{{if $line.RightIdx}}right{{else}}left{{end}}{{if (not $line.CanComment)}} tw-invisible{{end}}" data-side="{{if $line.RightIdx}}right{{else}}left{{end}}" data-idx="{{if $line.RightIdx}}{{$line.RightIdx}}{{else}}{{$line.LeftIdx}}{{end}}">
 					{{- svg "octicon-plus" -}}
 				</button>

web_src/css/review.css

@@ -128,7 +128,7 @@
   color: var(--color-primary-contrast);
   border-radius: 8px !important;
   left: 4px;
-  top: 4px;
+  top: 6px;
   text-align: center;
 }
 

web_src/js/features/repo-diff.ts

@@ -9,7 +9,7 @@ import {submitEventSubmitter, queryElemSiblings, hideElem, showElem, animateOnce
 import {POST, GET} from '../modules/fetch.ts';
 import {createTippy} from '../modules/tippy.ts';
 import {invertFileFolding} from './file-fold.ts';
-import {parseDom} from '../utils.ts';
+import {parseDom, sleep} from '../utils.ts';
 import {registerGlobalSelectorFunc} from '../modules/observer.ts';
 
 const {i18n} = window.config;
@@ -219,40 +219,32 @@ function initRepoDiffShowMore() {
 }
 
 async function loadUntilFound() {
-  const hashTargetSelector = window.location.hash;
-  if (!hashTargetSelector.startsWith('#diff-') && !hashTargetSelector.startsWith('#issuecomment-')) {
+  const currentHash = window.location.hash;
+  if (!currentHash.startsWith('#diff-') && !currentHash.startsWith('#issuecomment-')) {
     return;
   }
 
-  let wasExpanded = false;
-
-  while (true) {
+  while (window.location.hash === currentHash) {
     // use getElementById to avoid querySelector throws an error when the hash is invalid
     // eslint-disable-next-line unicorn/prefer-query-selector
-    const targetElement = document.getElementById(hashTargetSelector.substring(1));
+    const targetElement = document.getElementById(currentHash.substring(1));
     if (targetElement) {
-      // If we just expanded content, we need to re-trigger :target CSS
-      if (wasExpanded) {
-        const currentHash = window.location.hash;
-        window.location.hash = '';
-        setTimeout(() => {
-          window.location.hash = currentHash;
-        }, 10);
-      } else {
-        targetElement.scrollIntoView();
-      }
+      // re-trigger :target CSS and browser will scroll to the element
+      window.location.hash = '';
+      setTimeout(() => { window.location.hash = currentHash }, 0);
       return;
     }
 
-    // If looking for a comment, try to expand the section that contains it
-    if (hashTargetSelector.startsWith('#issuecomment-')) {
-      const commentId = hashTargetSelector.substring('#issuecomment-'.length);
+    // If looking for a hidden comment, try to expand the section that contains it
+    if (currentHash.startsWith('#issuecomment-')) {
+      const commentId = currentHash.substring('#issuecomment-'.length);
       const expandButton = findExpandButtonForComment(commentId);
       if (expandButton) {
+        // Avoid infinite loop, do not re-click the button if already clicked
+        if (expandButton.hasAttribute('data-auto-load-clicked')) return;
+        expandButton.setAttribute('data-auto-load-clicked', 'true');
         expandButton.click();
-        wasExpanded = true;
-        // Wait for HTMX to load the content
-        await new Promise((resolve) => setTimeout(resolve, 500));
+        await sleep(500); // Wait for HTMX to load the content. FIXME: need to drop htmx in the future
         continue; // Try again to find the element
       }
     }
@@ -273,16 +265,13 @@ async function loadUntilFound() {
 function findExpandButtonForComment(commentId: string): HTMLElement | null {
   const expandButtons = document.querySelectorAll('.code-expander-button[data-hidden-comment-ids]');
   for (const button of expandButtons) {
-    const hiddenIds = button.getAttribute('data-hidden-comment-ids');
-    if (hiddenIds) {
-      // Parse the comma-separated list of IDs
-      const ids = hiddenIds.replace(/[[\]]/g, '').split(' ').filter(Boolean);
-      if (ids.includes(commentId)) {
-        return button as HTMLElement;
-      }
+    const hiddenIdsAttr = button.getAttribute('data-hidden-comment-ids');
+    if (!hiddenIdsAttr) continue;
+    const hiddenIds = hiddenIdsAttr.split(',').filter(Boolean);
+    if (hiddenIds.includes(commentId)) {
+      return button as HTMLElement;
     }
   }
-
   return null;
 }