From 7f6ffc878699a7b7459a4ece01536d09b2662621 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 21 Dec 2018 22:42:42 +0100 Subject: [PATCH 1/3] Adjust formatting rules Apparently, VS Code's internal formatting interferes with TSLint's (resulting in the formatting seemingly reverting and then re-reverting within half a second upon save). So let's disable VS Code's. While at it, also define the alignment rules. Signed-off-by: Johannes Schindelin --- .vscode/settings.json | 2 +- tslint.json | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 41ce6f32d0..d6558a4972 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,8 +1,8 @@ { - "editor.formatOnSave": true, "tslint.autoFixOnSave": true, "[typescript]": { "editor.detectIndentation": false, + "editor.autoIndent": false, "editor.insertSpaces": true, "editor.tabSize": 4, "editor.wordWrap": "wordWrapColumn", diff --git a/tslint.json b/tslint.json index 776b57115b..998160be88 100644 --- a/tslint.json +++ b/tslint.json @@ -1,12 +1,19 @@ { "extends": "tslint:recommended", "rules": { + "align": [ + true, + "parameters", + "arguments", + "members", + "elements", + "statements" + ], "max-line-length": { "options": [ 80 ] }, - "no-floating-promises": true, "no-unused-expression": true, "no-console": false } From 7bdbeb04cc591bf5c22fa3640d243e39ba69fb60 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 21 Dec 2018 22:49:07 +0100 Subject: [PATCH 2/3] Re-format all Typescript code ... according to the new rules. This (huge) commit best viewed with --color-words. Signed-off-by: Johannes Schindelin --- lib/ci-helper.ts | 42 ++++++++--------- lib/git-notes.ts | 8 ++-- lib/git.ts | 8 ++-- lib/gitgitgadget.ts | 64 +++++++++++--------------- lib/github-glue.ts | 6 +-- lib/mail-commit-mapping.ts | 9 ++-- lib/patch-series.ts | 74 +++++++++++++++--------------- lib/project-options.ts | 29 ++++++------ script/mail-patch-series.ts | 2 +- script/misc-helper.ts | 2 +- script/monitor-upstream-commits.ts | 2 +- tests/ci-helper.test.ts | 4 +- tests/gitgitgadget.test.ts | 45 ++++++++++-------- tests/patch-series.test.ts | 16 ++++--- tests/project-options.test.ts | 5 +- tests/test-lib.ts | 19 ++++---- 16 files changed, 163 insertions(+), 172 deletions(-) diff --git a/lib/ci-helper.ts b/lib/ci-helper.ts index 44cfceef82..8175faee97 100644 --- a/lib/ci-helper.ts +++ b/lib/ci-helper.ts @@ -65,7 +65,7 @@ export class CIHelper { await this.maybeUpdateMail2CommitMap(); if (!this.commit2mailNotes) { this.commit2mailNotes = new GitNotes(this.mail2commit.workDir, - "refs/notes/commit-to-mail"); + "refs/notes/commit-to-mail"); await this.commit2mailNotes.update(); } const messageId = await @@ -74,7 +74,7 @@ export class CIHelper { return undefined; } await this.mail2commit.mail2CommitNotes.setString(messageId, - gitGitCommit, true); + gitGitCommit, true); await this.commit2mailNotes.appendCommitNote(gitGitCommit, messageId); } @@ -112,7 +112,7 @@ export class CIHelper { if (!this.testing && mailMeta.pullRequestURL) { await this.github.annotateCommit(mailMeta.originalCommit, - upstreamCommit); + upstreamCommit); } return true; @@ -231,12 +231,9 @@ export class CIHelper { // Identify branch in gitster/git let gitsterBranch: string | undefined = - await git([ - "for-each-ref", - `--points-at=${tipCommitInGitGit}`, - "--format=%(refname)", - "refs/remotes/gitster/", - ], { workDir: this.workDir }); + await git(["for-each-ref", `--points-at=${tipCommitInGitGit}`, + "--format=%(refname)", "refs/remotes/gitster/"], + { workDir: this.workDir }); if (gitsterBranch) { const newline = gitsterBranch.indexOf("\n"); if (newline > 0) { @@ -348,12 +345,10 @@ export class CIHelper { public async identifyMergeCommit(upstreamBranch: string, integratedCommit: string): Promise { - const revs = await git([ - "rev-list", - "--ancestry-path", - "--parents", - `${integratedCommit}..upstream/${upstreamBranch}`, - ], { workDir: this.workDir }); + const revs = + await git(["rev-list", "--ancestry-path", "--parents", + `${integratedCommit}..upstream/${upstreamBranch}`], + { workDir: this.workDir }); if (revs === "") { return undefined; } @@ -431,12 +426,9 @@ export class CIHelper { workDir: this.workDir, }); } - const revs = await git([ - "rev-list", - `${prMeta.baseCommit}..${prMeta.headCommit}`, - ], { - workDir: this.workDir, - }); + const revs = await git(["rev-list", + `${prMeta.baseCommit}..${prMeta.headCommit}`], + { workDir: this.workDir }); return revs.split(/\s+/); } @@ -507,9 +499,11 @@ export class CIHelper { comment.author}`); } - const coverMid = await gitGitGadget.submit(comment.author, - fullAuthorName, pullRequestURL, description, - pr.baseLabel, pr.baseCommit, pr.headLabel, pr.headCommit); + const coverMid = + await gitGitGadget.submit(comment.author, fullAuthorName, + pullRequestURL, description, + pr.baseLabel, pr.baseCommit, + pr.headLabel, pr.headCommit); await addComment(`Submitted as [${ coverMid}](https://public-inbox.org/git/${coverMid})`); } else if (command === "/allow") { diff --git a/lib/git-notes.ts b/lib/git-notes.ts index 0704701ff2..17bc9b547e 100644 --- a/lib/git-notes.ts +++ b/lib/git-notes.ts @@ -79,11 +79,9 @@ export class GitNotes { if (this.notesRef === "refs/notes/gitgitgadget" || this.notesRef === "refs/notes/commit-to-mail" || this.notesRef === "refs/notes/mail-to-commit") { - await git([ - "fetch", - "https://github.com/gitgitgadget/git", - `+${this.notesRef}:${this.notesRef}`, - ], { workDir: this.workDir }); + await git(["fetch", "https://github.com/gitgitgadget/git", + `+${this.notesRef}:${this.notesRef}`], + { workDir: this.workDir }); } else { throw new Error(`Don't know how to update ${this.notesRef}`); } diff --git a/lib/git.ts b/lib/git.ts index bb92d44dd1..76c5b28aa7 100644 --- a/lib/git.ts +++ b/lib/git.ts @@ -51,9 +51,9 @@ stdout: ${result.stdout} * @returns { string | undefined } the full SHA-1, or undefined */ export async function revParse(argument: string, workDir?: string) { - const result = await GitProcess.exec([ - "rev-parse", "--verify", "-q", argument, - ], workDir || "."); + const result = await GitProcess.exec(["rev-parse", "--verify", "-q", + argument], + workDir || "."); return result.exitCode ? undefined : trimTrailingNewline(result.stdout); } @@ -83,7 +83,7 @@ export async function gitConfigForEach(key: string, workDir?: string): Promise { const result = await GitProcess.exec(["config", "--get-all", key], - workDir || "."); + workDir || "."); result.stdout.split(/\r?\n/).map(callbackfn); } diff --git a/lib/gitgitgadget.ts b/lib/gitgitgadget.ts index f89207e766..b4eadf3bf6 100644 --- a/lib/gitgitgadget.ts +++ b/lib/gitgitgadget.ts @@ -46,31 +46,27 @@ export class GitGitGadget { } // Always fetch the Git notes first thing - await git([ - "fetch", - publishTagsAndNotesToRemote, - "--", - `+${GitNotes.defaultNotesRef}:${GitNotes.defaultNotesRef}`, - ], { workDir }); + await git(["fetch", publishTagsAndNotesToRemote, "--", + `+${GitNotes.defaultNotesRef}:${GitNotes.defaultNotesRef}`], + { workDir }); const notes = new GitNotes(workDir); const smtpUser = await gitConfig("gitgitgadget.smtpUser", - gitGitGadgetDir); + gitGitGadgetDir); const smtpHost = await gitConfig("gitgitgadget.smtpHost", - gitGitGadgetDir); + gitGitGadgetDir); const smtpPass = await gitConfig("gitgitgadget.smtpPass", - gitGitGadgetDir); + gitGitGadgetDir); if (!smtpUser || !smtpHost || !smtpPass) { throw new Error(`No SMTP settings configured`); } const [options, allowedUsers] = await GitGitGadget.readOptions(notes); - return new GitGitGadget(notes, - options, allowedUsers, - smtpUser, smtpHost, smtpPass, - publishTagsAndNotesToRemote); + return new GitGitGadget(notes, options, allowedUsers, + smtpUser, smtpHost, smtpPass, + publishTagsAndNotesToRemote); } public static parsePullRequestURL(pullRequestURL: string): @@ -188,17 +184,19 @@ export class GitGitGadget { `refs/tags/${metadata.latestTag}` : undefined; await this.updateNotesAndPullRef(pullRequestNumber, previousTag); - const series = await PatchSeries.getFromNotes(this.notes, - pullRequestURL, description, baseLabel, baseCommit, headLabel, - headCommit, gitHubUserName); - - const coverMid = await series.generateAndSend(console, - async (mail: string): Promise => { - return await parseHeadersAndSendMail(mail, this.smtpOptions); - }, - this.publishTagsAndNotesToRemote, pullRequestURL, - new Date(), - ); + const series = + await PatchSeries.getFromNotes(this.notes, pullRequestURL, + description, baseLabel, baseCommit, + headLabel, headCommit, + gitHubUserName); + + const send = async (mail: string): Promise => { + return await parseHeadersAndSendMail(mail, this.smtpOptions); + }; + const coverMid = + await series.generateAndSend(console, send, + this.publishTagsAndNotesToRemote, + pullRequestURL, new Date()); return coverMid; } @@ -234,23 +232,17 @@ export class GitGitGadget { } protected async fetchAndReReadOptions(): Promise { - await git([ - "fetch", - this.publishTagsAndNotesToRemote, - "--", - `+${GitNotes.defaultNotesRef}:${GitNotes.defaultNotesRef}`, - ], { workDir: this.workDir }); + await git(["fetch", this.publishTagsAndNotesToRemote, "--", + `+${GitNotes.defaultNotesRef}:${GitNotes.defaultNotesRef}`], + { workDir: this.workDir }); [this.options, this.allowedUsers] = await GitGitGadget.readOptions(this.notes); } protected async pushNotesRef(): Promise { - await git([ - "push", - this.publishTagsAndNotesToRemote, - "--", - `${this.notes.notesRef}`, - ], { workDir: this.workDir }); + await git(["push", this.publishTagsAndNotesToRemote, "--", + `${this.notes.notesRef}`], + { workDir: this.workDir }); // re-read options [this.options, this.allowedUsers] = diff --git a/lib/github-glue.ts b/lib/github-glue.ts index 2fcfe3d3c9..18d44e5b6e 100644 --- a/lib/github-glue.ts +++ b/lib/github-glue.ts @@ -30,9 +30,9 @@ export class GitHubGlue { public async annotateCommit(originalCommit: string, gitGitCommit: string): Promise { - const output = await git([ - "show", "-s", "--format=%h %cI", gitGitCommit, - ], { workDir: this.workDir }); + const output = + await git(["show", "-s", "--format=%h %cI", gitGitCommit], + { workDir: this.workDir }); const match = output.match(/^(\S+) (\S+)$/); if (!match) { throw new Error(`Could not find ${gitGitCommit}: '${output}'`); diff --git a/lib/mail-commit-mapping.ts b/lib/mail-commit-mapping.ts index b7bfb441b1..6ef69fb4e4 100644 --- a/lib/mail-commit-mapping.ts +++ b/lib/mail-commit-mapping.ts @@ -8,7 +8,7 @@ export class MailCommitMapping { public constructor(workDir?: string) { this.workDir = workDir; this.mail2CommitNotes = new GitNotes(workDir, - "refs/notes/mail-to-commit"); + "refs/notes/mail-to-commit"); } public async getGitGitCommitForMessageId(messageID: string): @@ -40,11 +40,8 @@ export class MailCommitMapping { refs.push("+refs/heads/*:refs/remotes/gitster/*"); } if (refs.length) { - await git([ - "fetch", - "https://github.com/gitgitgadget/git", - ...refs, - ], { workDir: this.workDir }); + await git(["fetch", "https://github.com/gitgitgadget/git", ...refs], + { workDir: this.workDir }); } } } diff --git a/lib/patch-series.ts b/lib/patch-series.ts index 75880bb001..99659bbfb9 100644 --- a/lib/patch-series.ts +++ b/lib/patch-series.ts @@ -20,7 +20,7 @@ export class PatchSeries { project: ProjectOptions): Promise { const latestTag: string = await this.getLatestTag(project.branchName, - options.redo); + options.redo); const baseCommit = await revParse(project.upstreamBranch); if (!baseCommit) { @@ -108,9 +108,9 @@ export class PatchSeries { pullRequestURL, }; } else { - if (!await git([ - "rev-list", `${metadata.headCommit}...${headCommit}`, - ], { workDir })) { + if (!await git(["rev-list", + `${metadata.headCommit}...${headCommit}`], + { workDir })) { throw new Error(`${headCommit} was already submitted`); } @@ -118,9 +118,9 @@ export class PatchSeries { `${metadata.baseCommit}..${metadata.headCommit}`; const currentRange = `${baseCommit}..${headCommit}`; if (await gitCommandExists("range-diff", workDir)) { - rangeDiff = await git([ - "range-diff", "--no-color", previousRange, currentRange, - ], { workDir }); + rangeDiff = await git(["range-diff", "--no-color", + previousRange, currentRange], + { workDir }); } metadata.iteration++; @@ -152,12 +152,14 @@ export class PatchSeries { const publishToRemote = undefined; const project = await ProjectOptions.get(workDir, headCommit, cc || [], - basedOn, publishToRemote, baseCommit); + basedOn, publishToRemote, + baseCommit); const wrapCoverLetterAtColumn = 76; return new PatchSeries(notes, options, project, metadata, - rangeDiff, md2text(coverLetter, wrapCoverLetterAtColumn), - senderName); + rangeDiff, + md2text(coverLetter, wrapCoverLetterAtColumn), + senderName); } protected static parsePullRequestDescription(description: string): { @@ -466,7 +468,7 @@ export class PatchSeries { logger.log("Adding Cc: and explicit From: lines for other authors, " + "if needed"); await PatchSeries.insertCcAndFromLines(mails, thisAuthor, - this.senderName); + this.senderName); if (mails.length > 1) { if (this.coverLetter) { const match2 = mails[0].match( @@ -514,8 +516,9 @@ export class PatchSeries { logger.log("Generating tag message"); let tagMessage = await PatchSeries.generateTagMessage(mails[0], mails.length > 1, - this.project.midUrlPrefix, - this.metadata.referencesMessageIds); + this.project.midUrlPrefix, + this.metadata + .referencesMessageIds); let tagName; if (!this.metadata.pullRequestURL) { tagName = `${this.project.branchName}-v${this.metadata.iteration}`; @@ -528,7 +531,7 @@ export class PatchSeries { if (this.project.publishToRemote) { const url = await gitConfig(`remote.${this.project.publishToRemote}.url`, - this.project.workDir); + this.project.workDir); if (!url) { throw new Error(`remote ${this.project.publishToRemote } lacks URL`); @@ -536,7 +539,7 @@ export class PatchSeries { logger.log("Inserting links"); tagMessage = await PatchSeries.insertLinks(tagMessage, url, tagName, - this.project.basedOn); + this.project.basedOn); } if (this.options.dryRun) { @@ -575,7 +578,7 @@ export class PatchSeries { logger.log("Inserting footers"); if (footers.length > 0) { mails[0] = PatchSeries.insertFooters(mails[0], - mails.length > 1, footers); + mails.length > 1, footers); } logger.log("Adjusting Date headers"); @@ -604,11 +607,12 @@ export class PatchSeries { const mid = messageID[1]; const commitMatch = mail.match(/^From ([0-9a-f]{40}) /); const originalCommit = commitMatch && commitMatch[1]; - await this.notes.set(mid, { + const mailMeta = { messageID: mid, originalCommit, pullRequestURL: this.metadata.pullRequestURL, - } as IMailMetadata, true); + } as IMailMetadata; + await this.notes.set(mid, mailMeta, true); if (globalOptions && originalCommit && this.metadata.pullRequestURL) { if (!globalOptions.activeMessageIDs) { @@ -638,17 +642,14 @@ export class PatchSeries { } if (!this.options.dryRun) { - await this.notes.set(this.metadata.pullRequestURL || - this.project.branchName, this.metadata, true); + const key = this.metadata.pullRequestURL || this.project.branchName; + await this.notes.set(key, this.metadata, true); } if (!this.options.dryRun && publishTagsAndNotesToRemote) { - await git([ - "push", - publishTagsAndNotesToRemote, - this.notes.notesRef, - `refs/tags/${tagName}`, - ], { workDir: this.notes.workDir }); + await git(["push", publishTagsAndNotesToRemote, this.notes.notesRef, + `refs/tags/${tagName}`], + { workDir: this.notes.workDir }); } return this.metadata.coverLetterMessageId; @@ -657,17 +658,17 @@ export class PatchSeries { protected async generateMBox(): Promise { const commitRange = `${this.project.baseCommit}..${this.project.branchName}`; - if (!this.coverLetter && 1 < parseInt(await git(["rev-list", "--count", - commitRange], { workDir: this.project.workDir }), 10)) { + if (!this.coverLetter && + 1 < parseInt(await git(["rev-list", "--count", commitRange], + { workDir: this.project.workDir }), + 10)) { throw new Error("Branch " + this.project.branchName + " needs a description"); } - const mergeBase = await git([ - "merge-base", - this.project.baseCommit, - this.project.branchName, - ], { workDir: this.project.workDir }); + const mergeBase = await git(["merge-base", this.project.baseCommit, + this.project.branchName], + { workDir: this.project.workDir }); const args = [ "format-patch", "--thread", "--stdout", "--signature=gitgitgadget", "--add-header=Fcc: Sent", @@ -724,10 +725,9 @@ export class PatchSeries { if (this.options.redo) { tagName = "+" + tagName; } - await git(["push", this.project.publishToRemote, "+" - + this.project.branchName, tagName], { - workDir: this.project.workDir, - }, + await git(["push", this.project.publishToRemote, + `+${this.project.branchName}`, tagName], + { workDir: this.project.workDir }, ); } } diff --git a/lib/project-options.ts b/lib/project-options.ts index c31c873964..0d9922241f 100644 --- a/lib/project-options.ts +++ b/lib/project-options.ts @@ -7,7 +7,7 @@ export class ProjectOptions { public static async getBranchName(workDir: string): Promise { // Get the current branch name const ref = await git(["rev-parse", "--symbolic-full-name", "HEAD"], - { workDir }); + { workDir }); const match = ref.match(/^refs\/heads\/(.*)/); if (!match) { throw new Error("Not on a branch (" + ref + ")?"); @@ -25,10 +25,10 @@ export class ProjectOptions { await gitConfig("mail.publishtoremote", workDir); const baseBranch = await ProjectOptions.determineBaseBranch(workDir, branchName, - publishToRemote); + publishToRemote); return await ProjectOptions.get(workDir, branchName, cc, baseBranch, - publishToRemote); + publishToRemote); } public static async get(workDir: string, branchName: string, @@ -45,11 +45,11 @@ export class ProjectOptions { cc.push("Junio C Hamano "); upstreamBranch = "upstream/pu"; if (await git(["rev-list", branchName + ".." + upstreamBranch], - { workDir })) { + { workDir })) { upstreamBranch = "upstream/next"; } if (await git(["rev-list", branchName + ".." + upstreamBranch], - { workDir })) { + { workDir })) { upstreamBranch = "upstream/master"; } midUrlPrefix = "https://public-inbox.org/git/"; @@ -80,13 +80,14 @@ export class ProjectOptions { if (!baseCommit && await git(["rev-list", branchName + ".." + upstreamBranch], - { workDir })) { + { workDir })) { throw new Error("Branch " + branchName + " is not rebased to " + upstreamBranch); } return new ProjectOptions(branchName, upstreamBranch, basedOn, - publishToRemote, to, cc, midUrlPrefix, workDir, baseCommit); + publishToRemote, to, cc, midUrlPrefix, + workDir, baseCommit); } protected static async determineBaseBranch(workDir: string, @@ -109,7 +110,7 @@ export class ProjectOptions { } const commit = await git(["rev-parse", "-q", "--verify", remoteRef], - { workDir }); + { workDir }); if (await git(["rev-parse", basedOn]) !== commit) { throw new Error(basedOn + " on " + publishToRemote + " disagrees with local branch"); @@ -122,12 +123,12 @@ export class ProjectOptions { Promise { // Cc: from config const cc: string[] = []; - await gitConfigForEach(`branch.${branchName}.cc`, - (email) => { - if (email) { - cc.push(email); - } - }, workDir); + const forEach = (email: string) => { + if (email) { + cc.push(email); + } + }; + await gitConfigForEach(`branch.${branchName}.cc`, forEach, workDir); return cc; } diff --git a/script/mail-patch-series.ts b/script/mail-patch-series.ts index bedd5e9e53..10b3cdc09e 100644 --- a/script/mail-patch-series.ts +++ b/script/mail-patch-series.ts @@ -109,7 +109,7 @@ async function main(argv: string[]) { await git(["config", "--add", key, arg]); } else { const id = await git(["log", "-1", "--format=%an <%ae>", - "--author=" + arg]); + "--author=" + arg]); if (!id) { throw new Error("Not an email address: " + arg); } diff --git a/script/misc-helper.ts b/script/misc-helper.ts index a048336590..fa900c688f 100644 --- a/script/misc-helper.ts +++ b/script/misc-helper.ts @@ -13,7 +13,7 @@ commander.version("1.0.0") + "annotate-commit )") .description("Command-line helper for GitGitGadget") .option("-w, --work-dir [directory]", - "Use a different GitGitGadget working directory than '.'", ".") + "Use a different GitGitGadget working directory than '.'", ".") .parse(process.argv); if (commander.args.length === 0) { diff --git a/script/monitor-upstream-commits.ts b/script/monitor-upstream-commits.ts index 80e20fcbf3..f5e680a6b2 100644 --- a/script/monitor-upstream-commits.ts +++ b/script/monitor-upstream-commits.ts @@ -13,7 +13,7 @@ commander.version("1.0.0") .usage("[options]") .description("GitGitGadget's refs/notes/mail-to-commit monitor") .option("-w, --work-dir [directory]", - "Use a different working directory than '.'", ".") + "Use a different working directory than '.'", ".") .parse(process.argv); if (commander.args.length === 0) { diff --git a/tests/ci-helper.test.ts b/tests/ci-helper.test.ts index 067941e220..97467b8020 100644 --- a/tests/ci-helper.test.ts +++ b/tests/ci-helper.test.ts @@ -44,7 +44,7 @@ test("identify upstream commit", async () => { // re-route the URLs await worktree.git(["config", `url.${gggRemote.workDir}.insteadOf`, - "https://github.com/gitgitgadget/git"]); + "https://github.com/gitgitgadget/git"]); // Set up fake upstream branches const A = await gggRemote.commit("A"); @@ -76,7 +76,7 @@ test("identify upstream commit", async () => { // Update the `mail-to-commit` notes ref, at least the part we care about const mail2CommitNotes = new GitNotes(gggRemote.workDir, - "refs/notes/mail-to-commit"); + "refs/notes/mail-to-commit"); await mail2CommitNotes.setString(messageID, B); // "publish" the gitgitgadget notes diff --git a/tests/gitgitgadget.test.ts b/tests/gitgitgadget.test.ts index 393b771221..eba9dea347 100644 --- a/tests/gitgitgadget.test.ts +++ b/tests/gitgitgadget.test.ts @@ -54,7 +54,8 @@ Fetch-It-Via: git fetch https://github.com/gitgitgadget/git ${ Pull-Request: https://github.com/gitgitgadget/git/pull/1 --${" "} gitgitgadget -`, `From cd048a1378e3f7b055cd467ff3a24ed0cf5e7453 Mon Sep 17 00:00:00 2001 +`, + `From cd048a1378e3f7b055cd467ff3a24ed0cf5e7453 Mon Sep 17 00:00:00 2001 Message-Id: > In-Reply-To: > References: > @@ -87,7 +88,8 @@ index 0000000..8c7e5a6 --${" "} gitgitgadget -`, `From b8acfa2635f9907e472d2b7396b260c6e73b1ed5 Mon Sep 17 00:00:00 2001 +`, + `From b8acfa2635f9907e472d2b7396b260c6e73b1ed5 Mon Sep 17 00:00:00 2001 Message-Id: > In-Reply-To: > References: > @@ -120,7 +122,8 @@ index 0000000..7371f47 --${" "} gitgitgadget -`, `From 07f68c195159518c5777ca4a7c1d07124e7a9956 Mon Sep 17 00:00:00 2001 +`, + `From 07f68c195159518c5777ca4a7c1d07124e7a9956 Mon Sep 17 00:00:00 2001 Message-Id: <07f68c195159518c5777ca4a7c1d07124e7a9956.> In-Reply-To: > References: > @@ -204,12 +207,13 @@ Cc: Some Body await git(["config", "user.name", "GitGitGadget"], repo.options); await git(["config", "user.email", "gitgitgadget@example.com"], - repo.options); + repo.options); - const patches = await PatchSeries.getFromNotes(notes, pullRequestURL, - description, - "gitgitgadget:next", baseCommit, - "somebody:master", headCommit, "GitHub User"); + const patches = + await PatchSeries.getFromNotes(notes, pullRequestURL, description, + "gitgitgadget:next", baseCommit, + "somebody:master", headCommit, + "GitHub User"); expect(patches.coverLetter).toEqual(`My first Pull Request! @@ -217,8 +221,8 @@ This Pull Request contains some really important changes that I would love to have included in git.git [https://github.com/git/git].`); const mails = []; - const midRegex = new RegExp("<(pull|[0-9a-f]{40})" - + "\\.\\d+(\\.v\\d+)?\\.git\\.gitgitgadget@example\\.com>", "g"); + const midRegex = + /<(pull|[0-9a-f]{40})\.\d+(\.v\d+)?\.git\.gitgitgadget@example\.com>/g; async function send(mail: string): Promise { if (mails.length === 0) { mail = mail.replace(/(\nDate: ).*/, "$1"); @@ -228,20 +232,21 @@ to have included in git.git [https://github.com/git/git].`); return "Message-ID"; } expect(await patches.generateAndSend(logger, send, undefined, - pullRequestURL)) + pullRequestURL)) .toEqual("pull.1.git.gitgitgadget@example.com"); expect(mails).toEqual(expectedMails); expect(await repo.commit("D")).not.toEqual(""); const headCommit2 = await repo.revParse("HEAD"); - const patches2 = await PatchSeries.getFromNotes(notes, pullRequestURL, - description, - "gitgitgadget:next", baseCommit, - "somebody:master", headCommit2, "GitHub User"); + const patches2 = + await PatchSeries.getFromNotes(notes, pullRequestURL, description, + "gitgitgadget:next", baseCommit, + "somebody:master", headCommit2, + "GitHub User"); mails.splice(0); expect(await patches2.generateAndSend(logger, send, undefined, - pullRequestURL)) + pullRequestURL)) .toEqual("pull.1.v2.git.gitgitgadget@example.com"); expect(mails.length).toEqual(5); if (await gitCommandExists("range-diff", repo.workDir)) { @@ -265,7 +270,7 @@ to have included in git.git [https://github.com/git/git].`); // verify that the tag was generated correctly expect((await git(["cat-file", "tag", "pr-1/somebody/master-v2"], - repo.options)) + repo.options)) .replace(/^[^]*?\n\n/, "")).toEqual(`My first Pull Request! This Pull Request contains some really important changes that I would love @@ -306,7 +311,8 @@ test("allow/disallow", async () => { await git(["config", "gitgitgadget.workDir", workDir], { workDir }); await git(["config", - "gitgitgadget.publishRemote", remote.workDir], { workDir }); + "gitgitgadget.publishRemote", remote.workDir], + { workDir }); await git(["config", "gitgitgadget.smtpUser", "test"], { workDir }); await git(["config", "gitgitgadget.smtpHost", "test"], { workDir }); await git(["config", "gitgitgadget.smtpPass", "test"], { workDir }); @@ -318,7 +324,8 @@ test("allow/disallow", async () => { // pretend that the notes ref had been changed in the meantime await notes.set("", - { allowedUsers: ["first-one"] } as IGitGitGadgetOptions, true); + { allowedUsers: ["first-one"] } as IGitGitGadgetOptions, + true); expect(gitGitGadget.isUserAllowed("second-one")).toBeFalsy(); expect(await gitGitGadget.allowUser("first-one", "second-one")) diff --git a/tests/patch-series.test.ts b/tests/patch-series.test.ts index e16ed4db55..541b08c5e8 100644 --- a/tests/patch-series.test.ts +++ b/tests/patch-series.test.ts @@ -80,7 +80,7 @@ class PatchSeriesTest extends PatchSeries { }); PatchSeries.insertCcAndFromLines(mails, - "A U Thor "); + "A U Thor "); test("Cc: is inserted correctly", () => { expect(mails[1]).toMatch( // tslint:disable-next-line:max-line-length @@ -92,18 +92,20 @@ class PatchSeriesTest extends PatchSeries { expect(coverLetter).toMatch(/\nSubject: .*This is the subject/); }); - const tagMessage = PatchSeries.generateTagMessage(coverLetter, true, - "https://mid.lookup/", [ - "cover.2.git.author@example.com", - "cover.1.git.author@example.com", - ]); + const mids = [ + "cover.2.git.author@example.com", + "cover.1.git.author@example.com", + ]; + const tagMessage = + PatchSeries.generateTagMessage(coverLetter, true, + "https://mid.lookup/", mids); test("Tag message is generated correctly", () => { expect(tagMessage).toBe(tagMessage1); }); const repoUrl = "https://github.com/author/git"; const withLinks = PatchSeries.insertLinks(tagMessage, repoUrl, - "my-series-v1", "next"); + "my-series-v1", "next"); test("Links are inserted correctly", () => { const footer = ` Based-On: next at ${repoUrl} diff --git a/tests/project-options.test.ts b/tests/project-options.test.ts index 9a0dd0e860..5e9f7b8493 100644 --- a/tests/project-options.test.ts +++ b/tests/project-options.test.ts @@ -27,8 +27,9 @@ test("project options", async () => { expect(options1.basedOn).toBeUndefined(); expect(options1.to).toEqual("--to=reviewer@example.com"); expect(options1.publishToRemote).toBeUndefined(); - const options2 = await ProjectOptions.get(repo.workDir, - "test-project-options", [], undefined, undefined); + const options2 = + await ProjectOptions.get(repo.workDir, "test-project-options", [], + undefined, undefined); expect(options2.workDir).not.toBeUndefined(); expect(options2.midUrlPrefix).toEqual("https://dummy.com/?mid="); }); diff --git a/tests/test-lib.ts b/tests/test-lib.ts index f896c48951..16cc467aed 100644 --- a/tests/test-lib.ts +++ b/tests/test-lib.ts @@ -163,17 +163,16 @@ export async function testCreateRepo(name: string, suffix?: string): if (user !== "Test User") { throw new Error(`Whoops. '${user}'`); } - await git([ - "commit-tree", "-m", "Test commit", - "4b825dc642cb6eb9a060e54bf8d69288fbee4904", - ], { - env: { - GIT_AUTHOR_DATE: `123457689 +0000`, - GIT_COMMITTER_DATE: `123457689 +0000`, - }, - workDir: dir, + const opts = { + env: { + GIT_AUTHOR_DATE: `123457689 +0000`, + GIT_COMMITTER_DATE: `123457689 +0000`, }, - ); + workDir: dir, + }; + await git(["commit-tree", "-m", "Test commit", + "4b825dc642cb6eb9a060e54bf8d69288fbee4904"], + opts); const gitOpts: ITestCommitOptions = { workDir: dir }; return new TestRepo(gitOpts); From 82899227d17e8e96ff07a8deb376c9263ec19566 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 8 Oct 2018 21:23:07 +0200 Subject: [PATCH 3/3] Fix the encoding of non-ASCII names in the email headers When sending mails, the From: or Cc: headers can only contain ASCII characters. Any non-ASCII characters need to be encoded, otherwise the emails will be rejected by the Git mailing list. With this change, the test now also finally learns to test inserting Cc: lines in the GitGitGadget mode (rather than testing the non-GitGitGadget one). This fixes https://github.com/gitgitgadget/gitgitgadget/issues/29 Signed-off-by: Johannes Schindelin --- .vscode/settings.json | 5 +++++ lib/patch-series.ts | 10 +++++++--- tests/patch-series.test.ts | 14 +++++++++++--- tests/project-options.test.ts | 25 +++++++++++++++++++++++-- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index d6558a4972..483a3182c5 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -22,8 +22,12 @@ "Hamano", "ISMTP", "Junio", + "Nguy", + "Nguyễn", + "Ngọc", "Probot", "Schindelin", + "Thái", "Truthy", "VSTS", "amlog", @@ -47,6 +51,7 @@ "mergeable", "octokit", "onelines", + "pclouds", "pobox", "promisify", "publishtoremote", diff --git a/lib/patch-series.ts b/lib/patch-series.ts index 99659bbfb9..c0f2f3dae2 100644 --- a/lib/patch-series.ts +++ b/lib/patch-series.ts @@ -1,3 +1,4 @@ +import { encodeWords } from "nodemailer/lib/mime-funcs"; import { commitExists, git, gitCommandExists, gitConfig, revParse, } from "./git"; @@ -248,10 +249,11 @@ export class PatchSeries { throw new Error("No From: line found in header:\n\n" + header); } - let replaceSender = thisAuthor; + let replaceSender = encodeWords(thisAuthor); if (isGitGitGadget) { const onBehalfOf = i === 0 && senderName ? - senderName : authorMatch[2].replace(/ <.*>$/, ""); + encodeWords(senderName) : + authorMatch[2].replace(/ <.*>$/, ""); // Special-case GitGitGadget to send from // " via GitGitGadget" replaceSender = "\"" @@ -677,7 +679,9 @@ export class PatchSeries { "--add-header=MIME-Version: 1.0", "--base", mergeBase, this.project.to, ]; - this.project.cc.map((email) => { args.push("--cc=" + email); }); + this.project.cc.map((email) => { + args.push("--cc=" + encodeWords(email)); + }); if (this.metadata.referencesMessageIds) { this.metadata.referencesMessageIds.map((email) => { args.push("--in-reply-to=" + email); diff --git a/tests/patch-series.test.ts b/tests/patch-series.test.ts index 541b08c5e8..1687b22284 100644 --- a/tests/patch-series.test.ts +++ b/tests/patch-series.test.ts @@ -79,12 +79,20 @@ class PatchSeriesTest extends PatchSeries { /^From [^]*\n---\n2\.17\.0\.windows\.1\n$/); }); - PatchSeries.insertCcAndFromLines(mails, - "A U Thor "); + const thisAuthor = "GitGitGadget "; + const senderName = "Nguyễn Thái Ngọc Duy"; + PatchSeries.insertCcAndFromLines(mails, thisAuthor, senderName); + + test("non-ASCII characters are encoded correctly", () => { + // tslint:disable-next-line:max-line-length + const needle = "\"=?UTF-8?Q?Nguy=E1=BB=85n_Th=C3=A1i_Ng=E1=BB=8Dc?= Duy via GitGitGadget\" "; + expect(mails[0]).toEqual(expect.stringContaining(needle)); + }); + test("Cc: is inserted correctly", () => { expect(mails[1]).toMatch( // tslint:disable-next-line:max-line-length - /From: A U Thor[^]*\nCc: Some One Else[^]*\n\nFrom: Some One Else.*\n\n/); + /From: "Some One Else via GitGitGadget"[^]*\nCc: Some One Else[^]*\n\nFrom: Some One Else.*\n\n/); }); const coverLetter = PatchSeries.adjustCoverLetter(mails[0]); diff --git a/tests/project-options.test.ts b/tests/project-options.test.ts index 5e9f7b8493..395fc8afe4 100644 --- a/tests/project-options.test.ts +++ b/tests/project-options.test.ts @@ -1,6 +1,8 @@ import "jest"; import { isDirectory } from "../lib/fs-util"; import { git } from "../lib/git"; +import { GitNotes } from "../lib/git-notes"; +import { PatchSeries } from "../lib/patch-series"; import { IPatchSeriesMetadata } from "../lib/patch-series-metadata"; import { ProjectOptions } from "../lib/project-options"; import { @@ -28,8 +30,27 @@ test("project options", async () => { expect(options1.to).toEqual("--to=reviewer@example.com"); expect(options1.publishToRemote).toBeUndefined(); const options2 = - await ProjectOptions.get(repo.workDir, "test-project-options", [], - undefined, undefined); + await ProjectOptions.get(repo.workDir, "test-project-options", + ["Nguyễn Thái Ngọc Duy "], + undefined, undefined, "test-project-options^"); expect(options2.workDir).not.toBeUndefined(); expect(options2.midUrlPrefix).toEqual("https://dummy.com/?mid="); + + await (class X extends PatchSeries { + public static async test(): Promise { + const prMeta = { + baseCommit: options2.baseCommit, + baseLabel: options2.upstreamBranch, + headCommit: options2.branchName, + headLabel: options2.branchName, + iteration: 1, + }; + const x = new X(new GitNotes(repo.workDir), {}, + options2, prMeta, ""); + const mbox = await x.generateMBox(); + const needle = + "=?UTF-8?Q?Nguy=E1=BB=85n_Th=C3=A1i_Ng=E1=BB=8Dc?= Duy"; + expect(mbox).toEqual(expect.stringContaining(needle)); + } + }).test(); });