Fix "closing keyword" logic in clear-pr-merge-commit-message
#6328
Conversation
| const issueLink = keyword.nextElementSibling as HTMLAnchorElement; // Account for issues not in the same repo | ||
| const sibling = keyword.nextElementSibling!; | ||
| // Get the full URL so it works on issues not in the same repo | ||
| const issueLink = sibling instanceof HTMLAnchorElement ? sibling : select('a', sibling)!; |
There was a problem hiding this comment.
sibling became a span
Before
After
| @@ -19,10 +19,12 @@ async function init(): Promise<void | false> { | |||
| } | |||
|
|
|||
| // Preserve closing issues numbers when a PR is merged into a non-default branch since GitHub doesn't close them #4531 | |||
| if (getBranches().base !== await getDefaultBranch()) { | |||
| if (getBranches().base.branch !== await getDefaultBranch()) { | |||
There was a problem hiding this comment.
On cross-repo PRs, this comparison was between "user:branch" and "branch", so it would never match.
This caused #6122 to appear on every cross-repo PR instead of just non-default-branch PRs
There was a problem hiding this comment.
What about it? The review that prompted this change was #6122 (comment), which was correct. It's 4a1a31d (#6122) that introduced the bug
There was a problem hiding this comment.
Oh I though you meant something else with your comment.
Never mind
| // https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue | ||
| for (const [line] of originalMessage.matchAll(/(fix(es|ed)?|close[sd]?|resolve[sd]?)([^\n]+)/gi)) { | ||
| // Ensure it includes a reference or URL | ||
| if (/#\d+/.test(line) || line.includes('http')) { |
There was a problem hiding this comment.
This restores my original suggestion, but looser #4531 (comment)
All of this code exists to "clear the field, except some data". The DOM-based approach:
- adds data from other parts, which was not part of GitHub’s suggested message
- breaks more easily (as seen)
This regex will not cleanly keep just the keywords, but that's fine, because that content was already in the field.
Test URLs
Tested in #6315
Should also be testable in refined-github/sandbox#53
Demo
Screen.Recording.5.mov