Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion source/features/clear-pr-merge-commit-message.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import features from '../feature-manager.js';
import getDefaultBranch from '../github-helpers/get-default-branch.js';
import {userHasPushAccess} from '../github-helpers/get-user-permission.js';
import {expectToken} from '../github-helpers/github-token.js';
import {getConversationAuthor} from '../github-helpers/index.js';
import {getBranches} from '../github-helpers/pr-branches.js';
import attachElement from '../helpers/attach-element.js';
import cleanCommitMessage from '../helpers/clean-commit-message.js';
Expand All @@ -15,7 +16,8 @@ const isPrAgainstDefaultBranch = async (): Promise<boolean> => getBranches().bas

async function clear(messageField: HTMLTextAreaElement): Promise<void> {
const originalMessage = messageField.value;
let cleanedMessage = cleanCommitMessage(originalMessage, !await isPrAgainstDefaultBranch());
const author = getConversationAuthor();
let cleanedMessage = cleanCommitMessage(originalMessage, !await isPrAgainstDefaultBranch(), [author]);

if (cleanedMessage === originalMessage.trim()) {
return;
Expand Down
10 changes: 9 additions & 1 deletion source/github-helpers/get-comment-author.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import {$closest, $closestOptional} from 'select-dom';

import {is} from '../helpers/css-selectors.js';

/**
Given any element in a comment, returns the comment’s author

Expand Down Expand Up @@ -37,7 +40,12 @@ export default function getCommentAuthor(anyElementInsideComment: Element): stri
.alt // Occasionally ends with `[bot]`
.replace(/^@/, ''); // May or may not be present

if (!name.endsWith('[bot]') && $closestOptional('[href^="https://github.com/apps/"]', avatar)) {
const appLink = $closestOptional('a' + is(
'[href^="/apps/"]',
'[href^="https://github.com/apps/"]',
), avatar);

if (!name.endsWith('[bot]') && appLink) {
// Example: https://github.com/webpack/webpack/pull/15926#issuecomment-1170670173
return name + '[bot]';
}
Expand Down
15 changes: 10 additions & 5 deletions source/github-helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import compareVersions from 'tiny-version-compare';
import type {RequireAtLeastOne} from 'type-fest';

import {is} from '../helpers/css-selectors.js';
import getCommentAuthor from './get-comment-author.js';
import {branchSelector} from './selectors.js';

// Re-export for convenience
Expand Down Expand Up @@ -210,11 +211,15 @@ export function scrollIntoViewIfNeeded(element: Element): void {
(element.scrollIntoViewIfNeeded ?? element.scrollIntoView).call(element);
}

function getConversationAuthor(): string | undefined {
return $optional([
'.js-command-palette-pull-body .author', // PR conversation
'[data-testid="issue-body-header-author"]', // Issue conversation
])?.textContent;
Comment thread
fregante marked this conversation as resolved.
export function getConversationBody(): Element {
return $([
'.react-issue-body', // Issues
'.js-command-palette-pull-body', // PRs
Comment thread
fregante marked this conversation as resolved.
]);
}

export function getConversationAuthor(): string {
return getCommentAuthor(getConversationBody());
}

export function isOwnConversation(): boolean {
Expand Down
64 changes: 64 additions & 0 deletions source/helpers/clean-commit-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,70 @@ test('cleanCommitMessage', () => {
'Should preserve both co-authored-by and signed-off-by',
);

assert.isEmpty(
cleanCommitMessage(
`
Some stuff happened
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
`,
false,
['dependabot[bot]'],
),
'Should drop co-author whose privacy email matches the PR author',
);

assert.equal(
cleanCommitMessage(
`
Some stuff happened
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
${coauthors[2]}
`,
false,
['dependabot[bot]'],
),
coauthors[2],
'Should drop only the matching co-author, and keep others',
);

assert.equal(
cleanCommitMessage(
`
Some stuff happened
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
`,
false,
['someone-else'],
),
'Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>',
'Should keep co-author when privacy email username does not match PR author',
);

assert.equal(
cleanCommitMessage(
`
Some stuff happened
${coauthors[0]}
`,
false,
['Me'],
),
coauthors[0],
'Should keep co-author when email is not a GitHub privacy email, even if the name matches PR author',
);

assert.isEmpty(
cleanCommitMessage(
`
Some stuff happened
Co-authored-by: someuser <someuser@users.noreply.github.com>
`,
false,
['someuser'],
),
'Should drop co-author with legacy privacy email without numeric prefix prior to July 18, 2017',
);
Comment thread
fregante marked this conversation as resolved.

assert.isEmpty(
cleanCommitMessage(`
Fixes #1345
Expand Down
14 changes: 12 additions & 2 deletions source/helpers/clean-commit-message.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
export default function cleanCommitMessage(message: string, closingKeywords = false): string {
function parseUserFromEmail(author: string): string | undefined {
return /<(?:\d+\+)?([^@>]+)@users\.noreply\.github\.com>/i.exec(author)?.[1];
}

export default function cleanCommitMessage(message: string, closingKeywords = false, excludeUsers: string[] = []): string {
const preservedContent = new Set();

// This method ensures that "Co-authored-by" capitalization doesn't affect deduplication
// This method ensures that "Co-authored-by" capitalization doesn't affect deduplication.
// Also drops co-authors whose GitHub privacy email username is in `excludeUsers`.
for (const [, author] of message.matchAll(/co-authored-by: ([^\n]+)/gi)) {
const username = parseUserFromEmail(author);
if (username && excludeUsers.includes(username)) {
continue;
}

preservedContent.add('Co-authored-by: ' + author);
}

Expand Down
Loading