diff --git a/.prettierignore b/.prettierignore index 9b587954a..d442ca2d1 100644 --- a/.prettierignore +++ b/.prettierignore @@ -2,3 +2,4 @@ source/danger-outgoing-process-schema.json source/danger-incoming-process-schema.json source/platforms/_tests/fixtures/bbs-dsl-input.json source/platforms/_tests/fixtures/bbc-dsl-input.json +CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md index f40d760c0..307ae0463 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ +- Enhancement(perf): [Github] Check if the filesystem can load files instead of always using Github API [#991](https://github.com/danger/danger-js/pull/991) [@orta] - Fix: [Github] Multiple Inline Comments on the same file/line should all be posted [#1176](https://github.com/danger/danger-js/pull/1176) [@Rouby] @@ -834,7 +835,7 @@ Also, `danger pr` now accepts a `--process` arg. ## 3.5.0 - 3.5.1 - Fixed a bug where Danger posts empty main comment when it have one or more inline comments to post [@codestergit] -- fix bug when commiting .png files on BitBucket [@Mifi] +- fix bug when committing .png files on BitBucket [@Mifi] - Adds support for inline comments for bitbucket server. [@codestergit] ## 3.4.7 diff --git a/source/platforms/git/localGetHeadSHA.ts b/source/platforms/git/localGetHeadSHA.ts new file mode 100644 index 000000000..f17942f10 --- /dev/null +++ b/source/platforms/git/localGetHeadSHA.ts @@ -0,0 +1,20 @@ +import { debug } from "../../debug" +import { exec } from "child_process" + +const d = debug("localGetHeadSHA") + +export const localGetHeadSHA = () => + new Promise(done => { + const call = `git rev-parse HEAD` + d(call) + + exec(call, (err, stdout, _stderr) => { + if (err) { + console.error(`Could not get the git HEAD for the current path`) + console.error(err) + return + } + + done(stdout.trim()) + }) + }) diff --git a/source/platforms/github/GitHubAPI.ts b/source/platforms/github/GitHubAPI.ts index 485fcf7fe..abf30a823 100644 --- a/source/platforms/github/GitHubAPI.ts +++ b/source/platforms/github/GitHubAPI.ts @@ -3,6 +3,7 @@ import { debug } from "../../debug" import * as node_fetch from "node-fetch" import parse from "parse-link-header" import pLimit from "p-limit" +import { existsSync, readFileSync } from "fs" import { GitHubPRDSL, GitHubIssueComment, GitHubUser } from "../../dsl/GitHubDSL" @@ -10,6 +11,7 @@ import { dangerIDToString } from "../../runner/templates/githubIssueTemplate" import { api as fetch } from "../../api/fetch" import { RepoMetaData } from "../../dsl/BitBucketServerDSL" import { CheckOptions } from "./comms/checks/resultsToCheck" +import { localGetHeadSHA } from "../git/localGetHeadSHA" // The Handle the API specific parts of the github @@ -28,6 +30,7 @@ export class GitHubAPI { private readonly d = debug("GitHubAPI") private pr: GitHubPRDSL | undefined + private gitSha: string | undefined constructor(public readonly repoMetadata: RepoMetaData, public readonly token?: APIToken) { // This allows Peril to DI in a new Fetch function @@ -75,6 +78,19 @@ export class GitHubAPI { ref = prJSON.head.ref } + // Only make the check the first time file contents are requested + if (!this.gitSha) { + this.gitSha = await localGetHeadSHA() + } + + // See if we can short-cut the API request with a FS lookup + // when we're sure it's on the same setup + if (ref === this.gitSha) { + if (existsSync(path)) { + return readFileSync(path, "utf8") + } + } + const data = await this.getFileContents(path, repoSlug, ref) const buffer = new Buffer(data.content, "base64") return buffer.toString() diff --git a/source/platforms/github/getStringDiff.test.ts b/source/platforms/github/getStringDiff.test.ts new file mode 100644 index 000000000..030d662c8 --- /dev/null +++ b/source/platforms/github/getStringDiff.test.ts @@ -0,0 +1,98 @@ +function rangesOfDifferBetweenTwoStrings(source: string, target: string) { + const ranges = [] as { start: number; length: number }[] + + const addToIndex = (index: number) => { + const closestIndex = ranges[ranges.length - 1] + if (closestIndex) { + const doesAddToIndex = closestIndex.start + closestIndex.length === index - 1 + if (doesAddToIndex) { + closestIndex.length = closestIndex.length + 1 + } else { + ranges.push({ start: index - 1, length: 1 }) + } + } else { + ranges.push({ start: index - 1, length: 1 }) + } + } + + for (let index = 0; index < source.length; index++) { + const srcChar = source[index] + const targetChar = target[index] + if (srcChar !== targetChar) { + addToIndex(index) + } + } + + return ranges +} + +function highlightDifferenceBetweenInStrings(source: string, target: string) { + const ranges = rangesOfDifferBetweenTwoStrings(source, target) + let emTarget = target + ranges.forEach((range, index) => { + const lhs = `\e[3m` + const rhs = `\e[0m` + const additionalOffset = index * lhs.length + index * rhs.length + const before = emTarget.slice(0, range.start + 1 + additionalOffset) + const between = emTarget.slice( + range.start + 1 + additionalOffset, + range.start + range.length + 1 + additionalOffset + ) + const after = emTarget.slice(range.start + range.length + 1 + additionalOffset, emTarget.length) + emTarget = before + lhs + between + rhs + after + }) + return emTarget +} + +it("sees a difference", () => { + const src = "Hello world" + const target = "Hello w0rld" + expect(highlightDifferenceBetweenInStrings(src, target)).toMatchInlineSnapshot(`"Hello we[3m0e[0mrld"`) + + const src2 = "Hello world" + const target2 = "H3llo w0rld" + expect(highlightDifferenceBetweenInStrings(src2, target2)).toMatchInlineSnapshot(`"He[3m3e[0mllo we[3m0e[0mrld"`) + + const src3 = "Hello world ok then" + const target3 = "H3llo w0rld no then" + expect(highlightDifferenceBetweenInStrings(src3, target3)).toMatchInlineSnapshot( + `"He[3m3e[0mllo we[3m0e[0mrld e[3mnoe[0m then"` + ) + + // expect(rangesOfDifferBetweenTwoStrings(src, target)).toMatchInlineSnapshot(` + // Array [ + // Object { + // "length": 1, + // "start": 6, + // }, + // ] + // `) + + // const multilineInput = ` + // ↓ + // (↓ + // ····
↓ + // ······text↓ + // ····
↓ + // ) + // ` + // const multilineOutput = ` + // ↓ + // (↓ + // ····
↓ + // ········text↓ + // ······
↓ + // )` + // expect(rangesOfDifferBetweenTwoStrings(multilineInput, multilineOutput)).toMatchInlineSnapshot(` + // Array [ + // Object { + // "length": 8, + // "start": 22, + // }, + // Object { + // "length": 10, + // "start": 32, + // }, + // ] + // `) +})