fix(web): more GitLab merge request links open their file, line or commit

apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx

@@ -3,6 +3,7 @@ import {
   pullRequestAnchorMatchesUrl,
   pullRequestDiffPathMatches,
   pullRequestLinkCommit,
+  resolvePullRequestAnchorRange,
   resolvePullRequestReviewAnchor,
 } from "@t3tools/shared/pullRequestAnchor";
 import { AppActivityIndicator } from "../../components/ui/AppActivityIndicator";
@@ -1072,7 +1073,15 @@ function PullRequestFileRouteContent({ route }: PullRequestFileRouteProps) {
       : !jumpPath || requestedFile
         ? data.files[0]
         : undefined);
-  const range = pickedPath === null && linkAnchor?.kind === "diff" ? linkAnchor.range : null;
+  const linkRange = pickedPath === null && linkAnchor?.kind === "diff" ? linkAnchor.range : null;
+  const range =
+    linkRange &&
+    resolvePullRequestAnchorRange(linkRange, (line) =>
+      data.rows.some(
+        (row) =>
+          row.fileId === targetFile?.id && row.change === "delete" && row.oldLineNumber === line,
+      ),
+    );
   const focus =
     route.params.focus ??
     (range

apps/web/src/components/pullRequest/PullRequestCodeTab.tsx

@@ -2,6 +2,7 @@ import {
   parsePullRequestAnchor,
   pullRequestDiffPathMatches,
   pullRequestLinkCommit,
+  resolvePullRequestAnchorRange,
 } from "@t3tools/shared/pullRequestAnchor";
 import {
   capitalizeTerm,
@@ -349,6 +350,18 @@ export interface PullRequestAgentSelectionInput {
   readonly request: string;
 }
 
+function isRemovedLine(fileDiff: FileDiffMetadata, oldLine: number): boolean {
+  return fileDiff.hunks.some((hunk) => {
+    let next = hunk.deletionStart;
+    return hunk.hunkContent.some((content) => {
+      const count = content.type === "context" ? content.lines : content.deletions;
+      const removed = content.type === "change" && oldLine >= next && oldLine < next + count;
+      next += count;
+      return removed;
+    });
+  });
+}
+
 function PullRequestCodeTab({
   scrollerRef,
   environmentId,
@@ -1556,7 +1569,12 @@ function PullRequestCodeTab({
     answeredLink.current = key;
     setRevealedLink({ scope: scopeKey, path: pendingLinkPath });
     onRevealSettled?.(revealUrl);
-    const range = linkAnchor.range;
+    const range =
+      linkAnchor.range &&
+      resolvePullRequestAnchorRange(
+        linkAnchor.range,
+        (line) => linkFile !== undefined && isRemovedLine(linkFile, line),
+      );
     revealFile(
       pendingLinkPath,
       range

packages/shared/src/pullRequestAnchor.test.ts

@@ -4,9 +4,12 @@ import {
   pullRequestAnchorMatchesUrl,
   pullRequestDiffPathMatches,
   pullRequestLinkCommit,
+  resolvePullRequestAnchorRange,
   resolvePullRequestReviewAnchor,
 } from "./pullRequestAnchor.ts";
 
+const gitlab = "https://gitlab.com/group/repo/-/merge_requests/7";
+
 const url = "https://github.com/owner/repo/pull/7";
 it.each(["discussion_r123", "r123", "discussion-diff-123R5-L9", "discussion-diff-123"])(
   "resolves review comment alias %s",
@@ -53,19 +56,22 @@ it.each([
     "src/hello world.ts",
     "bcea8deb9d67b305e28c4a0b0d021615",
     "066802cf2cf056706e4e4951cb7dff2717a2ae32fd9797c1f1b41062d57db929",
+    "f3f78c2026bd34f4ad36bf36e9c557dd9df528fb",
   ],
   [
     "src/日本語.ts",
     "5c66d5d565666e46924cf80d7bbfdab3",
     "191b8ebf2241225db999eaf8b6ff44b07934d577f347d0ff684591eecad34944",
+    "2ec8ce696c4c6df9419ad640fbc0d6ef4d79ebad",
   ],
   [
     "old/path.ts",
     "f5d455478d496859b654884654393a61",
     "aa38d67014e2c59d51ea456a155e039f4e32f636a4c450714c3b517bb8bde7e7",
+    "ca0d69af01140f25bafd6ab7aec204ce52a9bbb3",
   ],
-])("matches GitHub path digests for %s", (path, legacy, current) => {
-  for (const digest of [legacy, current]) {
+])("matches GitHub and GitLab path digests for %s", (path, legacy, current, gitLab) => {
+  for (const digest of [legacy, current, gitLab, gitLab.slice(0, 9)]) {
     expect(pullRequestDiffPathMatches(digest, path)).toBe(true);
     expect(pullRequestDiffPathMatches(digest, "other.ts")).toBe(false);
   }
@@ -77,6 +83,19 @@ it("handles encoded hashes and rejects invalid line numbers and URLs", () => {
   });
   expect(parsePullRequestAnchor(`${url}#diff-abcR0`)).toBeNull();
   expect(parsePullRequestAnchor(`${url}#diff-abcR9999999999999999999`)).toBeNull();
+  const sha = "c".repeat(40);
+  for (const anchor of [
+    `${sha}_0_0`,
+    `${sha}_1_9999999999999999999`,
+    `line_${sha.slice(0, 9)}_0`,
+  ]) {
+    expect(parsePullRequestAnchor(`${gitlab}/diffs#${anchor}`)).toMatchObject({ range: null });
+  }
+  expect(parsePullRequestAnchor(`${gitlab}/diffs#${sha}_0_5`)).toEqual({
+    kind: "diff",
+    digest: sha,
+    range: { start: 5, side: "right", end: 5, endSide: "right" },
+  });
   expect(parsePullRequestAnchor("invalid")).toBeNull();
   expect(parsePullRequestAnchor(url)).toBeNull();
   expect(parsePullRequestAnchor(`${url}#unknown`)).toEqual({ kind: "summary", anchor: "unknown" });
@@ -89,9 +108,44 @@ it("keeps the commit scope of a PR diff permalink", () => {
   const sha = "a".repeat(40);
   expect(pullRequestLinkCommit(`${url}/commits/${sha}#diff-abcR5`)).toBe(sha);
   expect(pullRequestLinkCommit(`${url}/files#diff-abcR5`)).toBeNull();
+  expect(pullRequestLinkCommit(`${gitlab}/diffs?commit_id=${sha}#${"b".repeat(40)}`)).toBe(sha);
+  expect(pullRequestLinkCommit(`${gitlab}/diffs?commit_id=main`)).toBeNull();
+  expect(pullRequestLinkCommit(`${gitlab}?commit_id=${sha}`)).toBeNull();
+});
+
+it("reads GitLab file and line anchors, letting a removed old line win", () => {
+  const digest = "c".repeat(40);
+  const diff = (range: object | null, at = digest) => ({ kind: "diff", digest: at, range });
+  const line = (start: number, side: "left" | "right") => ({
+    start,
+    side,
+    end: start,
+    endSide: side,
+  });
+  expect(parsePullRequestAnchor(`${gitlab}/diffs#${digest}`)).toEqual(diff(null));
+  expect(parsePullRequestAnchor(`${gitlab}/diffs#diff-content-${digest}`)).toEqual(diff(null));
+  expect(parsePullRequestAnchor(`${gitlab}/diffs?file=${digest}`)).toEqual(diff(null));
+  expect(parsePullRequestAnchor(`${gitlab}/diffs?file=${digest}#note_5`)).toEqual({
+    kind: "timeline",
+    anchor: "note_5",
+  });
+  const code = parsePullRequestAnchor(`${gitlab}/diffs#${digest}_248_253`);
+  expect(code).toEqual(diff({ ...line(253, "right"), oldLine: 248 }));
+  const range = code?.kind === "diff" ? code.range : null;
+  expect(resolvePullRequestAnchorRange(range!, () => false)).toBe(range);
+  expect(resolvePullRequestAnchorRange(range!, (old) => old === 248)).toEqual(line(248, "left"));
+  expect(parsePullRequestAnchor(`${gitlab}/diffs#${digest}_12_0`)).toEqual(diff(line(12, "left")));
+  expect(parsePullRequestAnchor(`${gitlab}/diffs#line_${digest.slice(0, 9)}_A12`)).toEqual(
+    diff(line(12, "right"), digest.slice(0, 9)),
+  );
+  expect(parsePullRequestAnchor(`${gitlab}/diffs#line_${digest.slice(0, 9)}_12`)).toEqual(
+    diff(line(12, "left"), digest.slice(0, 9)),
+  );
+  expect(parsePullRequestAnchor(`${gitlab}#${digest}_248_253`)).toEqual(code);
+  expect(parsePullRequestAnchor(`${gitlab}#${digest}`)).toEqual(diff(null));
+  expect(parsePullRequestAnchor(`${gitlab}?file=${digest}`)).toBeNull();
 });
 
-const gitlab = "https://gitlab.com/group/repo/-/merge_requests/7";
 it.each([
   [`${url}/files`, "code"],
   [`${url}/files/`, "code"],
@@ -133,22 +187,3 @@ it("finds a review with no text of its own through the earliest comment it poste
   );
   expect(resolvePullRequestReviewAnchor("issuecomment-9", [], threads)).toBe("issuecomment-9");
 });
-
-it("reads GitLab merge request file and line anchors", () => {
-  const path = "internal/commands/issuable/view/issuable_view_test.go";
-  const digest = "0cd3702f060909d3161d1095b8347d4d07671427";
-  expect(pullRequestDiffPathMatches(digest, path)).toBe(true);
-  expect(parsePullRequestAnchor(`${gitlab}/diffs#${digest}`)).toEqual({
-    kind: "diff",
-    digest,
-    range: null,
-  });
-  expect(parsePullRequestAnchor(`${gitlab}/diffs#${digest}_248_253`)).toEqual({
-    kind: "diff",
-    digest,
-    range: { start: 253, side: "right", end: 253, endSide: "right" },
-  });
-  expect(parsePullRequestAnchor(`${gitlab}/diffs#${digest}_12_0`)).toMatchObject({
-    range: { start: 12, side: "left" },
-  });
-});

packages/shared/src/pullRequestAnchor.ts

@@ -7,6 +7,8 @@ export interface PullRequestAnchorRange {
   readonly side: "left" | "right";
   readonly end: number;
   readonly endSide: "left" | "right";
+  /** A GitLab line code's old-side counter: the line it names when that old line was removed. */
+  readonly oldLine?: number;
 }
 
 export type PullRequestAnchor =
@@ -57,6 +59,55 @@ function pullRequestPathTab(pathname: string): PullRequestAnchor | null {
   }
 }
 
+const isLine = (line: number) => Number.isSafeInteger(line) && line >= 1;
+
+function gitLabLine(
+  start: number,
+  side: "left" | "right",
+  oldLine = 0,
+): PullRequestAnchorRange | null {
+  if (!isLine(start)) return null;
+  return { start, side, end: start, endSide: side, ...(isLine(oldLine) ? { oldLine } : {}) };
+}
+
+// GitLab names a file by its path's SHA-1 and a line by `<sha1>_<old>_<new>`, which carries both
+// sides' counters. Rapid Diffs writes `line_<sha1 prefix>_<old>` or `_A<new>`.
+function gitLabDiffAnchor(anchor: string, file: string | null): PullRequestAnchor | null {
+  const rapid = /^line_([a-f0-9]{9})_(A?)(\d+)$/i.exec(anchor);
+  if (rapid) {
+    const line = Number(rapid[3]);
+    return {
+      kind: "diff",
+      digest: (rapid[1] ?? "").toLowerCase(),
+      range: gitLabLine(line, rapid[2] ? "right" : "left"),
+    };
+  }
+  const code = /^(?:diff-content-)?([a-f0-9]{40})(?:_(\d+)_(\d+))?$/i.exec(anchor);
+  // A comment anchor beside `?file=` names the comment.
+  const digest =
+    code?.[1] ?? (!anchor && file !== null && /^[a-f0-9]{40}$/i.test(file) ? file : null);
+  if (digest === null) return null;
+  const [oldLine, newLine] = [Number(code?.[2]), Number(code?.[3])];
+  // A deleted file has no new side, so its new counter is 0.
+  const range =
+    code?.[2] === undefined
+      ? null
+      : newLine === 0
+        ? gitLabLine(oldLine, "left")
+        : gitLabLine(newLine, "right", oldLine);
+  return { kind: "diff", digest: digest.toLowerCase(), range };
+}
+
+/** The range to select, once the diff shows whether a GitLab line code's old line was removed. */
+export function resolvePullRequestAnchorRange(
+  range: PullRequestAnchorRange,
+  isRemovedLine: (oldLine: number) => boolean,
+): PullRequestAnchorRange {
+  return range.oldLine !== undefined && isRemovedLine(range.oldLine)
+    ? (gitLabLine(range.oldLine, "left") ?? range)
+    : range;
+}
+
 export function parsePullRequestAnchor(url: string | null | undefined): PullRequestAnchor | null {
   let parsed: URL;
   try {
@@ -66,6 +117,13 @@ export function parsePullRequestAnchor(url: string | null | undefined): PullRequ
   }
   const fallback = pullRequestPathTab(parsed.pathname);
   const anchor = pullRequestUrlHash(url);
+  const gitLab = gitLabDiffAnchor(
+    anchor,
+    /\/-\/merge_requests\/\d+\/diffs\/?$/i.test(parsed.pathname)
+      ? parsed.searchParams.get("file")
+      : null,
+  );
+  if (gitLab) return gitLab;
   if (!anchor) return fallback;
   // GitHub line ranges may cross diff sides; file digests were MD5 before SHA-256.
   const diff = /^diff-([a-f0-9]+)(?:([LR])(\d+)(?:-([LR])(\d+))?)?$/i.exec(anchor);
@@ -90,19 +148,6 @@ export function parsePullRequestAnchor(url: string | null | undefined): PullRequ
         : null,
     };
   }
-  // GitLab: SHA-1 of the path, then old and new line counters. A removed line still carries a
-  // new counter, so the new side wins unless the file has none (a 0 for a deleted file).
-  const gitlab = /^([a-f0-9]{40})(?:_(\d+)_(\d+))?$/i.exec(anchor);
-  if (gitlab) {
-    const digest = (gitlab[1] ?? "").toLowerCase();
-    if (!gitlab[2]) return { kind: "diff", digest, range: null };
-    const newLine = Number(gitlab[3]);
-    const oldLine = Number(gitlab[2]);
-    const side = newLine >= 1 ? "right" : "left";
-    const line = side === "right" ? newLine : oldLine;
-    if (!Number.isSafeInteger(line) || line < 1) return fallback;
-    return { kind: "diff", digest, range: { start: line, side, end: line, endSide: side } };
-  }
   if (
     /^(?:(?:issuecomment-|pullrequestreview-|discussion_r|r|commitcomment-|event-|note_|comment-)\d+|discussion-diff-\d+(?:[LR]\d+(?:-[LR]\d+)?)?)$/i.test(
       anchor,
@@ -149,14 +194,20 @@ export function resolvePullRequestReviewAnchor(
 
 export function pullRequestDiffPathMatches(digest: string, path: string): boolean {
   const bytes = utf8ToBytes(path);
-  const hash = digest.length === 32 ? md5 : digest.length === 40 ? sha1 : sha256;
-  return digest === bytesToHex(hash(bytes));
+  // GitLab uses SHA-1, abbreviated to 9 characters in Rapid Diffs line anchors.
+  if (digest.length === 40 || digest.length === 9)
+    return bytesToHex(sha1(bytes)).startsWith(digest);
+  return digest === bytesToHex(digest.length === 32 ? md5(bytes) : sha256(bytes));
 }
 
 export function pullRequestLinkCommit(url: string | null | undefined): string | null {
   if (!url) return null;
   try {
-    return /\/pull\/\d+\/commits\/([a-f0-9]{40})\/?$/i.exec(new URL(url).pathname)?.[1] ?? null;
+    const { pathname, searchParams } = new URL(url);
+    const commit = /\/-\/merge_requests\/\d+\/diffs\/?$/i.test(pathname)
+      ? searchParams.get("commit_id")
+      : /\/pull\/\d+\/commits\/([a-f0-9]{40})\/?$/i.exec(pathname)?.[1];
+    return commit && /^[a-f0-9]{40}$/i.test(commit) ? commit : null;
   } catch {
     return null;
   }