diff --git a/src/components/CobCommitTeaser.svelte b/src/components/CobCommitTeaser.svelte index db084527d..927b52f39 100644 --- a/src/components/CobCommitTeaser.svelte +++ b/src/components/CobCommitTeaser.svelte @@ -2,14 +2,10 @@ import type { Commit } from "@bindings/repo/Commit"; import type { Snippet } from "svelte"; - import { - absoluteTimestamp, - formatTimestamp, - gravatarURL, - twemoji, - } from "@app/lib/utils"; + import { commitTimes, formatTimestamp, twemoji } from "@app/lib/utils"; import Button from "@app/components/Button.svelte"; + import CommitAuthors from "@app/components/CommitAuthors.svelte"; import CompactCommitAuthorship from "@app/components/CompactCommitAuthorship.svelte"; import Icon from "@app/components/Icon.svelte"; import Id from "@app/components/Id.svelte"; @@ -101,13 +97,6 @@ gap: 0.5rem; min-width: 0; } - .commit-avatar { - width: 1rem; - height: 1rem; - border-radius: var(--border-radius-sm); - flex-shrink: 0; - object-fit: cover; - } .stacked-header .summary-title { flex: 1; min-width: 0; @@ -149,11 +138,7 @@ aria-label="commit-teaser"> {#if stacked}
- +
- + {formatTimestamp(commit.committer.time * 1000)}
diff --git a/src/components/CommitAuthors.svelte b/src/components/CommitAuthors.svelte new file mode 100644 index 000000000..9147d3ba7 --- /dev/null +++ b/src/components/CommitAuthors.svelte @@ -0,0 +1,279 @@ + + + + +{#snippet role(label: string, members: GitIdentity[])} +
+
{label}
+ {#each members as who (utils.identityKey(who))} +
+ +
+ {#if who.name} + {who.name} + {/if} + {#if who.email} + + {/if} +
+
+ {/each} +
+{/snippet} + + + {#snippet toggle()} +
+ {#each stacked as who, index (utils.identityKey(who))} + + {/each} + {#if overflow > 0} + +{overflow} + {/if} +
+ {/snippet} + + {#snippet popover()} +
+ {#if authorIsCommitter} + {@render role("Author & committer", [commit.author])} + {:else} + {@render role("Author", [commit.author])} + {@render role("Committer", [commit.committer])} + {/if} + {#if coAuthors.length > 0} + {@render role( + coAuthors.length === 1 ? "Co-author" : "Co-authors", + coAuthors, + )} + {/if} +
+ {/snippet} + diff --git a/src/components/CompactCommitAuthorship.svelte b/src/components/CompactCommitAuthorship.svelte index 78b58e656..59be56759 100644 --- a/src/components/CompactCommitAuthorship.svelte +++ b/src/components/CompactCommitAuthorship.svelte @@ -4,7 +4,7 @@ import * as utils from "@app/lib/utils"; - import HoverPopover from "@app/components/HoverPopover.svelte"; + import CommitAuthors from "@app/components/CommitAuthors.svelte"; interface Props { children: Snippet; @@ -22,80 +22,12 @@ align-items: center; white-space: nowrap; } - .person { - display: flex; - align-items: center; - flex-wrap: nowrap; - white-space: nowrap; - gap: 0.5rem; - font: var(--txt-code-regular); - } - .label { - color: var(--color-text-secondary); - } - .avatar { - width: 1rem; - height: 1rem; - }
- - {#snippet toggle()} -
- {#if commit.author.email === commit.committer.email} - avatar - {:else} - avatar - avatar - {/if} -
- {/snippet} - - {#snippet popover()} -
- {#if commit.author.email === commit.committer.email} -
-
Author
- avatar - {commit.author.name} -
- {:else} -
-
Author
- avatar - {commit.author.name} -
-
-
Committer
- avatar - {commit.committer.name} -
- {/if} -
- {/snippet} - + {@render children()} -
+
{utils.formatTimestamp(commit.committer.time * 1000)}
diff --git a/src/components/HoverPopover.svelte b/src/components/HoverPopover.svelte index ce10b3af5..da079ce18 100644 --- a/src/components/HoverPopover.svelte +++ b/src/components/HoverPopover.svelte @@ -35,6 +35,9 @@ // Whether the pointer is currently over the anchor or popover. let hovering = false; + // Whether focus is inside the anchor or popover, so that a keyboard user can + // reach any controls the popover holds. + let focused = false; // Whether a text-selection drag started inside the popover is ongoing. let selecting = false; @@ -46,31 +49,49 @@ visible = false; }, 200); - function enter() { - hovering = true; + function open() { hide.cancel(); show(); } - function leave() { - hovering = false; - show.cancel(); - // A selection drag may move the pointer out of the popover; keep it open - // until the drag ends (see endSelection). - if (!selecting) { + // Any one of pointer, focus or an ongoing selection drag is enough to keep + // the popover open, so closing waits until none of them holds it. + function close() { + if (!hovering && !focused && !selecting) { + show.cancel(); hide(); } } + function enter() { + hovering = true; + open(); + } + + function leave() { + hovering = false; + close(); + } + + // Focus crossing between the anchor and the portalled popover leaves and + // re-enters within one task, which the debounced hide absorbs. + function focus() { + focused = true; + open(); + } + + function blur() { + focused = false; + close(); + } + // A text-selection drag can move the pointer outside the popover and release // anywhere, so we follow it for the duration of the drag with a listener that // removes itself on release, rather than closing on pointer-leave. function endSelection() { selecting = false; document.removeEventListener("pointerup", endSelection); - if (!hovering) { - hide(); - } + close(); } function startSelection() { @@ -121,7 +142,9 @@ class="container" bind:this={anchorEl} onmouseenter={enter} - onmouseleave={leave}> + onmouseleave={leave} + onfocusin={focus} + onfocusout={blur}>
e.stopPropagation()}> {@render popover()} diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 0aee91a47..f7736e642 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -3,6 +3,7 @@ import type { Issue } from "@bindings/cob/issue/Issue"; import type { Patch } from "@bindings/cob/patch/Patch"; import type { Review } from "@bindings/cob/patch/Review"; import type { Config } from "@bindings/config/Config"; +import type { Commit } from "@bindings/repo/Commit"; import type { RepoInfo } from "@bindings/repo/RepoInfo"; import type { ComponentProps } from "svelte"; @@ -374,3 +375,54 @@ export function explorerUrl(path: string, config: Config): string { .replace("$host", seed) .replace("$rid$path", path); } + +export interface GitIdentity { + name: string; + email: string; +} + +// Two git identities are the same person when they share an address, compared +// case-insensitively the way `gravatarURL` normalises it. An identity may carry +// no address at all, so those fall back to the name rather than collapsing into +// each other. +export function identityKey(who: GitIdentity): string { + const email = who.email.trim().toLowerCase(); + return email ? `email:${email}` : `name:${who.name.trim().toLowerCase()}`; +} + +// A commit carries an author date and a commit date, which come apart on a +// rebase or an amend. Views show the commit date, so the tooltip only names the +// two apart when there is something to tell apart. +export function commitTimes(commit: Commit): string { + const committed = absoluteTimestamp(commit.committer.time * 1000); + if (commit.author.time === commit.committer.time) { + return committed; + } + + return [ + `Authored — ${absoluteTimestamp(commit.author.time * 1000)}`, + `Committed — ${committed}`, + ].join("\n"); +} + +// Extract `Co-authored-by:` trailers from a commit message. +// +// Git only treats the final paragraph of a message as trailers, so a line +// appearing anywhere earlier — quoted in prose, or in a nested patch — is not +// one and is ignored here too. Matching is case-insensitive because git's own +// trailer handling is. The name may be empty, since a git identity only +// requires an address. +export function coAuthors(message: string): GitIdentity[] { + const paragraphs = message.trimEnd().split(/(?:\r?\n){2,}/); + const trailers = paragraphs[paragraphs.length - 1] ?? ""; + const authors: GitIdentity[] = []; + + for (const line of trailers.split("\n")) { + const match = /^co-authored-by:\s*(.*?)\s*<([^<>\s]+)>$/i.exec(line.trim()); + if (match) { + authors.push({ name: match[1], email: match[2] }); + } + } + + return authors; +} diff --git a/src/views/repo/RepoCommit.svelte b/src/views/repo/RepoCommit.svelte index 52b9aee94..94713f699 100644 --- a/src/views/repo/RepoCommit.svelte +++ b/src/views/repo/RepoCommit.svelte @@ -13,14 +13,15 @@ import * as router from "@app/lib/router"; import type { SidebarData } from "@app/lib/router/definitions"; import { - absoluteTimestamp, + commitTimes, formatOid, formatTimestamp, - gravatarURL, + identityKey, pluralize, } from "@app/lib/utils"; import Button from "@app/components/Button.svelte"; + import CommitAuthors from "@app/components/CommitAuthors.svelte"; import DiffActions from "@app/components/DiffActions.svelte"; import DiffStatBadge from "@app/components/DiffStatBadge.svelte"; import Icon from "@app/components/Icon.svelte"; @@ -51,6 +52,10 @@ }>(); let allCollapsed = $state(false); + const authorIsCommitter = $derived( + identityKey(commit.author) === identityKey(commit.committer), + ); + const changedFiles = $derived(gitStatusEntries(diff.files)); const treePaths = $derived(changedFiles.map(file => file.path)); @@ -143,12 +148,6 @@ align-items: center; gap: 0.5rem; } - .summary-avatar { - width: 1rem; - height: 1rem; - border-radius: 999px; - flex: none; - } .summary-timestamp { color: var(--color-text-quaternary); } @@ -211,17 +210,12 @@
- + {commit.author.name} - committed + {authorIsCommitter ? "committed" : "authored"} - + {formatTimestamp(commit.committer.time * 1000)} diff --git a/tests/unit/utils.test.ts b/tests/unit/utils.test.ts index 3be1c5070..cd43a130f 100644 --- a/tests/unit/utils.test.ts +++ b/tests/unit/utils.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "vitest"; -import { safeHttpUrl } from "@app/lib/utils"; +import { coAuthors, safeHttpUrl } from "@app/lib/utils"; describe("safeHttpUrl", () => { test.each([ @@ -27,3 +27,56 @@ describe("safeHttpUrl", () => { expect(safeHttpUrl(input)).toBeUndefined(); }); }); + +describe("coAuthors", () => { + test("reads trailers from the final paragraph", () => { + expect( + coAuthors( + "Fix the thing\n\nSome body text.\n\nSigned-off-by: A \nCo-authored-by: Alice Lidell \nCo-authored-by: Bob ", + ), + ).toEqual([ + { name: "Alice Lidell", email: "alice@lidell.com" }, + { name: "Bob", email: "bob@example.com" }, + ]); + }); + + test("is case-insensitive", () => { + expect( + coAuthors("Subject\n\nCO-AUTHORED-BY: Bob "), + ).toEqual([{ name: "Bob", email: "bob@example.com" }]); + }); + + test("ignores a trailer that is not in the last paragraph", () => { + expect( + coAuthors( + "Subject\n\nCo-authored-by: Bob \n\nThat line was quoted, not a trailer.", + ), + ).toEqual([]); + }); + + test("keeps the last paragraph rule for CRLF messages", () => { + expect( + coAuthors( + "Subject\r\n\r\nCo-authored-by: Bob \r\n\r\nThat line was quoted, not a trailer.\r\n", + ), + ).toEqual([]); + }); + + test("reads a trailer that carries no name", () => { + expect(coAuthors("Subject\n\nCo-authored-by: ")).toEqual([ + { name: "", email: "bob@example.com" }, + ]); + }); + + test("ignores malformed lines", () => { + expect( + coAuthors( + "Subject\n\nCo-authored-by: Bob\nCo-authored-by:\nNot-a-trailer: x ", + ), + ).toEqual([]); + }); + + test("returns nothing when there are no trailers", () => { + expect(coAuthors("Subject\n\nJust a body.")).toEqual([]); + }); +});