mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 00:03:20 +00:00
Merge remote-tracking branch 'origin/main' into brennanb2025/fix-pin-duplication
This commit is contained in:
+23
-12
@@ -485,16 +485,32 @@ function mapIssueWorkItem(item: Record<string, unknown>): MainWorkItem {
|
||||
.filter(Boolean)
|
||||
: [],
|
||||
updatedAt: String(item.updated_at ?? item.updatedAt ?? ''),
|
||||
author:
|
||||
typeof item.user === 'object' && item.user !== null && 'login' in item.user
|
||||
? String((item.user as { login?: unknown }).login ?? '')
|
||||
: typeof item.author === 'object' && item.author !== null && 'login' in item.author
|
||||
? String((item.author as { login?: unknown }).login ?? '')
|
||||
: null,
|
||||
...authorFieldsFromUnknown(item),
|
||||
...(item.assignees !== undefined ? { assignees: usersFromUnknown(item.assignees) } : {})
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive both the author login and its API avatar_url in one place so GHE
|
||||
* avatars render (the login-only `github.com/{login}.png` URL 404s on GHE).
|
||||
*
|
||||
* REST exposes `user.avatar_url`; gh/GraphQL expose `author.avatarUrl`. `gh pr
|
||||
* view` omits the avatar entirely, so authorAvatarUrl is left undefined and the
|
||||
* UI falls back to the login URL then a placeholder. See #8784.
|
||||
*/
|
||||
function authorFieldsFromUnknown(
|
||||
item: Record<string, unknown>
|
||||
): Pick<MainWorkItem, 'author' | 'authorAvatarUrl'> {
|
||||
const user = userFromUnknown(item.user ?? item.author)
|
||||
if (!user) {
|
||||
return { author: null }
|
||||
}
|
||||
return {
|
||||
author: user.login,
|
||||
...(user.avatarUrl ? { authorAvatarUrl: user.avatarUrl } : {})
|
||||
}
|
||||
}
|
||||
|
||||
function extractHeadOwnerLogin(item: Record<string, unknown>): string | null {
|
||||
// gh CLI `pr list --json headRepositoryOwner` shape: { login }
|
||||
if (typeof item.headRepositoryOwner === 'object' && item.headRepositoryOwner !== null) {
|
||||
@@ -746,12 +762,7 @@ function mapPullRequestWorkItem(
|
||||
.filter(Boolean)
|
||||
: [],
|
||||
updatedAt: String(item.updated_at ?? item.updatedAt ?? ''),
|
||||
author:
|
||||
typeof item.user === 'object' && item.user !== null && 'login' in item.user
|
||||
? String((item.user as { login?: unknown }).login ?? '')
|
||||
: typeof item.author === 'object' && item.author !== null && 'login' in item.author
|
||||
? String((item.author as { login?: unknown }).login ?? '')
|
||||
: null,
|
||||
...authorFieldsFromUnknown(item),
|
||||
branchName:
|
||||
typeof item.head === 'object' && item.head !== null && 'ref' in item.head
|
||||
? String((item.head as { ref?: unknown }).ref ?? '')
|
||||
|
||||
@@ -216,6 +216,56 @@ describe('getWorkItemDetails', () => {
|
||||
expect(details?.participants?.[0]?.login).toBe('issue-author')
|
||||
})
|
||||
|
||||
it('enriches a non-participating assignee avatar from the GraphQL assignees connection', async () => {
|
||||
// Why: a GHE assignee who never commented is absent from `participants`, so
|
||||
// the enrichment must also draw avatars from the `assignees` connection or
|
||||
// item.assignees keeps the blank avatar `gh` returns.
|
||||
getWorkItemMock.mockResolvedValueOnce({
|
||||
id: 'issue:924',
|
||||
type: 'issue',
|
||||
number: 924,
|
||||
title: 'Assignee avatar',
|
||||
state: 'open',
|
||||
url: 'https://github.com/acme/widgets/issues/924',
|
||||
labels: [],
|
||||
updatedAt: '2026-04-01T00:00:00Z',
|
||||
author: 'issue-author',
|
||||
assignees: [{ login: 'ghe-assignee', name: 'GHE Assignee', avatarUrl: '' }]
|
||||
})
|
||||
getIssueOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
data: {
|
||||
repository: {
|
||||
issue: {
|
||||
body: 'Issue body',
|
||||
assignees: {
|
||||
nodes: [
|
||||
{
|
||||
login: 'ghe-assignee',
|
||||
name: 'GHE Assignee',
|
||||
avatarUrl: 'https://ghe.example.com/avatars/ghe-assignee'
|
||||
}
|
||||
]
|
||||
},
|
||||
participants: { nodes: [{ login: 'issue-author', avatarUrl: 'https://x/y' }] },
|
||||
comments: { nodes: [] }
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
.mockResolvedValueOnce({ stdout: '' })
|
||||
|
||||
const details = await getWorkItemDetails('/repo-root', 924, 'issue')
|
||||
|
||||
expect(details?.item.assignees?.[0]).toMatchObject({
|
||||
login: 'ghe-assignee',
|
||||
avatarUrl: 'https://ghe.example.com/avatars/ghe-assignee'
|
||||
})
|
||||
})
|
||||
|
||||
it('caps issue timeline pagination by supported activity items', async () => {
|
||||
getWorkItemMock.mockResolvedValueOnce({
|
||||
id: 'issue:923',
|
||||
@@ -568,4 +618,66 @@ describe('getWorkItemDetails', () => {
|
||||
expect(details?.filesUnavailable).toBe(false)
|
||||
expect(details?.files).toEqual([])
|
||||
})
|
||||
|
||||
// Why: `gh pr view` omits avatar_url, so the login-based github.com URL 404s on
|
||||
// GHE. getWorkItemDetails must resolve author/reviewer/assignee avatars via the
|
||||
// GraphQL user(login:) batch and stamp them onto the returned item. See #8784.
|
||||
it('enriches PR author, reviewer, and assignee avatars from the GraphQL user lookup', async () => {
|
||||
getWorkItemMock.mockResolvedValueOnce({
|
||||
id: 'pr:1102',
|
||||
type: 'pr',
|
||||
number: 1102,
|
||||
title: 'Enterprise PR',
|
||||
state: 'open',
|
||||
url: 'https://ghe.example.com/acme/widgets/pull/1102',
|
||||
labels: [],
|
||||
updatedAt: '2026-07-11T00:00:00Z',
|
||||
author: 'seah',
|
||||
reviewRequests: [{ login: 'ludi', name: null, avatarUrl: '' }],
|
||||
latestReviews: [{ login: 'inho', state: 'APPROVED', avatarUrl: '' }],
|
||||
// A default `u/0` placeholder must be replaced by the resolved avatar.
|
||||
assignees: [{ login: 'seah', name: 'Seah', avatarUrl: 'https://avatars.example.com/u/0?v=4' }]
|
||||
})
|
||||
getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
|
||||
getPRCommentsMock.mockResolvedValue([])
|
||||
getPRChecksMock.mockResolvedValue([])
|
||||
const avatars: Record<string, string> = {
|
||||
seah: 'https://avatars.example.com/u/1?v=4',
|
||||
ludi: 'https://avatars.example.com/u/2?v=4',
|
||||
inho: 'https://avatars.example.com/u/3?v=4'
|
||||
}
|
||||
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
||||
const target = args.at(-1)
|
||||
if (target === 'repos/acme/widgets/pulls/1102') {
|
||||
return {
|
||||
stdout: JSON.stringify({ body: 'PR body', head: { sha: 'h' }, base: { sha: 'b' } })
|
||||
}
|
||||
}
|
||||
if (target === 'repos/acme/widgets/pulls/1102/files?per_page=100') {
|
||||
return { stdout: '[]' }
|
||||
}
|
||||
const query = args.find((arg) => arg.startsWith('query=')) ?? ''
|
||||
if (query.includes('user(login:')) {
|
||||
// Return the aliased users the batch asked for, keyed by their login.
|
||||
const data: Record<string, { login: string; name: null; avatarUrl: string }> = {}
|
||||
let index = 0
|
||||
for (const login of Object.keys(avatars)) {
|
||||
if (query.includes(`user(login: "${login}")`)) {
|
||||
data[`u${index}`] = { login, name: null, avatarUrl: avatars[login] }
|
||||
index += 1
|
||||
}
|
||||
}
|
||||
return { stdout: JSON.stringify({ data }) }
|
||||
}
|
||||
return { stdout: JSON.stringify({ data: {} }) }
|
||||
})
|
||||
|
||||
const details = await getWorkItemDetails('/repo-root', 1102, 'pr')
|
||||
|
||||
expect(details?.item.authorAvatarUrl).toBe(avatars.seah)
|
||||
expect(details?.item.reviewRequests?.[0]?.avatarUrl).toBe(avatars.ludi)
|
||||
expect(details?.item.latestReviews?.[0]?.avatarUrl).toBe(avatars.inho)
|
||||
// The default u/0 placeholder is overridden by the resolved avatar.
|
||||
expect(details?.item.assignees?.[0]?.avatarUrl).toBe(avatars.seah)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -85,7 +85,7 @@ const ISSUE_DETAILS_QUERY = `query($owner: String!, $repo: String!, $number: Int
|
||||
repository(owner: $owner, name: $repo) {
|
||||
issue(number: $number) {
|
||||
body
|
||||
assignees(first: 50) { nodes { login } }
|
||||
assignees(first: 50) { nodes { login avatarUrl(size: 48) ... on User { name } } }
|
||||
participants(first: 100) {
|
||||
nodes { login avatarUrl(size: 48) ... on User { name } }
|
||||
}
|
||||
@@ -111,7 +111,7 @@ type GraphQLIssueDetailsResponse = {
|
||||
repository?: {
|
||||
issue?: {
|
||||
body?: string | null
|
||||
assignees?: { nodes?: { login?: string }[] }
|
||||
assignees?: { nodes?: { login?: string; avatarUrl?: string; name?: string | null }[] }
|
||||
participants?: { nodes?: GitHubAssignableUser[] }
|
||||
comments?: {
|
||||
nodes?: {
|
||||
@@ -323,6 +323,12 @@ async function getIssueTimelineItems(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch an issue's body, comments, assignees, participants, and timeline in a
|
||||
* single GraphQL round-trip. Returns null on any partial error so the caller
|
||||
* falls back to the strict REST path. Assignee/participant avatars are resolved
|
||||
* here so GitHub Enterprise users don't render blank.
|
||||
*/
|
||||
async function getIssueDetailsViaGraphQL(
|
||||
repoPath: string,
|
||||
issueNumber: number,
|
||||
@@ -332,6 +338,10 @@ async function getIssueDetailsViaGraphQL(
|
||||
body: string
|
||||
comments: PRComment[]
|
||||
assignees: string[]
|
||||
// Avatar-bearing assignees, kept separate from the `assignees` login list so
|
||||
// callers that only need logins are unaffected; used to enrich assignee
|
||||
// avatars that `gh` leaves blank (notably GitHub Enterprise users).
|
||||
assigneeUsers: GitHubAssignableUser[]
|
||||
participants: GitHubAssignableUser[]
|
||||
timelineItems: GitHubIssueTimelineItem[]
|
||||
} | null> {
|
||||
@@ -386,9 +396,16 @@ async function getIssueDetailsViaGraphQL(
|
||||
url: c.url ?? '',
|
||||
isBot: c.author?.__typename === 'Bot'
|
||||
}))
|
||||
const assignees = (issue.assignees?.nodes ?? [])
|
||||
.map((a) => a.login)
|
||||
.filter((login): login is string => Boolean(login))
|
||||
const assigneeUsers: GitHubAssignableUser[] = (issue.assignees?.nodes ?? [])
|
||||
.filter((a): a is { login: string; avatarUrl?: string; name?: string | null } =>
|
||||
Boolean(a.login)
|
||||
)
|
||||
.map((a) => ({
|
||||
login: a.login,
|
||||
name: a.name ?? null,
|
||||
avatarUrl: a.avatarUrl ?? ''
|
||||
}))
|
||||
const assignees = assigneeUsers.map((a) => a.login)
|
||||
const participants: GitHubAssignableUser[] = (issue.participants?.nodes ?? [])
|
||||
.filter((u) => Boolean(u.login))
|
||||
.map((u) => ({
|
||||
@@ -401,6 +418,7 @@ async function getIssueDetailsViaGraphQL(
|
||||
body: issue.body ?? '',
|
||||
comments,
|
||||
assignees,
|
||||
assigneeUsers,
|
||||
participants,
|
||||
timelineItems
|
||||
}
|
||||
@@ -877,6 +895,11 @@ async function getGitHubUsersByLogin(
|
||||
return []
|
||||
}
|
||||
if (rateLimitGuard('graphql').blocked) {
|
||||
// Why: surface the skip. Callers degrade silently to login-based avatar URLs
|
||||
// (blank on GHE), so without this log "why are avatars missing" is untraceable.
|
||||
console.warn(
|
||||
`getGitHubUsersByLogin skipped: GraphQL rate-limit budget exhausted (${uniqueLogins.length} logins unresolved)`
|
||||
)
|
||||
return []
|
||||
}
|
||||
const fields = uniqueLogins
|
||||
@@ -921,19 +944,100 @@ async function getGitHubUsersByLogin(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Stamp GraphQL-resolved avatars onto a work item's author, reviewers, review
|
||||
* requests, latest reviews, and assignees.
|
||||
*
|
||||
* The work item's users come from `gh pr view`, which omits avatar_url. On
|
||||
* GitHub Enterprise the login-based `github.com/{login}.png` URL 404s, so those
|
||||
* avatars render blank while comment avatars (fetched via GraphQL) work.
|
||||
* `knownUsers` (participants + the mention-author batch, which already includes
|
||||
* the item's reviewers/assignees) carries the resolved avatars, so we apply
|
||||
* them without any extra round-trip. See #8784.
|
||||
*/
|
||||
function enrichItemDisplayAvatars(
|
||||
item: Omit<GitHubWorkItem, 'repoId'>,
|
||||
knownUsers: GitHubAssignableUser[]
|
||||
): Omit<GitHubWorkItem, 'repoId'> {
|
||||
const avatarByLogin = new Map<string, string>()
|
||||
for (const user of knownUsers) {
|
||||
if (user.login && user.avatarUrl) {
|
||||
avatarByLogin.set(user.login.toLowerCase(), user.avatarUrl)
|
||||
}
|
||||
}
|
||||
if (avatarByLogin.size === 0) {
|
||||
return item
|
||||
}
|
||||
// Why: prefer the GraphQL-resolved avatar over whatever `gh` returned. `gh pr
|
||||
// view` often yields an empty avatar or the default `u/0` placeholder for
|
||||
// enterprise users; the resolved avatar is the authoritative one for that
|
||||
// login. Fall back to the original only when the lookup found nothing.
|
||||
const avatarFor = (login: string): string | undefined => avatarByLogin.get(login.toLowerCase())
|
||||
const resolvedAvatar = (login: string, existing?: string | null): string | undefined =>
|
||||
avatarFor(login) || existing || undefined
|
||||
// Callers coalesce a missing result to the field's "no avatar" sentinel (''
|
||||
// for GitHubAssignableUser, null for GitHubPRReviewSummary); GitHubUserAvatar
|
||||
// treats both as falsy and falls back to the login URL then initials.
|
||||
const authorAvatarUrl = (item.author ? avatarFor(item.author) : undefined) || item.authorAvatarUrl
|
||||
return {
|
||||
...item,
|
||||
...(authorAvatarUrl ? { authorAvatarUrl } : {}),
|
||||
...(item.reviewRequests
|
||||
? {
|
||||
reviewRequests: item.reviewRequests.map((user) => ({
|
||||
...user,
|
||||
avatarUrl: resolvedAvatar(user.login, user.avatarUrl) ?? ''
|
||||
}))
|
||||
}
|
||||
: {}),
|
||||
...(item.latestReviews
|
||||
? {
|
||||
latestReviews: item.latestReviews.map((review) => ({
|
||||
...review,
|
||||
avatarUrl: resolvedAvatar(review.login, review.avatarUrl) ?? null
|
||||
}))
|
||||
}
|
||||
: {}),
|
||||
...(item.assignees
|
||||
? {
|
||||
assignees: item.assignees.map((user) => ({
|
||||
...user,
|
||||
avatarUrl: resolvedAvatar(user.login, user.avatarUrl) ?? ''
|
||||
}))
|
||||
}
|
||||
: {})
|
||||
}
|
||||
}
|
||||
|
||||
async function getMentionParticipants(
|
||||
repoPath: string,
|
||||
item: Pick<GitHubWorkItem, 'author' | 'number' | 'type'>,
|
||||
item: Pick<
|
||||
GitHubWorkItem,
|
||||
'author' | 'number' | 'type' | 'reviewRequests' | 'latestReviews' | 'assignees'
|
||||
>,
|
||||
comments: PRComment[],
|
||||
participants: GitHubAssignableUser[],
|
||||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<GitHubAssignableUser[]> {
|
||||
const visibleLogins = [item.author ?? '', ...comments.map((comment) => comment.author)]
|
||||
// Why: one aliased GraphQL query returns login/name/avatarUrl for every
|
||||
// mentioned author in a single round-trip. The previous REST fan-out
|
||||
// (/users/<login>) returned the same fields but cost one rate-limit point
|
||||
// per user.
|
||||
// Why: resolve mention authors AND the item's reviewers/assignees in ONE
|
||||
// aliased GraphQL round-trip. Folding the display users in here means the
|
||||
// caller can reuse this result to stamp avatars (see enrichItemDisplayAvatars)
|
||||
// without a second, separately rate-limited lookup — which is what made GHE
|
||||
// reviewer/assignee avatars resolve intermittently. See #8784.
|
||||
// Why: order the always-visible display users (author, reviewers, assignees)
|
||||
// before comment authors so the 40-login cap in getGitHubUsersByLogin never
|
||||
// drops a reviewer/assignee avatar on a PR with many commenters. On a PR with
|
||||
// 40+ distinct participants the trailing comment authors are dropped from this
|
||||
// batch and lose only their @mention-suggestion avatar; rendered comment
|
||||
// avatars are unaffected (they carry their own avatarUrl from the comment).
|
||||
const visibleLogins = [
|
||||
item.author ?? '',
|
||||
...(item.reviewRequests ?? []).map((user) => user.login),
|
||||
...(item.latestReviews ?? []).map((review) => review.login),
|
||||
...(item.assignees ?? []).map((user) => user.login),
|
||||
...comments.map((comment) => comment.author)
|
||||
]
|
||||
const graphQlUsers = await getGitHubUsersByLogin(
|
||||
repoPath,
|
||||
visibleLogins,
|
||||
@@ -1007,7 +1111,13 @@ export async function getWorkItemDetails(
|
||||
)
|
||||
if (collapsed) {
|
||||
return {
|
||||
item,
|
||||
// Include assigneeUsers: a non-participating assignee is absent from
|
||||
// the participants connection, so without them item.assignees keeps a
|
||||
// blank/placeholder avatar (common for GitHub Enterprise users).
|
||||
item: enrichItemDisplayAvatars(item, [
|
||||
...collapsed.participants,
|
||||
...collapsed.assigneeUsers
|
||||
]),
|
||||
body: collapsed.body,
|
||||
comments: collapsed.comments,
|
||||
assignees: collapsed.assignees,
|
||||
@@ -1030,7 +1140,7 @@ export async function getWorkItemDetails(
|
||||
localGitOptions
|
||||
)
|
||||
return {
|
||||
item,
|
||||
item: enrichItemDisplayAvatars(item, mentionParticipants),
|
||||
body,
|
||||
comments,
|
||||
assignees,
|
||||
@@ -1064,7 +1174,7 @@ export async function getWorkItemDetails(
|
||||
])
|
||||
|
||||
return {
|
||||
item,
|
||||
item: enrichItemDisplayAvatars(item, mentionParticipants),
|
||||
body,
|
||||
comments,
|
||||
headSha: shas?.headSha,
|
||||
|
||||
@@ -181,7 +181,7 @@ import {
|
||||
filterGitHubPRReviewerCandidates,
|
||||
getGitHubPRReviewerQueryState
|
||||
} from '@/components/github/github-pr-reviewer-candidate-filter'
|
||||
import { githubAvatarUrl } from '@/components/github/github-issue-comment-helpers'
|
||||
import { GitHubUserAvatar } from '@/components/github/github-user-avatar'
|
||||
import { presentGitHubPRMergeState } from '@/components/github-pr-merge-state'
|
||||
import {
|
||||
GITHUB_PR_MERGE_METHOD_LABELS,
|
||||
@@ -421,16 +421,7 @@ function ReviewerAvatar({
|
||||
login: string
|
||||
avatarUrl: string
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<img
|
||||
src={avatarUrl || githubAvatarUrl(login)}
|
||||
alt=""
|
||||
loading="lazy"
|
||||
decoding="async"
|
||||
title={login}
|
||||
className="size-6 shrink-0 rounded-full border border-border/50 bg-muted object-cover"
|
||||
/>
|
||||
)
|
||||
return <GitHubUserAvatar login={login} avatarUrl={avatarUrl} title={login} className="size-6" />
|
||||
}
|
||||
|
||||
function mergeReviewerSuggestions(
|
||||
|
||||
@@ -157,7 +157,7 @@ import {
|
||||
filterGitHubPRReviewerCandidates,
|
||||
getGitHubPRReviewerQueryState
|
||||
} from '@/components/github/github-pr-reviewer-candidate-filter'
|
||||
import { githubAvatarUrl } from '@/components/github/github-issue-comment-helpers'
|
||||
import { GitHubUserAvatar } from '@/components/github/github-user-avatar'
|
||||
import { filterGitHubMentionOptions } from '@/components/github/github-mention-option-filter'
|
||||
import {
|
||||
getCommentBodySubmitState,
|
||||
@@ -468,16 +468,7 @@ function ReviewerAvatar({
|
||||
login: string
|
||||
avatarUrl: string
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<img
|
||||
src={avatarUrl || githubAvatarUrl(login)}
|
||||
alt=""
|
||||
loading="lazy"
|
||||
decoding="async"
|
||||
title={login}
|
||||
className="size-6 shrink-0 rounded-full border border-border/50 bg-muted object-cover"
|
||||
/>
|
||||
)
|
||||
return <GitHubUserAvatar login={login} avatarUrl={avatarUrl} title={login} className="size-6" />
|
||||
}
|
||||
|
||||
function mergeReviewerSuggestions(
|
||||
@@ -7148,12 +7139,10 @@ export default function PullRequestPage({
|
||||
</span>
|
||||
<span className="flex min-w-0 items-center gap-1.5">
|
||||
{workItem.author ? (
|
||||
<img
|
||||
src={githubAvatarUrl(workItem.author)}
|
||||
alt=""
|
||||
loading="lazy"
|
||||
decoding="async"
|
||||
className="size-5 shrink-0 rounded-full border border-border/50 bg-muted object-cover"
|
||||
<GitHubUserAvatar
|
||||
login={workItem.author}
|
||||
avatarUrl={displayWorkItem?.authorAvatarUrl ?? workItem.authorAvatarUrl}
|
||||
className="size-5"
|
||||
/>
|
||||
) : null}
|
||||
<span className="font-semibold text-foreground">
|
||||
|
||||
@@ -133,6 +133,7 @@ import {
|
||||
} from '../../../shared/linear-links'
|
||||
import PRFilterDropdowns, { type PRFilterChange } from '@/components/github/PRFilterDropdowns'
|
||||
import { GitHubMarkdownComposer } from '@/components/github/GitHubMarkdownComposer'
|
||||
import { GitHubUserAvatar } from '@/components/github/github-user-avatar'
|
||||
import { buildGitHubRepoUrl, parseGitHubIssueOrPRLink } from '@/lib/github-links'
|
||||
import {
|
||||
findGithubWorkItemWorkspaceAttachment,
|
||||
@@ -1520,17 +1521,16 @@ function ReviewChipAvatar({
|
||||
reviewer: GitHubPRPrimaryReviewer | null
|
||||
}): React.JSX.Element {
|
||||
if (reviewer?.login) {
|
||||
// Why: `gh pr list --json reviewRequests` can return only logins; GitHub's
|
||||
// public avatar endpoint keeps the list visual aligned with assignee cells.
|
||||
const avatarUrl = reviewer.avatarUrl || `https://github.com/${reviewer.login}.png?size=40`
|
||||
// Why: `gh pr list --json reviewRequests` can return only logins. Prefer the
|
||||
// API avatar_url so GHE renders; GitHubUserAvatar falls back to the login URL
|
||||
// and then an initials placeholder when no avatar loads. See #8784.
|
||||
return (
|
||||
<img
|
||||
src={avatarUrl}
|
||||
alt=""
|
||||
loading="lazy"
|
||||
decoding="async"
|
||||
<GitHubUserAvatar
|
||||
login={reviewer.login}
|
||||
name={reviewer.name}
|
||||
avatarUrl={reviewer.avatarUrl}
|
||||
title={reviewer.name ? `${reviewer.name} (${reviewer.login})` : reviewer.login}
|
||||
className="size-5 shrink-0 rounded-full border border-border/50 bg-muted object-cover"
|
||||
className="size-5"
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { act } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { GitHubUserAvatar, resolveGitHubUserAvatarSrc } from './github-user-avatar'
|
||||
|
||||
function render(node: React.JSX.Element): { root: Root; container: HTMLDivElement } {
|
||||
const container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
const root = createRoot(container)
|
||||
act(() => {
|
||||
root.render(node)
|
||||
})
|
||||
return { root, container }
|
||||
}
|
||||
|
||||
describe('GitHubUserAvatar', () => {
|
||||
let root: Root | null = null
|
||||
let container: HTMLDivElement | null = null
|
||||
|
||||
afterEach(() => {
|
||||
act(() => root?.unmount())
|
||||
container?.remove()
|
||||
root = null
|
||||
container = null
|
||||
})
|
||||
|
||||
it('renders the API avatar_url when provided (works for github.com and GHE)', () => {
|
||||
const url = 'https://avatars.example.com/u/42?u=hash&v=4'
|
||||
;({ root, container } = render(<GitHubUserAvatar login="enterprise-user" avatarUrl={url} />))
|
||||
const img = container.querySelector('img')
|
||||
expect(img?.getAttribute('src')).toBe(url)
|
||||
})
|
||||
|
||||
it('falls back to the login URL when no avatar_url is available', () => {
|
||||
;({ root, container } = render(<GitHubUserAvatar login="octocat" />))
|
||||
const img = container.querySelector('img')
|
||||
expect(img?.getAttribute('src')).toBe('https://github.com/octocat.png?size=64')
|
||||
})
|
||||
|
||||
it('degrades to an initials placeholder when the image fails to load (GHE 404)', () => {
|
||||
;({ root, container } = render(
|
||||
<GitHubUserAvatar login="enterprise-user" name="Ada Lovelace" />
|
||||
))
|
||||
const img = container.querySelector('img')
|
||||
expect(img).not.toBeNull()
|
||||
act(() => {
|
||||
img?.dispatchEvent(new Event('error'))
|
||||
})
|
||||
expect(container.querySelector('img')).toBeNull()
|
||||
expect(container.textContent).toBe('AL')
|
||||
})
|
||||
|
||||
it('skips leading non-BMP chars so initials never render a broken surrogate', () => {
|
||||
;({ root, container } = render(<GitHubUserAvatar login="rocketdev" name="🚀 Developer" />))
|
||||
act(() => {
|
||||
container?.querySelector('img')?.dispatchEvent(new Event('error'))
|
||||
})
|
||||
// Emoji word is skipped entirely rather than split into a broken half.
|
||||
expect(container.textContent).toBe('D')
|
||||
})
|
||||
|
||||
it('derives initials from the login when no name is given', () => {
|
||||
;({ root, container } = render(<GitHubUserAvatar login="octocat" />))
|
||||
act(() => {
|
||||
container?.querySelector('img')?.dispatchEvent(new Event('error'))
|
||||
})
|
||||
expect(container.textContent).toBe('OC')
|
||||
})
|
||||
|
||||
it('retries when avatarUrl changes after an earlier src failed (late-arriving avatar)', () => {
|
||||
// Why: PR detail enrichment supplies avatar_url after first paint. A row that
|
||||
// 404s on the early login-based URL must recover once the real URL lands.
|
||||
;({ root, container } = render(<GitHubUserAvatar login="enterprise-user" />))
|
||||
// First src is the login fallback; simulate the GHE 404.
|
||||
act(() => {
|
||||
container?.querySelector('img')?.dispatchEvent(new Event('error'))
|
||||
})
|
||||
expect(container.querySelector('img')).toBeNull()
|
||||
// Enriched avatar_url arrives → component must render the new image, not the
|
||||
// latched placeholder.
|
||||
const url = 'https://avatars.example.com/u/7?v=4'
|
||||
act(() => {
|
||||
root?.render(<GitHubUserAvatar login="enterprise-user" avatarUrl={url} />)
|
||||
})
|
||||
expect(container.querySelector('img')?.getAttribute('src')).toBe(url)
|
||||
})
|
||||
|
||||
it('shows initials immediately when login and avatarUrl are both empty', () => {
|
||||
;({ root, container } = render(<GitHubUserAvatar login="" name="Ada" />))
|
||||
expect(container.querySelector('img')).toBeNull()
|
||||
expect(container.textContent).toBe('AD')
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveGitHubUserAvatarSrc', () => {
|
||||
it('prefers trimmed API avatar over login.png', () => {
|
||||
expect(resolveGitHubUserAvatarSrc('u', ' https://avatars.example.com/u/1?v=4 ')).toBe(
|
||||
'https://avatars.example.com/u/1?v=4'
|
||||
)
|
||||
})
|
||||
|
||||
it('skips empty login so no github.com/.png request is built', () => {
|
||||
expect(resolveGitHubUserAvatarSrc('', null)).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,104 @@
|
||||
import { useState } from 'react'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { githubAvatarUrl } from '@/components/github/github-issue-comment-helpers'
|
||||
|
||||
/**
|
||||
* Build a 1-2 character initials placeholder from a display name or login,
|
||||
* used when no avatar image is available or the image fails to load.
|
||||
*/
|
||||
function initialsFor(login: string, name?: string | null): string {
|
||||
const source = (name?.trim() || login).trim()
|
||||
if (!source) {
|
||||
return '?'
|
||||
}
|
||||
// Iterate by code point (not UTF-16 unit) and keep only letters/digits, so a
|
||||
// display name that leads with an emoji or other non-BMP char yields clean
|
||||
// initials instead of a broken surrogate half. Words that are all symbols are
|
||||
// skipped; an all-symbol name falls back to '?'.
|
||||
const alnum = (word: string): string[] => [...word].filter((ch) => /[\p{L}\p{N}]/u.test(ch))
|
||||
const parts = source.split(/\s+/).filter(Boolean)
|
||||
const letters =
|
||||
parts.length >= 2
|
||||
? parts
|
||||
.map((word) => alnum(word)[0])
|
||||
.filter(Boolean)
|
||||
.slice(0, 2)
|
||||
.join('')
|
||||
: alnum(source).slice(0, 2).join('')
|
||||
return letters.toUpperCase() || '?'
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer the API `avatar_url`, then the public login.png URL. Empty/whitespace
|
||||
* API values and empty logins never produce a bogus request. See #8784.
|
||||
*/
|
||||
export function resolveGitHubUserAvatarSrc(
|
||||
login: string,
|
||||
avatarUrl?: string | null
|
||||
): string | null {
|
||||
const fromApi = avatarUrl?.trim()
|
||||
if (fromApi) {
|
||||
return fromApi
|
||||
}
|
||||
const trimmedLogin = login.trim()
|
||||
if (!trimmedLogin) {
|
||||
return null
|
||||
}
|
||||
return githubAvatarUrl(trimmedLogin)
|
||||
}
|
||||
|
||||
/**
|
||||
* Avatar for a GitHub user that renders correctly on github.com and GitHub
|
||||
* Enterprise. GHE logins don't exist on github.com, so the login-based
|
||||
* `github.com/{login}.png` URL 404s. We prefer the API-provided `avatarUrl`
|
||||
* and, when it (or the login fallback) fails to load, degrade to an initials
|
||||
* placeholder instead of a broken image. See #8784.
|
||||
*/
|
||||
export function GitHubUserAvatar({
|
||||
login,
|
||||
name,
|
||||
avatarUrl,
|
||||
title,
|
||||
className
|
||||
}: {
|
||||
login: string
|
||||
name?: string | null
|
||||
avatarUrl?: string | null
|
||||
title?: string
|
||||
className?: string
|
||||
}): React.JSX.Element {
|
||||
// Why: track the specific src that failed rather than a boolean. Avatar data
|
||||
// arrives after the first paint (PR detail enrichment), so a row that 404s on
|
||||
// the early login-based URL must retry once the real avatar_url lands — a
|
||||
// latched boolean would keep showing the placeholder forever. See #8784.
|
||||
const [failedSrc, setFailedSrc] = useState<string | null>(null)
|
||||
const src = resolveGitHubUserAvatarSrc(login, avatarUrl)
|
||||
if (src && failedSrc !== src) {
|
||||
return (
|
||||
<img
|
||||
src={src}
|
||||
alt=""
|
||||
loading="lazy"
|
||||
decoding="async"
|
||||
title={title}
|
||||
onError={() => setFailedSrc(src)}
|
||||
className={cn(
|
||||
'shrink-0 rounded-full border border-border/50 bg-muted object-cover',
|
||||
className
|
||||
)}
|
||||
/>
|
||||
)
|
||||
}
|
||||
return (
|
||||
<span
|
||||
title={title}
|
||||
aria-hidden
|
||||
className={cn(
|
||||
'inline-flex shrink-0 items-center justify-center rounded-full border border-border/50 bg-muted text-[10px] font-semibold text-muted-foreground',
|
||||
className
|
||||
)}
|
||||
>
|
||||
{initialsFor(login, name)}
|
||||
</span>
|
||||
)
|
||||
}
|
||||
@@ -0,0 +1,55 @@
|
||||
/**
|
||||
* Issue #8784 — GHE PR avatars must prefer API avatar_url over github.com/{login}.png.
|
||||
*
|
||||
* Regression guard (was a repro that documented the broken path). After the fix:
|
||||
* - resolveGitHubUserAvatarSrc prefers API URLs
|
||||
* - PullRequestPage author uses authorAvatarUrl via GitHubUserAvatar
|
||||
* - TaskPage ReviewChipAvatar no longer hardcodes github.com login.png
|
||||
*
|
||||
* Re-run:
|
||||
* pnpm exec vitest run --config config/vitest.config.ts \
|
||||
* src/renderer/src/components/github/repro-8784-ghe-avatar-fallback.test.ts
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { githubAvatarUrl } from './github-issue-comment-helpers'
|
||||
import { resolveGitHubUserAvatarSrc } from './github-user-avatar'
|
||||
|
||||
describe('issue #8784 GHE avatar fallback (regression)', () => {
|
||||
it('prefers API avatar_url over login.png (GHE healthy path)', () => {
|
||||
const api = 'https://ghe.example.com/avatars/u/42?v=4'
|
||||
expect(resolveGitHubUserAvatarSrc('enterprise-only-user', api)).toBe(api)
|
||||
})
|
||||
|
||||
it('trims whitespace-only avatarUrl and falls back to login.png for github.com', () => {
|
||||
expect(resolveGitHubUserAvatarSrc('octocat', ' ')).toBe(
|
||||
'https://github.com/octocat.png?size=64'
|
||||
)
|
||||
expect(resolveGitHubUserAvatarSrc('octocat', null)).toBe(githubAvatarUrl('octocat'))
|
||||
})
|
||||
|
||||
it('returns null when neither avatarUrl nor login is usable (no bogus request)', () => {
|
||||
expect(resolveGitHubUserAvatarSrc('', '')).toBeNull()
|
||||
expect(resolveGitHubUserAvatarSrc(' ', null)).toBeNull()
|
||||
})
|
||||
|
||||
it('login-only fallback still hardcodes public github.com png (github.com path)', () => {
|
||||
// Why: github.com users without avatar_url still use this; GHE relies on
|
||||
// enrichment + image onError → initials when this 404s.
|
||||
expect(githubAvatarUrl('corp-user')).toBe('https://github.com/corp-user.png?size=64')
|
||||
})
|
||||
|
||||
it('source routes PR author/reviewer avatars through GitHubUserAvatar + authorAvatarUrl', () => {
|
||||
const prPage = readFileSync(join(__dirname, '../PullRequestPage.tsx'), 'utf8')
|
||||
expect(prPage).toMatch(/GitHubUserAvatar/)
|
||||
expect(prPage).toMatch(/authorAvatarUrl/)
|
||||
// Why: author chip must not ignore API avatar_url and only pass login.
|
||||
expect(prPage).not.toMatch(/githubAvatarUrl\(workItem\.author\)/)
|
||||
|
||||
const taskPage = readFileSync(join(__dirname, '../TaskPage.tsx'), 'utf8')
|
||||
expect(taskPage).toMatch(/GitHubUserAvatar/)
|
||||
// Why: list chip must not hardcode github.com/{login}.png.
|
||||
expect(taskPage).not.toMatch(/github\.com\/\$\{reviewer\.login\}\.png/)
|
||||
})
|
||||
})
|
||||
@@ -106,6 +106,8 @@ export function ShortcutCommandBlock({
|
||||
/>
|
||||
)
|
||||
|
||||
// Why: ShortcutsPane already filters rows against the global query;
|
||||
// forceVisible skips the re-match here that would hide rows it kept.
|
||||
return (
|
||||
<SearchableSetting
|
||||
title={item.title}
|
||||
@@ -115,6 +117,7 @@ export function ShortcutCommandBlock({
|
||||
{ value0: groupTitle }
|
||||
)}
|
||||
keywords={[...item.searchKeywords]}
|
||||
forceVisible
|
||||
className="group/shortcut flex max-w-none flex-col"
|
||||
>
|
||||
<div className="flex min-h-9 items-center gap-3 rounded-md px-2 py-1 transition-colors hover:bg-accent/40 focus-within:bg-accent/40">
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
|
||||
import type { KeybindingActionId } from '../../../../shared/keybindings'
|
||||
import {
|
||||
SHORTCUT_LOCAL_SEARCH_QUERY_MAX_BYTES,
|
||||
buildShortcutGlobalSearchMatcher,
|
||||
isShortcutLocalSearchQueryTooLarge,
|
||||
matchesShortcutLocalSearch,
|
||||
normalizeShortcutLocalSearchQuery,
|
||||
@@ -52,6 +53,40 @@ describe('ShortcutFilterRail search helpers', () => {
|
||||
expect(normalizeShortcutLocalSearchQuery(query)).toBeNull()
|
||||
})
|
||||
|
||||
it('narrows rows when the global settings query matches specific rows', () => {
|
||||
const settingsRow = createShortcutRow()
|
||||
const worktreeRow: ShortcutRowModel = {
|
||||
...createShortcutRow(),
|
||||
item: {
|
||||
...createShortcutRow().item,
|
||||
id: 'worktree.create' as KeybindingActionId,
|
||||
title: 'Create worktree',
|
||||
searchKeywords: ['worktree', 'create']
|
||||
}
|
||||
}
|
||||
|
||||
const matcher = buildShortcutGlobalSearchMatcher([settingsRow, worktreeRow], 'worktree')
|
||||
|
||||
expect(matcher(settingsRow)).toBe(false)
|
||||
expect(matcher(worktreeRow)).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps every row visible on a pane-title-only global query', () => {
|
||||
const rows = [createShortcutRow()]
|
||||
|
||||
// Mirrors sidebar search selecting the pane with a query that matches only
|
||||
// pane-level metadata (e.g. a localized pane title) and no row metadata.
|
||||
const matcher = buildShortcutGlobalSearchMatcher(rows, '단축키')
|
||||
|
||||
expect(matcher(rows[0])).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps every row visible when the global query is empty', () => {
|
||||
const rows = [createShortcutRow()]
|
||||
|
||||
expect(buildShortcutGlobalSearchMatcher(rows, '')(rows[0])).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects oversized pasted shortcut searches before reading row metadata', () => {
|
||||
const oversizedQuery = 'secret-shortcut-search'.repeat(SHORTCUT_LOCAL_SEARCH_QUERY_MAX_BYTES)
|
||||
const row = {
|
||||
|
||||
@@ -6,7 +6,7 @@ import { cn } from '../../lib/utils'
|
||||
import { Button } from '../ui/button'
|
||||
import { Input } from '../ui/input'
|
||||
import type { ShortcutTerminalStatus } from './shortcut-terminal-status'
|
||||
import type { SettingsSearchEntry } from './settings-search'
|
||||
import { matchesSettingsSearch, type SettingsSearchEntry } from './settings-search'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
export type ShortcutFilter = 'all' | 'modified' | 'unassigned' | 'conflicts'
|
||||
@@ -60,6 +60,17 @@ export function getShortcutSearchEntry(row: ShortcutRowModel): SettingsSearchEnt
|
||||
}
|
||||
}
|
||||
|
||||
// Why: a sidebar query can select this pane by title alone while matching zero
|
||||
// rows; that would blank the list, so zero-row queries keep every row visible.
|
||||
export function buildShortcutGlobalSearchMatcher(
|
||||
rows: readonly ShortcutRowModel[],
|
||||
searchQuery: string
|
||||
): (row: ShortcutRowModel) => boolean {
|
||||
const rowMatches = (row: ShortcutRowModel): boolean =>
|
||||
matchesSettingsSearch(searchQuery, getShortcutSearchEntry(row))
|
||||
return rows.some(rowMatches) ? rowMatches : () => true
|
||||
}
|
||||
|
||||
export function matchesShortcutFilter(row: ShortcutRowModel, filter: ShortcutFilter): boolean {
|
||||
switch (filter) {
|
||||
case 'modified':
|
||||
|
||||
@@ -25,7 +25,7 @@ import {
|
||||
sameBindings
|
||||
} from './keybinding-override-edits'
|
||||
import {
|
||||
getShortcutSearchEntry,
|
||||
buildShortcutGlobalSearchMatcher,
|
||||
matchesShortcutFilter,
|
||||
matchesShortcutLocalSearch,
|
||||
normalizeShortcutLocalSearchQuery,
|
||||
@@ -138,9 +138,10 @@ export function ShortcutsPane(): React.JSX.Element {
|
||||
)
|
||||
const shortcutSearchQuery = normalizeShortcutLocalSearchQuery(shortcutQuery)
|
||||
const shortcutRows = shortcutGroups.flatMap((group) => group.rows)
|
||||
const matchesShortcutGlobalSearch = buildShortcutGlobalSearchMatcher(shortcutRows, searchQuery)
|
||||
const matchesShortcutSearch = (row: ShortcutRowsByGroup['rows'][number]): boolean =>
|
||||
shortcutSearchQuery !== null &&
|
||||
matchesSettingsSearch(searchQuery, getShortcutSearchEntry(row)) &&
|
||||
matchesShortcutGlobalSearch(row) &&
|
||||
matchesShortcutLocalSearch(row, shortcutSearchQuery, platform)
|
||||
const baseVisibleRows = shortcutRows.filter((row) => matchesShortcutSearch(row))
|
||||
const filterCounts: Record<ShortcutFilter, number> = {
|
||||
|
||||
@@ -5805,9 +5805,10 @@ const WorktreeList = React.memo(function WorktreeList({
|
||||
groupBy,
|
||||
repos: visibleReposForRows,
|
||||
worktreesByRepo,
|
||||
visibleWorktrees,
|
||||
filterRepoIds
|
||||
})
|
||||
}, [filterRepoIds, groupBy, visibleReposForRows, worktreesByRepo])
|
||||
}, [filterRepoIds, groupBy, visibleReposForRows, visibleWorktrees, worktreesByRepo])
|
||||
const allRepoIds = useMemo(() => repos.map((r) => r.id), [repos])
|
||||
|
||||
// Why: buildRows only needs which creates exist and their repo. Subscribe on a
|
||||
|
||||
@@ -38,6 +38,7 @@ describe('getEmptyProjectPlaceholderRepoIds', () => {
|
||||
groupBy: 'repo',
|
||||
repos: [repo],
|
||||
worktreesByRepo: { [repo.id]: [] },
|
||||
visibleWorktrees: [],
|
||||
filterRepoIds: []
|
||||
})
|
||||
)
|
||||
@@ -51,6 +52,7 @@ describe('getEmptyProjectPlaceholderRepoIds', () => {
|
||||
groupBy: 'repo',
|
||||
repos: [repo],
|
||||
worktreesByRepo: {},
|
||||
visibleWorktrees: [],
|
||||
filterRepoIds: []
|
||||
})
|
||||
)
|
||||
@@ -67,6 +69,7 @@ describe('getEmptyProjectPlaceholderRepoIds', () => {
|
||||
groupBy: 'repo',
|
||||
repos: [selectedRepo, hiddenRepo],
|
||||
worktreesByRepo: { [selectedRepo.id]: [], [hiddenRepo.id]: [] },
|
||||
visibleWorktrees: [],
|
||||
filterRepoIds: [selectedRepo.id]
|
||||
})
|
||||
)
|
||||
@@ -79,6 +82,7 @@ describe('getEmptyProjectPlaceholderRepoIds', () => {
|
||||
groupBy: 'none',
|
||||
repos: [repo],
|
||||
worktreesByRepo: { [repo.id]: [] },
|
||||
visibleWorktrees: [],
|
||||
filterRepoIds: []
|
||||
}).size
|
||||
).toBe(0)
|
||||
@@ -90,8 +94,109 @@ describe('getEmptyProjectPlaceholderRepoIds', () => {
|
||||
groupBy: 'repo',
|
||||
repos: [repo],
|
||||
worktreesByRepo: { [repo.id]: [worktree] },
|
||||
visibleWorktrees: [],
|
||||
filterRepoIds: []
|
||||
}).size
|
||||
).toBe(0)
|
||||
})
|
||||
|
||||
it('keeps grouped repos visible when workspace filters hide all of their rows', () => {
|
||||
const groupedRepo: Repo = { ...repo, projectGroupId: 'group-1' }
|
||||
const groupedWorktree: Worktree = { ...worktree, repoId: groupedRepo.id }
|
||||
|
||||
expect(
|
||||
Array.from(
|
||||
getEmptyProjectPlaceholderRepoIds({
|
||||
groupBy: 'repo',
|
||||
repos: [groupedRepo],
|
||||
worktreesByRepo: { [groupedRepo.id]: [groupedWorktree] },
|
||||
visibleWorktrees: [],
|
||||
filterRepoIds: []
|
||||
})
|
||||
)
|
||||
).toEqual([groupedRepo.id])
|
||||
})
|
||||
|
||||
it('does not create a grouped repo placeholder when one of its workspaces is visible', () => {
|
||||
const groupedRepo: Repo = { ...repo, projectGroupId: 'group-1' }
|
||||
const groupedWorktree: Worktree = { ...worktree, repoId: groupedRepo.id }
|
||||
|
||||
expect(
|
||||
getEmptyProjectPlaceholderRepoIds({
|
||||
groupBy: 'repo',
|
||||
repos: [groupedRepo],
|
||||
worktreesByRepo: { [groupedRepo.id]: [groupedWorktree] },
|
||||
visibleWorktrees: [groupedWorktree],
|
||||
filterRepoIds: []
|
||||
}).size
|
||||
).toBe(0)
|
||||
})
|
||||
|
||||
it('still respects explicit project filters for sleep-filtered grouped members', () => {
|
||||
const selected: Repo = { ...repo, id: 'repo-selected', projectGroupId: 'group-1' }
|
||||
const filteredOut: Repo = { ...repo, id: 'repo-hidden', projectGroupId: 'group-1' }
|
||||
const selectedWt: Worktree = { ...worktree, id: 'wt-selected', repoId: selected.id }
|
||||
const hiddenWt: Worktree = { ...worktree, id: 'wt-hidden', repoId: filteredOut.id }
|
||||
|
||||
expect(
|
||||
Array.from(
|
||||
getEmptyProjectPlaceholderRepoIds({
|
||||
groupBy: 'repo',
|
||||
repos: [selected, filteredOut],
|
||||
worktreesByRepo: {
|
||||
[selected.id]: [selectedWt],
|
||||
[filteredOut.id]: [hiddenWt]
|
||||
},
|
||||
// Why: simulate Hide sleeping removing every card while the project
|
||||
// filter still intentionally excludes `filteredOut`.
|
||||
visibleWorktrees: [],
|
||||
filterRepoIds: [selected.id]
|
||||
})
|
||||
)
|
||||
).toEqual([selected.id])
|
||||
})
|
||||
|
||||
it('placeholders only the fully-filtered members of a multi-project group', () => {
|
||||
const sleeping: Repo = { ...repo, id: 'repo-sleeping', projectGroupId: 'group-1' }
|
||||
const awake: Repo = { ...repo, id: 'repo-awake', projectGroupId: 'group-1' }
|
||||
const sleepingWt: Worktree = { ...worktree, id: 'wt-sleeping', repoId: sleeping.id }
|
||||
const awakeWt: Worktree = { ...worktree, id: 'wt-awake', repoId: awake.id }
|
||||
|
||||
expect(
|
||||
Array.from(
|
||||
getEmptyProjectPlaceholderRepoIds({
|
||||
groupBy: 'repo',
|
||||
repos: [sleeping, awake],
|
||||
worktreesByRepo: {
|
||||
[sleeping.id]: [sleepingWt],
|
||||
[awake.id]: [awakeWt]
|
||||
},
|
||||
visibleWorktrees: [awakeWt],
|
||||
filterRepoIds: []
|
||||
})
|
||||
)
|
||||
).toEqual([sleeping.id])
|
||||
})
|
||||
|
||||
it('does not placeholder ungrouped neighbors of a filtered grouped member', () => {
|
||||
const grouped: Repo = { ...repo, id: 'repo-grouped', projectGroupId: 'group-1' }
|
||||
const ungrouped: Repo = { ...repo, id: 'repo-ungrouped' }
|
||||
const groupedWt: Worktree = { ...worktree, id: 'wt-grouped', repoId: grouped.id }
|
||||
const ungroupedWt: Worktree = { ...worktree, id: 'wt-ungrouped', repoId: ungrouped.id }
|
||||
|
||||
expect(
|
||||
Array.from(
|
||||
getEmptyProjectPlaceholderRepoIds({
|
||||
groupBy: 'repo',
|
||||
repos: [grouped, ungrouped],
|
||||
worktreesByRepo: {
|
||||
[grouped.id]: [groupedWt],
|
||||
[ungrouped.id]: [ungroupedWt]
|
||||
},
|
||||
visibleWorktrees: [],
|
||||
filterRepoIds: []
|
||||
})
|
||||
)
|
||||
).toEqual([grouped.id])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -5,6 +5,7 @@ export function getEmptyProjectPlaceholderRepoIds(args: {
|
||||
groupBy: WorktreeGroupBy
|
||||
repos: readonly Repo[]
|
||||
worktreesByRepo: Readonly<Record<string, readonly Worktree[] | undefined>>
|
||||
visibleWorktrees: readonly Worktree[]
|
||||
filterRepoIds: readonly string[]
|
||||
}): Set<string> {
|
||||
if (args.groupBy !== 'repo') {
|
||||
@@ -12,12 +13,17 @@ export function getEmptyProjectPlaceholderRepoIds(args: {
|
||||
}
|
||||
|
||||
const filterSet = args.filterRepoIds.length > 0 ? new Set(args.filterRepoIds) : null
|
||||
const visibleRepoIds = new Set(args.visibleWorktrees.map((worktree) => worktree.repoId))
|
||||
const placeholderRepoIds = new Set<string>()
|
||||
for (const repo of args.repos) {
|
||||
if (filterSet && !filterSet.has(repo.id)) {
|
||||
continue
|
||||
}
|
||||
if ((args.worktreesByRepo[repo.id]?.length ?? 0) === 0) {
|
||||
const hasNoWorktrees = (args.worktreesByRepo[repo.id]?.length ?? 0) === 0
|
||||
// Why: workspace filters hide cards, but must not rewrite the visible
|
||||
// membership of a persisted Project Group. #8865
|
||||
const isFilteredProjectGroupMember = repo.projectGroupId != null && !visibleRepoIds.has(repo.id)
|
||||
if (hasNoWorktrees || isFilteredProjectGroupMember) {
|
||||
placeholderRepoIds.add(repo.id)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2213,6 +2213,73 @@ describe('project groups', () => {
|
||||
expect(rows[0]).toMatchObject({ label: 'Platform' })
|
||||
})
|
||||
|
||||
it('keeps sleep-filtered Project Group members as empty project headers', () => {
|
||||
// Why: #8865 — Hide sleeping removes workspace cards; membership placeholders
|
||||
// must still project the grouped project header so the group count stays honest.
|
||||
const group: ProjectGroup = {
|
||||
id: 'group-1',
|
||||
name: 'Platform',
|
||||
parentPath: '/platform',
|
||||
parentGroupId: null,
|
||||
createdFrom: 'folder-scan',
|
||||
tabOrder: 0,
|
||||
isCollapsed: false,
|
||||
color: null,
|
||||
createdAt: 1,
|
||||
updatedAt: 1
|
||||
}
|
||||
const sleepingRepo: Repo = {
|
||||
...repo,
|
||||
id: 'repo-sleeping',
|
||||
displayName: 'sleeping-project',
|
||||
projectGroupId: group.id
|
||||
}
|
||||
const awakeRepo: Repo = {
|
||||
...repo,
|
||||
id: 'repo-awake',
|
||||
displayName: 'awake-project',
|
||||
projectGroupId: group.id
|
||||
}
|
||||
const awakeWorktree: Worktree = {
|
||||
...worktree,
|
||||
id: 'wt-awake',
|
||||
repoId: awakeRepo.id,
|
||||
path: '/tmp/awake'
|
||||
}
|
||||
|
||||
const rows = buildRows(
|
||||
'repo',
|
||||
[awakeWorktree],
|
||||
new Map([
|
||||
[sleepingRepo.id, sleepingRepo],
|
||||
[awakeRepo.id, awakeRepo]
|
||||
]),
|
||||
null,
|
||||
new Set(),
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
{},
|
||||
new Map([[awakeWorktree.id, awakeWorktree]]),
|
||||
false,
|
||||
undefined,
|
||||
[group],
|
||||
new Set([sleepingRepo.id])
|
||||
)
|
||||
|
||||
expect(rows[0]).toMatchObject({
|
||||
type: 'header',
|
||||
key: 'project-group:group-1',
|
||||
count: 2
|
||||
})
|
||||
// Why: empty/placeholder projects sort after projects with visible activity.
|
||||
expect(rows.filter((row) => row.type === 'header').map((row) => row.key)).toEqual([
|
||||
'project-group:group-1',
|
||||
`repo:${awakeRepo.id}`,
|
||||
`repo:${sleepingRepo.id}`
|
||||
])
|
||||
})
|
||||
|
||||
it('renders ungrouped repos as top-level repo rows when Project Groups exist', () => {
|
||||
const group: ProjectGroup = {
|
||||
id: 'group-1',
|
||||
|
||||
@@ -0,0 +1,425 @@
|
||||
import type { IBufferLine } from '@xterm/xterm'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { registerHttpLinkStoreAccessor } from '@/lib/http-link-routing'
|
||||
import { openHttpLinkAtBufferPosition } from './terminal-url-link-hit-testing'
|
||||
|
||||
const COLS = 40
|
||||
const WIDE_GLYPH = '界'
|
||||
const openUrlMock = vi.fn()
|
||||
|
||||
function makeBufferLine(content: string, cols = COLS): IBufferLine {
|
||||
return makeBufferLineFromCells(
|
||||
Array.from(content, (text) => ({ text, width: text === WIDE_GLYPH ? 2 : 1 })),
|
||||
cols
|
||||
)
|
||||
}
|
||||
|
||||
function makeBufferLineFromCells(
|
||||
cells: { text: string; width: number }[],
|
||||
cols = COLS
|
||||
): IBufferLine {
|
||||
const columns: number[] = []
|
||||
let column = 0
|
||||
let text = ''
|
||||
for (const cell of cells) {
|
||||
text += cell.text
|
||||
for (let index = 0; index < cell.text.length; index++) {
|
||||
columns.push(column)
|
||||
}
|
||||
column += cell.width
|
||||
}
|
||||
while (column < cols) {
|
||||
text += ' '
|
||||
columns.push(column)
|
||||
column++
|
||||
}
|
||||
columns.push(column)
|
||||
|
||||
return {
|
||||
isWrapped: false,
|
||||
length: cols,
|
||||
translateToString: (
|
||||
_trimRight?: boolean,
|
||||
startColumn = 0,
|
||||
endColumn = text.length,
|
||||
outColumns?: number[]
|
||||
) => {
|
||||
outColumns?.splice(0, outColumns.length, ...columns.slice(startColumn, endColumn + 1))
|
||||
return text.slice(startColumn, endColumn)
|
||||
}
|
||||
} as IBufferLine
|
||||
}
|
||||
|
||||
describe('edge-wrapped terminal HTTP links', () => {
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal('window', { api: { shell: { openUrl: openUrlMock } } })
|
||||
registerHttpLinkStoreAccessor(() => ({
|
||||
settings: { openLinksInApp: false },
|
||||
setActiveWorktree: vi.fn(),
|
||||
createBrowserTab: vi.fn()
|
||||
}))
|
||||
openUrlMock.mockReset()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
it('opens a URL when a wide glyph starts the continuation row', () => {
|
||||
const prefix = 'https://example.com/wide/'
|
||||
const firstRow = `${prefix}${'c'.repeat(COLS - 1 - prefix.length)}`
|
||||
const continuationRow = `${WIDE_GLYPH}tail`
|
||||
const expectedUrl = new URL(`${firstRow}${continuationRow}`).toString()
|
||||
const rows = [makeBufferLine(firstRow), makeBufferLine(continuationRow)]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(expectedUrl)
|
||||
|
||||
openUrlMock.mockReset()
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 3, y: 2 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(expectedUrl)
|
||||
})
|
||||
|
||||
it('opens a URL when a multi-code-point wide cell starts the continuation row', () => {
|
||||
const wideCell = '👩💻'
|
||||
const prefix = 'https://example.com/wide/'
|
||||
const firstRow = `${prefix}${'c'.repeat(COLS - 1 - prefix.length)}`
|
||||
const continuationRow = `${wideCell}tail`
|
||||
const expectedUrl = new URL(`${firstRow}${continuationRow}`).toString()
|
||||
const rows = [
|
||||
makeBufferLine(firstRow),
|
||||
makeBufferLineFromCells([
|
||||
{ text: wideCell, width: 2 },
|
||||
...Array.from('tail', (text) => ({ text, width: 1 }))
|
||||
])
|
||||
]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(expectedUrl)
|
||||
|
||||
openUrlMock.mockReset()
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 1, y: 2 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(expectedUrl)
|
||||
})
|
||||
|
||||
it('does not use the wide-glyph tolerance for an ASCII continuation row', () => {
|
||||
const firstRow = `https://example.com/${'a'.repeat(COLS - 1 - 'https://example.com/'.length)}`
|
||||
const rows = [makeBufferLine(firstRow), makeBufferLine('next')]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(firstRow)
|
||||
|
||||
openUrlMock.mockReset()
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 2, y: 2 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(false)
|
||||
expect(openUrlMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each(['Description: 123', 'Description:', '404: Not Found', 'HTTP/2: 200', 'HTTP/2:200'])(
|
||||
'does not append a %s label when a complete URL ends at the terminal edge',
|
||||
(labelRow) => {
|
||||
const firstRow = `https://example.com/${'a'.repeat(COLS - 1 - 'https://example.com/'.length)}/`
|
||||
const rows = [makeBufferLine(firstRow), makeBufferLine(labelRow)]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(firstRow).toHaveLength(COLS)
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(firstRow)
|
||||
|
||||
openUrlMock.mockReset()
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 2, y: 2 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(false)
|
||||
expect(openUrlMock).not.toHaveBeenCalled()
|
||||
}
|
||||
)
|
||||
|
||||
it('does not append an adjacent URL that starts on the next row', () => {
|
||||
const firstRow = `https://example.com/${'a'.repeat(COLS - 'https://example.com/'.length)}`
|
||||
const secondRow = 'https://two.test/path'
|
||||
const rows = [makeBufferLine(firstRow), makeBufferLine(secondRow)]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(firstRow)
|
||||
|
||||
openUrlMock.mockReset()
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 11, y: 2 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(secondRow)
|
||||
})
|
||||
|
||||
it('keeps a full-width continuation row that ends in a colon', () => {
|
||||
const prefix = 'https://example.com/'
|
||||
const firstRow = `${prefix}${'a'.repeat(COLS - prefix.length)}`
|
||||
const secondRow = `${'b'.repeat(COLS - 1)}:`
|
||||
const thirdRow = 'tail'
|
||||
const expectedUrl = new URL(`${firstRow}${secondRow}${thirdRow}`).toString()
|
||||
const rows = [makeBufferLine(firstRow), makeBufferLine(secondRow), makeBufferLine(thirdRow)]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(expectedUrl)
|
||||
|
||||
openUrlMock.mockReset()
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 20, y: 2 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(expectedUrl)
|
||||
})
|
||||
|
||||
it('keeps a short colon-bearing URL continuation', () => {
|
||||
const firstRow = `https://example.com/${'a'.repeat(COLS - 'https://example.com/'.length)}`
|
||||
const secondRow = 'urn:abc'
|
||||
const expectedUrl = `${firstRow}${secondRow}`
|
||||
const rows = [makeBufferLine(firstRow), makeBufferLine(secondRow)]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 4, y: 2 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(expectedUrl)
|
||||
})
|
||||
|
||||
it('does not count text before the URL against the URL length limit', () => {
|
||||
const cols = 200
|
||||
const prefix = `${'P'.repeat(179)} `
|
||||
const url = `https://example.com/${'a'.repeat(1_880)}`
|
||||
const displayed = `${prefix}${url}`
|
||||
const rowTexts = Array.from({ length: Math.ceil(displayed.length / cols) }, (_value, index) =>
|
||||
displayed.slice(index * cols, (index + 1) * cols)
|
||||
)
|
||||
const rows = rowTexts.map((row) => makeBufferLine(row, cols))
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(url).toHaveLength(1_900)
|
||||
expect(displayed.length).toBeGreaterThan(2_048)
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 191, y: 1 }, cols, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(url)
|
||||
|
||||
openUrlMock.mockReset()
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 10, y: rows.length }, cols, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(url)
|
||||
})
|
||||
|
||||
it('reconstructs a narrow-terminal URL spanning more than 20 rows from either end', () => {
|
||||
const url = `https://example.com/${'a'.repeat(1_000)}`
|
||||
const rowTexts = Array.from({ length: Math.ceil(url.length / COLS) }, (_value, index) =>
|
||||
url.slice(index * COLS, (index + 1) * COLS)
|
||||
)
|
||||
const rows = rowTexts.map((row) => makeBufferLine(row))
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(url.length).toBeLessThanOrEqual(2_048)
|
||||
expect(rowTexts.length).toBeGreaterThan(20)
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(url)
|
||||
|
||||
openUrlMock.mockReset()
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 5, y: rowTexts.length }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(url)
|
||||
})
|
||||
|
||||
it('reconstructs a wide-glyph URL whose row span exceeds the single-width column bound', () => {
|
||||
const wideCount = 1_100
|
||||
const url = `https://example.com/${WIDE_GLYPH.repeat(wideCount)}`
|
||||
const rowTexts: string[] = []
|
||||
let row = ''
|
||||
let width = 0
|
||||
for (const char of url) {
|
||||
const charWidth = char === WIDE_GLYPH ? 2 : 1
|
||||
if (width + charWidth > COLS) {
|
||||
rowTexts.push(row)
|
||||
row = ''
|
||||
width = 0
|
||||
}
|
||||
row += char
|
||||
width += charWidth
|
||||
}
|
||||
if (row.length > 0) {
|
||||
rowTexts.push(row)
|
||||
}
|
||||
const rows = rowTexts.map((rowText) => makeBufferLine(rowText))
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(url.length).toBeLessThanOrEqual(2_048)
|
||||
expect(rowTexts.length).toBeGreaterThan(Math.ceil(2_048 / COLS) + 1)
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 5, y: rowTexts.length }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(new URL(url).toString())
|
||||
})
|
||||
|
||||
it('joins a real multi-line URL whose continuation is a Chinese path segment', () => {
|
||||
const prefix = 'https://example.com/'
|
||||
const firstRow = `${prefix}${'a'.repeat(COLS - prefix.length)}`
|
||||
const continuation = `${WIDE_GLYPH}文档/路径`
|
||||
const expectedUrl = new URL(`${firstRow}${continuation}`).toString()
|
||||
const rows = [makeBufferLine(firstRow), makeBufferLine(continuation)]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(firstRow).toHaveLength(COLS)
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 11, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(expectedUrl)
|
||||
})
|
||||
|
||||
it('does not glue a mid-row URL to a next-line Windows path', () => {
|
||||
const firstRow = 'Repo: https://example.com/repo/'
|
||||
const secondRow = 'C:\\Users\\demo\\project\\file.ts'
|
||||
const rows = [makeBufferLine(firstRow), makeBufferLine(secondRow)]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: firstRow.indexOf('https') + 8, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith('https://example.com/repo/')
|
||||
})
|
||||
|
||||
it('does not create an edge candidate for a scheme embedded in a word', () => {
|
||||
const embeddedUrl = `abchttps://example.com/${'a'.repeat(
|
||||
COLS - 'abchttps://example.com/'.length
|
||||
)}`
|
||||
const rows = [makeBufferLine(embeddedUrl), makeBufferLine('tail')]
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 10, y: 1 }, COLS, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(false)
|
||||
expect(openUrlMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps a URL under the length limit when its final row also contains a label', () => {
|
||||
const cols = 200
|
||||
const url = `https://example.com/${'a'.repeat(2_020)}`
|
||||
const displayed = `${url} ${'L'.repeat(159)}`
|
||||
const rowTexts = Array.from({ length: Math.ceil(displayed.length / cols) }, (_value, index) =>
|
||||
displayed.slice(index * cols, (index + 1) * cols)
|
||||
)
|
||||
const rows = rowTexts.map((row) => makeBufferLine(row, cols))
|
||||
const buffer = { getLine: (y: number) => rows[y] }
|
||||
|
||||
expect(url).toHaveLength(2_040)
|
||||
expect(rowTexts).toHaveLength(11)
|
||||
expect(rowTexts.at(-1)).toHaveLength(cols)
|
||||
expect(
|
||||
openHttpLinkAtBufferPosition(buffer, { x: 10, y: 1 }, cols, {
|
||||
worktreeId: 'wt-1',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
).toBe(true)
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(url)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,183 @@
|
||||
import type { IBufferLine } from '@xterm/xterm'
|
||||
import { TERMINAL_HTTP_URL_MAX_LENGTH } from './terminal-http-link-limits'
|
||||
import { translateLineWithColumns, type WrappedLogicalLine } from './wrapped-terminal-link-ranges'
|
||||
|
||||
const HTTP_SCHEME_PATTERN = /https?:\/\//i
|
||||
const HTTP_SCHEME_START_PATTERN = /^https?:\/\//i
|
||||
const HTTP_STATUS_ROW_PATTERN = /^HTTP\/\d(?:\.\d)?:\d{3}(?:\s|$)/i
|
||||
const VERTICAL_LAYOUT_FRAME_PATTERN = /[│┃║╎╏┆┇┊┋|]/
|
||||
const EMPTY_LABEL_ROW_PATTERN = /^[^\s:][^:]*:$/
|
||||
const LABEL_WITH_SPACING_ROW_PATTERN = /^[^\s:][^:]*:\s/
|
||||
|
||||
function maxEdgeWrappedHttpRows(lineLength: number): number {
|
||||
// Why: every continued row reaches the terminal edge, so even all-width-2
|
||||
// cells carry at least floor(columns / 2) characters per row; the URL length
|
||||
// limit then bounds the scan without truncating URLs on narrow terminals.
|
||||
// The +2 covers the start and tail rows, which may hold only a fragment.
|
||||
const minCharsPerFullRow = Math.max(1, Math.floor(lineLength / 2))
|
||||
return Math.ceil(TERMINAL_HTTP_URL_MAX_LENGTH / minCharsPerFullRow) + 2
|
||||
}
|
||||
|
||||
type TrimmedTranslatedLine = {
|
||||
text: string
|
||||
sourceText: string
|
||||
columns: number[]
|
||||
lineLength: number
|
||||
}
|
||||
|
||||
function trimRightTranslatedLine(line: IBufferLine): TrimmedTranslatedLine | null {
|
||||
const translated = translateLineWithColumns(line)
|
||||
let endIndex = translated.text.length
|
||||
while (endIndex > 0 && /\s/.test(translated.text[endIndex - 1])) {
|
||||
endIndex--
|
||||
}
|
||||
if (endIndex === 0) {
|
||||
return null
|
||||
}
|
||||
return {
|
||||
text: translated.text.slice(0, endIndex),
|
||||
sourceText: translated.text,
|
||||
columns: translated.columns.slice(0, endIndex + 1),
|
||||
lineLength: line.length
|
||||
}
|
||||
}
|
||||
|
||||
function firstCellWidth(row: TrimmedTranslatedLine): number {
|
||||
const firstColumn = row.columns[0]
|
||||
if (firstColumn === undefined) {
|
||||
return 0
|
||||
}
|
||||
const nextColumn = row.columns.find((column) => column > firstColumn)
|
||||
return nextColumn === undefined ? 0 : nextColumn - firstColumn
|
||||
}
|
||||
|
||||
function isAsciiWordCode(code: number): boolean {
|
||||
return (
|
||||
(code >= 48 && code <= 57) ||
|
||||
(code >= 65 && code <= 90) ||
|
||||
code === 95 ||
|
||||
(code >= 97 && code <= 122)
|
||||
)
|
||||
}
|
||||
|
||||
function findHttpSchemeIndex(text: string): number {
|
||||
let searchStart = 0
|
||||
while (searchStart < text.length) {
|
||||
const relativeIndex = text.slice(searchStart).search(HTTP_SCHEME_PATTERN)
|
||||
if (relativeIndex === -1) {
|
||||
return -1
|
||||
}
|
||||
const schemeIndex = searchStart + relativeIndex
|
||||
if (schemeIndex === 0 || !isAsciiWordCode(text.charCodeAt(schemeIndex - 1))) {
|
||||
return schemeIndex
|
||||
}
|
||||
searchStart = schemeIndex + 1
|
||||
}
|
||||
return -1
|
||||
}
|
||||
|
||||
function rowsCanContinueAtTerminalEdge(
|
||||
row: WrappedLogicalLine['rows'][number],
|
||||
nextRow: TrimmedTranslatedLine
|
||||
): boolean {
|
||||
// Why: a cursor-positioned label is not URL continuation evidence even when
|
||||
// the preceding URL happens to end at the terminal edge (#8832).
|
||||
const nextColumnAfterText = nextRow.columns.at(-1)
|
||||
const nextRowReachesEdge =
|
||||
nextColumnAfterText !== undefined && nextColumnAfterText >= nextRow.lineLength
|
||||
const isLabelRow =
|
||||
LABEL_WITH_SPACING_ROW_PATTERN.test(nextRow.text) ||
|
||||
((EMPTY_LABEL_ROW_PATTERN.test(nextRow.text) || HTTP_STATUS_ROW_PATTERN.test(nextRow.text)) &&
|
||||
!nextRowReachesEdge)
|
||||
if (isLabelRow || HTTP_SCHEME_START_PATTERN.test(nextRow.text)) {
|
||||
return false
|
||||
}
|
||||
const columnAfterText = row.columns.at(-1)
|
||||
if (columnAfterText === undefined || columnAfterText < row.lineLength - 1) {
|
||||
return false
|
||||
}
|
||||
return columnAfterText >= row.lineLength || firstCellWidth(nextRow) > 1
|
||||
}
|
||||
|
||||
export function buildEdgeWrappedHttpLogicalLineCandidates(
|
||||
buffer: { getLine(y: number): IBufferLine | undefined },
|
||||
bufferLineNumber: number
|
||||
): WrappedLogicalLine[] {
|
||||
// Why: cursor-positioned output lacks xterm wrap metadata, but an HTTP URL
|
||||
// may still continue when each earlier row reaches the terminal edge.
|
||||
const currentY = bufferLineNumber - 1
|
||||
const currentLine = buffer.getLine(currentY)
|
||||
if (!currentLine) {
|
||||
return []
|
||||
}
|
||||
const maxRows = maxEdgeWrappedHttpRows(currentLine.length)
|
||||
|
||||
const candidates: WrappedLogicalLine[] = []
|
||||
const translatedLines = new Map<number, TrimmedTranslatedLine | null>()
|
||||
const getTranslatedLine = (rowY: number): TrimmedTranslatedLine | null => {
|
||||
if (translatedLines.has(rowY)) {
|
||||
return translatedLines.get(rowY) ?? null
|
||||
}
|
||||
const line = buffer.getLine(rowY)
|
||||
const translated = line ? trimRightTranslatedLine(line) : null
|
||||
translatedLines.set(rowY, translated)
|
||||
return translated
|
||||
}
|
||||
const minY = Math.max(0, currentY - maxRows + 1)
|
||||
for (let startY = currentY; startY >= minY; startY--) {
|
||||
const start = getTranslatedLine(startY)
|
||||
const schemeIndex = start ? findHttpSchemeIndex(start.text) : -1
|
||||
if (!start || schemeIndex === -1 || VERTICAL_LAYOUT_FRAME_PATTERN.test(start.text)) {
|
||||
continue
|
||||
}
|
||||
|
||||
let text = ''
|
||||
const rows: WrappedLogicalLine['rows'] = []
|
||||
for (let rowY = startY; rowY < startY + maxRows; rowY++) {
|
||||
const line = buffer.getLine(rowY)
|
||||
const translated = getTranslatedLine(rowY)
|
||||
if (!line || !translated) {
|
||||
break
|
||||
}
|
||||
if (VERTICAL_LAYOUT_FRAME_PATTERN.test(translated.text)) {
|
||||
break
|
||||
}
|
||||
if (
|
||||
rows.length > 0 &&
|
||||
!line.isWrapped &&
|
||||
!rowsCanContinueAtTerminalEdge(rows.at(-1)!, translated)
|
||||
) {
|
||||
break
|
||||
}
|
||||
const fragmentStartIndex = rowY === startY ? schemeIndex : 0
|
||||
const fragment = translated.text.slice(fragmentStartIndex)
|
||||
const whitespaceIndex = fragment.search(/\s/)
|
||||
const possibleUrlFragmentLength = whitespaceIndex === -1 ? fragment.length : whitespaceIndex
|
||||
if (text.length + possibleUrlFragmentLength > TERMINAL_HTTP_URL_MAX_LENGTH) {
|
||||
break
|
||||
}
|
||||
|
||||
rows.push({
|
||||
y: rowY,
|
||||
text: fragment,
|
||||
sourceText: translated.sourceText,
|
||||
columns: translated.columns.slice(fragmentStartIndex),
|
||||
startIndex: text.length,
|
||||
isWrapped: line.isWrapped,
|
||||
lineLength: line.length
|
||||
})
|
||||
text += fragment
|
||||
if (rows.length > 1 && rowY >= currentY) {
|
||||
candidates.push({
|
||||
text,
|
||||
rows: [...rows],
|
||||
fingerprint: `edge-http:${rows.map((row) => `${row.y}:${row.sourceText}`).join('\0')}`
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return candidates.sort(
|
||||
(left, right) => right.rows.length - left.rows.length || right.text.length - left.text.length
|
||||
)
|
||||
}
|
||||
@@ -0,0 +1,203 @@
|
||||
/**
|
||||
* Issue #8832 — Cmd-click URL must not glue the next logical line.
|
||||
*
|
||||
* Root cause: path hard-wrap reconstruction (from #8339) still joins a URL
|
||||
* suffix ending in `/` with the next row's path-like prefix. HTTP hit-testing
|
||||
* must not consume those candidates; it uses soft-wrap, framed hard-wrap HTTP,
|
||||
* and edge-wrap reconstruction only.
|
||||
*
|
||||
* Re-run:
|
||||
* pnpm exec vitest run --config config/vitest.config.ts \
|
||||
* src/renderer/src/components/terminal-pane/repro-8832-url-next-line.test.ts
|
||||
*/
|
||||
import type { IBufferLine } from '@xterm/xterm'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { registerHttpLinkStoreAccessor } from '@/lib/http-link-routing'
|
||||
import { buildCandidateLogicalLinesForBufferPosition } from './terminal-file-link-hit-testing'
|
||||
import {
|
||||
extractTerminalHttpLinks,
|
||||
openHttpLinkAtBufferPosition
|
||||
} from './terminal-url-link-hit-testing'
|
||||
import { buildHardWrappedPathLogicalLineCandidates } from './wrapped-terminal-link-ranges'
|
||||
|
||||
const LINE_1 = 'Repo: https://github.com/stablyai/orca/'
|
||||
const LINE_2 = 'Description: 123'
|
||||
const EXPECTED_URL = 'https://github.com/stablyai/orca/'
|
||||
const BUGGY_URL = 'https://github.com/stablyai/orca/Description'
|
||||
|
||||
const openUrlMock = vi.fn()
|
||||
|
||||
function makeBufferLine(
|
||||
text: string,
|
||||
options: { cols?: number; isWrapped?: boolean } = {}
|
||||
): IBufferLine {
|
||||
const cols = options.cols ?? Math.max(text.length, 80)
|
||||
const padded = text.padEnd(cols)
|
||||
return {
|
||||
isWrapped: options.isWrapped ?? false,
|
||||
length: cols,
|
||||
getCell: () => undefined,
|
||||
translateToString: (
|
||||
_trimRight?: boolean,
|
||||
startColumn = 0,
|
||||
endColumn = padded.length,
|
||||
outColumns?: number[]
|
||||
) => {
|
||||
if (outColumns) {
|
||||
outColumns.length = 0
|
||||
for (let index = startColumn; index <= endColumn; index++) {
|
||||
outColumns.push(index)
|
||||
}
|
||||
}
|
||||
return padded.slice(startColumn, endColumn)
|
||||
}
|
||||
} as IBufferLine
|
||||
}
|
||||
|
||||
function twoRowBuffer(
|
||||
row0: string,
|
||||
row1: string,
|
||||
options: { cols?: number; softWrapped?: boolean } = {}
|
||||
): { getLine(y: number): IBufferLine | undefined } {
|
||||
const cols = options.cols ?? 120
|
||||
const rows = [
|
||||
makeBufferLine(row0, { cols, isWrapped: false }),
|
||||
makeBufferLine(row1, { cols, isWrapped: options.softWrapped === true })
|
||||
]
|
||||
return { getLine: (y: number) => rows[y] }
|
||||
}
|
||||
|
||||
function issueBuffer(): { getLine(y: number): IBufferLine | undefined } {
|
||||
return twoRowBuffer(LINE_1, LINE_2, { cols: 120 })
|
||||
}
|
||||
|
||||
function openUrlAt(
|
||||
buffer: { getLine(y: number): IBufferLine | undefined },
|
||||
x: number,
|
||||
y: number,
|
||||
cols = 120
|
||||
) {
|
||||
openUrlMock.mockReset()
|
||||
const opened = openHttpLinkAtBufferPosition(buffer, { x, y }, cols, {
|
||||
worktreeId: 'wt-repro-8832',
|
||||
forceSystemBrowser: true
|
||||
})
|
||||
return { opened, url: openUrlMock.mock.calls[0]?.[0] as string | undefined }
|
||||
}
|
||||
|
||||
describe('#8832 hard-wrapped path candidates glue next-line text into URLs', () => {
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal('navigator', { userAgent: 'Macintosh' })
|
||||
vi.stubGlobal('window', { api: { shell: { openUrl: openUrlMock } } })
|
||||
registerHttpLinkStoreAccessor(() => ({
|
||||
settings: { openLinksInApp: false },
|
||||
setActiveWorktree: vi.fn(),
|
||||
createBrowserTab: vi.fn()
|
||||
}))
|
||||
openUrlMock.mockReset()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
it('still builds a multi-row path candidate that concatenates Description onto the URL', () => {
|
||||
// Why: documents the path-reconstruction poison that HTTP hit-testing must
|
||||
// not consume. File links remain protected by existence checks.
|
||||
const buffer = issueBuffer()
|
||||
const candidates = buildHardWrappedPathLogicalLineCandidates(buffer, 1)
|
||||
const multiRow = candidates.filter((candidate) => candidate.rows.length > 1)
|
||||
|
||||
expect(multiRow.some((candidate) => candidate.text.includes('Description'))).toBe(true)
|
||||
|
||||
const glued = multiRow.find((candidate) => candidate.text.includes('https://'))
|
||||
expect(glued).toBeDefined()
|
||||
expect(glued!.text).toContain('https://github.com/stablyai/orca/Description')
|
||||
})
|
||||
|
||||
it('HTTP extraction on a path logical line still yields the glued URL', () => {
|
||||
const buffer = issueBuffer()
|
||||
const candidates = buildCandidateLogicalLinesForBufferPosition(buffer, 1)
|
||||
const extracted = candidates.flatMap((line) => extractTerminalHttpLinks(line.text))
|
||||
|
||||
expect(extracted.map((link) => link.url)).toContain(BUGGY_URL)
|
||||
const singleLineUrls = extractTerminalHttpLinks(LINE_1).map((link) => link.url)
|
||||
expect(singleLineUrls).toEqual([EXPECTED_URL])
|
||||
})
|
||||
|
||||
it('openHttpLinkAtBufferPosition opens the unglued URL when Cmd-clicking the URL row', () => {
|
||||
const buffer = issueBuffer()
|
||||
const urlStart = LINE_1.indexOf('https://')
|
||||
const { opened, url } = openUrlAt(buffer, urlStart + 10, 1)
|
||||
|
||||
expect(opened).toBe(true)
|
||||
expect(url).toBe(EXPECTED_URL)
|
||||
expect(url).not.toBe(BUGGY_URL)
|
||||
})
|
||||
|
||||
it.each([
|
||||
['Chinese label', '说明: 中文路径/文件.ts'],
|
||||
['Windows path', 'C:\\Users\\demo\\project\\README.md'],
|
||||
['POSIX path', '/usr/local/bin/orca'],
|
||||
['relative path', './src/main/index.ts'],
|
||||
['UNC-ish path', '\\\\server\\share\\file.txt']
|
||||
])('does not glue a mid-row URL to a next-line %s', (_label, nextLine) => {
|
||||
const row0 = 'See https://example.com/docs/'
|
||||
const buffer = twoRowBuffer(row0, nextLine, { cols: 100 })
|
||||
const urlStart = row0.indexOf('https://')
|
||||
const { opened, url } = openUrlAt(buffer, urlStart + 8, 1)
|
||||
|
||||
expect(opened).toBe(true)
|
||||
expect(url).toBe('https://example.com/docs/')
|
||||
expect(url).not.toMatch(/说明|Users|usr|src|server/)
|
||||
})
|
||||
|
||||
it('still joins a soft-wrapped multi-line URL via native wrap metadata', () => {
|
||||
const cols = 40
|
||||
const url = 'https://example.com/very/long/path/segments/that/continue/more'
|
||||
const row0 = url.slice(0, cols)
|
||||
const row1 = url.slice(cols)
|
||||
const buffer = twoRowBuffer(row0, row1, { cols, softWrapped: true })
|
||||
const first = openUrlAt(buffer, 11, 1, cols)
|
||||
const second = openUrlAt(buffer, 5, 2, cols)
|
||||
|
||||
expect(first.opened).toBe(true)
|
||||
expect(first.url).toBe(url)
|
||||
expect(second.opened).toBe(true)
|
||||
expect(second.url).toBe(url)
|
||||
})
|
||||
|
||||
it('still joins an edge-wrapped multi-line URL without wrap metadata', () => {
|
||||
const cols = 40
|
||||
const url = 'https://example.com/very/long/path/segments/that/continue/more'
|
||||
const row0 = url.slice(0, cols)
|
||||
const row1 = url.slice(cols)
|
||||
expect(row0).toHaveLength(cols)
|
||||
const buffer = twoRowBuffer(row0, row1, { cols, softWrapped: false })
|
||||
const first = openUrlAt(buffer, 11, 1, cols)
|
||||
const second = openUrlAt(buffer, 5, 2, cols)
|
||||
|
||||
expect(first.opened).toBe(true)
|
||||
expect(first.url).toBe(url)
|
||||
expect(second.opened).toBe(true)
|
||||
expect(second.url).toBe(url)
|
||||
})
|
||||
|
||||
it('does not treat file:// as an HTTP link (file routing stays separate)', () => {
|
||||
const row0 = 'open file:///Users/demo/project/README.md'
|
||||
const buffer = twoRowBuffer(row0, 'next: line', { cols: 100 })
|
||||
const { opened, url } = openUrlAt(buffer, row0.indexOf('file://') + 4, 1)
|
||||
|
||||
expect(opened).toBe(false)
|
||||
expect(url).toBeUndefined()
|
||||
})
|
||||
|
||||
it('opens only the HTTP URL when a Windows path shares the same row', () => {
|
||||
const row0 = 'https://example.com/a C:\\Users\\demo\\file.txt'
|
||||
const buffer = twoRowBuffer(row0, 'Description: more', { cols: 100 })
|
||||
const { opened, url } = openUrlAt(buffer, 8, 1)
|
||||
|
||||
expect(opened).toBe(true)
|
||||
expect(url).toBe('https://example.com/a')
|
||||
})
|
||||
})
|
||||
@@ -379,6 +379,44 @@ describe('hard-wrapped terminal HTTP clicks', () => {
|
||||
expect(openUrlMock).toHaveBeenCalledWith('http://example.com/')
|
||||
})
|
||||
|
||||
it('does not glue the next logical line onto a URL that ends mid-row (#8832)', () => {
|
||||
const { terminal, registrations } = makeTerminal({
|
||||
cols: 80,
|
||||
urlRows: ['Repo: https://github.com/stablyai/orca/', 'Description: 123'],
|
||||
softWrapped: false
|
||||
})
|
||||
const disposable = installHttpLinkClickFallback(terminal, { worktreeId: 'wt-1' })
|
||||
|
||||
const fallback = registrations.find(
|
||||
([name, _listener, options]) => name === 'mouseup' && options === undefined
|
||||
)?.[1]
|
||||
fallback!(mouseEventForRow(0))
|
||||
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith('https://github.com/stablyai/orca/')
|
||||
disposable.dispose()
|
||||
})
|
||||
|
||||
it('still joins a URL hard-wrapped at the row edge without native wrap metadata', () => {
|
||||
const cols = 40
|
||||
const url = 'https://example.com/very/long/path/segments/that/continue/more'
|
||||
const { terminal, registrations } = makeTerminal({
|
||||
cols,
|
||||
urlRows: [url.slice(0, cols), url.slice(cols)],
|
||||
softWrapped: false
|
||||
})
|
||||
const disposable = installHttpLinkClickFallback(terminal, { worktreeId: 'wt-1' })
|
||||
|
||||
const fallback = registrations.find(
|
||||
([name, _listener, options]) => name === 'mouseup' && options === undefined
|
||||
)?.[1]
|
||||
fallback!(mouseEventForRow(0))
|
||||
|
||||
expect(openUrlMock).toHaveBeenCalledOnce()
|
||||
expect(openUrlMock).toHaveBeenCalledWith(url)
|
||||
disposable.dispose()
|
||||
})
|
||||
|
||||
it('does not suppress a modifier-click when the buffer position is not an HTTP link', () => {
|
||||
const { terminal, registrations } = makeTerminal({ urlRows: ['not-a-link'] })
|
||||
const disposable = installHttpLinkClickFallback(terminal, { worktreeId: 'wt-1' })
|
||||
|
||||
@@ -1,10 +1,8 @@
|
||||
import type { IBufferLine, IBufferRange, IDisposable, Terminal } from '@xterm/xterm'
|
||||
import { openHttpLink } from '@/lib/http-link-routing'
|
||||
import {
|
||||
buildCandidateLogicalLinesForBufferPosition,
|
||||
dedupeLogicalLines
|
||||
} from './terminal-file-link-hit-testing'
|
||||
import { buildEdgeWrappedHttpLogicalLineCandidates } from './edge-wrapped-terminal-http-links'
|
||||
import { buildHardWrappedHttpLogicalLineCandidates } from './hard-wrapped-terminal-http-links'
|
||||
import { dedupeLogicalLines } from './terminal-file-link-hit-testing'
|
||||
import { isTerminalHttpLinkActivation } from './terminal-http-link-activation'
|
||||
import { installTerminalLinkPtyMouseSuppression } from './terminal-link-pty-mouse-suppression'
|
||||
import { getTerminalBufferPositionForMouseEvent } from './terminal-mouse-buffer-position'
|
||||
@@ -271,7 +269,10 @@ function findHttpLinkAtBufferPosition(
|
||||
? [nativeWrappedLogicalLine]
|
||||
: []),
|
||||
...buildHardWrappedHttpLogicalLineCandidates(buffer, position.y),
|
||||
...buildCandidateLogicalLinesForBufferPosition(buffer, position.y)
|
||||
...buildEdgeWrappedHttpLogicalLineCandidates(buffer, position.y),
|
||||
...(nativeWrappedLogicalLine && nativeWrappedLogicalLine.rows.length === 1
|
||||
? [nativeWrappedLogicalLine]
|
||||
: [])
|
||||
])
|
||||
if (logicalLines.length === 0) {
|
||||
return null
|
||||
@@ -290,6 +291,17 @@ function findHttpLinkAtBufferPosition(
|
||||
return null
|
||||
}
|
||||
|
||||
function rangeContainsBufferPosition(
|
||||
range: IBufferRange,
|
||||
position: { x: number; y: number },
|
||||
terminalColumns: number
|
||||
): boolean {
|
||||
const lower = range.start.y * terminalColumns + range.start.x
|
||||
const upper = range.end.y * terminalColumns + range.end.x
|
||||
const current = position.y * terminalColumns + position.x
|
||||
return lower <= current && current <= upper
|
||||
}
|
||||
|
||||
export function openTerminalHttpLink(url: string, deps: UrlLinkHitTestDeps): void {
|
||||
if (deps.forceSystemBrowser) {
|
||||
openHttpLink(url, { worktreeId: deps.worktreeId, forceSystemBrowser: true })
|
||||
@@ -316,14 +328,3 @@ export function openTerminalHttpLink(url: string, deps: UrlLinkHitTestDeps): voi
|
||||
openHttpLink(url, { worktreeId: deps.worktreeId, forceSystemBrowser: true })
|
||||
})
|
||||
}
|
||||
|
||||
function rangeContainsBufferPosition(
|
||||
range: IBufferRange,
|
||||
position: { x: number; y: number },
|
||||
terminalColumns: number
|
||||
): boolean {
|
||||
const lower = range.start.y * terminalColumns + range.start.x
|
||||
const upper = range.end.y * terminalColumns + range.end.x
|
||||
const current = position.y * terminalColumns + position.x
|
||||
return lower <= current && current <= upper
|
||||
}
|
||||
|
||||
@@ -1491,6 +1491,11 @@ export type GitHubWorkItem = {
|
||||
labels: string[]
|
||||
updatedAt: string
|
||||
author: string | null
|
||||
// Why: GHE user logins don't exist on github.com, so the github.com/{login}.png
|
||||
// fallback 404s. Carry the API-provided avatar_url so github.com + Enterprise
|
||||
// both render; absent on the gh-pr-view path (gh omits avatar), then the UI
|
||||
// falls back to the login URL and finally an initials placeholder. See #8784.
|
||||
authorAvatarUrl?: string
|
||||
branchName?: string
|
||||
baseRefName?: string
|
||||
// Why: PR checks are keyed by head commit; carrying this lets task rows use
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import { waitForSessionReady } from './helpers/store'
|
||||
|
||||
test.describe('Settings sidebar search on the Shortcuts pane', () => {
|
||||
test('pane-title-only query keeps rows visible and local search usable', async ({ orcaPage }) => {
|
||||
await waitForSessionReady(orcaPage)
|
||||
|
||||
await orcaPage.evaluate(async () => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
throw new Error('window.__store is not available')
|
||||
}
|
||||
// Why: the spec asserts on English strings; the host machine may run a
|
||||
// non-English system locale, which 'system' would follow.
|
||||
await store.getState().updateSettings({ uiLanguage: 'en' })
|
||||
store.getState().openSettingsPage()
|
||||
})
|
||||
|
||||
const searchInput = orcaPage.getByPlaceholder('Search settings')
|
||||
await expect(searchInput).toBeVisible()
|
||||
await searchInput.fill('shortcuts')
|
||||
|
||||
// The query matches the pane title, so the Shortcuts pane auto-activates.
|
||||
await expect(orcaPage.getByRole('heading', { name: 'Shortcuts', exact: true })).toBeVisible()
|
||||
|
||||
// Regression: a pane-title-only query used to blank the whole list (0/112).
|
||||
await expect(orcaPage.getByText('Go to File', { exact: true })).toBeVisible()
|
||||
await expect(orcaPage.getByText('No shortcuts match those filters.')).not.toBeVisible()
|
||||
|
||||
// Regression: the pane's own search was dead while the global query was
|
||||
// active, because it intersected with an already-empty base list.
|
||||
const localSearch = orcaPage.getByPlaceholder('Search command or keys')
|
||||
await localSearch.fill('go to')
|
||||
await expect(orcaPage.getByText('Go to File', { exact: true })).toBeVisible()
|
||||
await expect(orcaPage.getByText('Force Reload', { exact: true })).not.toBeVisible()
|
||||
|
||||
// A row-matching global query still narrows the list as before.
|
||||
await localSearch.clear()
|
||||
await searchInput.fill('worktree')
|
||||
await expect(orcaPage.getByText('Create worktree', { exact: true })).toBeVisible()
|
||||
await expect(orcaPage.getByText('Go to File', { exact: true })).not.toBeVisible()
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user