Skip to content

Commit aba7bc5

Browse files
check-links: review comment lists every finding on the line, naming the ones the suggestion leaves
Amp-Thread-ID: https://ampcode.com/threads/T-01a08a01-44c1-775b-84d0-d67ff9501905 Co-authored-by: Amp <amp@ampcode.com>
1 parent c17d30e commit aba7bc5

1 file changed

Lines changed: 25 additions & 19 deletions

File tree

‎dev/check-links.mjs‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,8 @@ function extractLinks(content, filePath) {
276276
// Absolute links to this site, in every form the docs have used: http or https,
277277
// scheme-relative, www., the legacy docs.sourcegraph.com host, or sourcegraph.com/docs.
278278
// Links pinned to an old version (/@5.1/..., /v/5.1/...) are external: the
279-
// middleware sends them to that version's own site.
279+
// middleware sends them to that version's own site (5.1.sourcegraph.com), whose
280+
// pages are not in this repo, so only --check-external can validate them.
280281
const SELF_LINK_REGEX = /^(?:https?:)?\/\/(?:www\.)?(?:docs\.sourcegraph\.com|sourcegraph\.com\/docs)(?=[/#?]|$)(?!\/@|\/v\/)/i;
281282

282283
export function isSelfLink(url) {
@@ -619,27 +620,32 @@ function formatMarkdown(findings) {
619620
}
620621

621622
// Body for POST /repos/{owner}/{repo}/pulls/{n}/reviews: one suggested change per
622-
// added line that has fixes, so the author can apply them from the PR. Review
623-
// comments must sit on a line of the diff, hence the added-line restriction.
623+
// added line that has fixes, so the author can apply them from the PR. The comment
624+
// lists every finding on the line, so the ones the suggestion cannot fix are not
625+
// mistaken for accepted. Review comments must sit on a line of the diff, hence the
626+
// added-line restriction.
624627
function reviewRequest(findings) {
625-
const fixesByLine = new Map();
628+
const findingsByLine = new Map();
626629
for (const finding of findings) {
627-
if (!finding.fix || !isAddedLine(finding.file, finding.line)) continue;
630+
if (!isAddedLine(finding.file, finding.line)) continue;
628631
const key = `${finding.file}:${finding.line}`;
629-
if (!fixesByLine.has(key)) fixesByLine.set(key, { file: finding.file, line: finding.line, fixes: [] });
630-
fixesByLine.get(key).fixes.push(finding);
631-
}
632-
633-
const comments = [...fixesByLine.values()].map(({ file, line, fixes }) => {
634-
const source = fs.readFileSync(path.join(ROOT_DIR, file), 'utf-8').split('\n')[line - 1];
635-
const fixed = fixes.reduce((text, { url, fix }) => text.split(url).join(fix), source);
636-
return {
637-
path: file,
638-
line,
639-
side: 'RIGHT',
640-
body: [...fixes.map(({ error }) => `- ${error}`), '```suggestion', fixed, '```'].join('\n')
641-
};
642-
});
632+
if (!findingsByLine.has(key)) findingsByLine.set(key, []);
633+
findingsByLine.get(key).push(finding);
634+
}
635+
636+
const comments = [...findingsByLine.values()]
637+
.filter(lineFindings => lineFindings.some(finding => finding.fix))
638+
.map(lineFindings => {
639+
const { file, line } = lineFindings[0];
640+
const source = fs.readFileSync(path.join(ROOT_DIR, file), 'utf-8').split('\n')[line - 1];
641+
const fixed = lineFindings
642+
.filter(finding => finding.fix)
643+
.reduce((text, { url, fix }) => text.split(url).join(fix), source);
644+
const notes = lineFindings.map(
645+
({ url, error, fix }) => `- \`${url}\`: ${error}${fix ? '' : ' (not fixed by this suggestion)'}`
646+
);
647+
return { path: file, line, side: 'RIGHT', body: [...notes, '```suggestion', fixed, '```'].join('\n') };
648+
});
643649
return {
644650
event: 'COMMENT',
645651
body: 'Suggested fixes for the links this PR adds; details in the check-links comment.',

0 commit comments

Comments
 (0)