From 773a61c211edc716c2888989e291d2ae7f712640 Mon Sep 17 00:00:00 2001 From: Chris Reeves Date: Sat, 1 Jun 2024 01:21:57 +0100 Subject: [PATCH] Pass --filename to shfmt when available This will aid in language dialect detection. --- server/src/shfmt/__tests__/index.test.ts | 20 +++++++++++++++++++- server/src/shfmt/index.ts | 14 +++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/server/src/shfmt/__tests__/index.test.ts b/server/src/shfmt/__tests__/index.test.ts index 0c05e5d1..481120a8 100644 --- a/server/src/shfmt/__tests__/index.test.ts +++ b/server/src/shfmt/__tests__/index.test.ts @@ -58,7 +58,7 @@ describe('formatter', () => { expect(async () => { await getFormattingResult({ document: FIXTURE_DOCUMENT.PARSE_PROBLEMS }) }).rejects.toThrow( - 'Shfmt: exited with status 1: :10:1: > must be followed by a word', + /Shfmt: exited with status 1: .*\/testing\/fixtures\/parse-problems.sh:10:1: > must be followed by a word/, ) }) @@ -584,4 +584,22 @@ describe('formatter', () => { ] `) }) + + it('should omit filename from the shfmt comment when it cannot be determined', async () => { + // There's no easy way to see what filename has been passed to shfmt without inspecting the + // contents of the logs. As a workaround, we set a non-file:// URI on a dodgy document to + // trigger an exception and inspect the error message. + const testDocument = TextDocument.create( + 'http://localhost/', + 'shellscript', + 0, + FIXTURE_DOCUMENT.PARSE_PROBLEMS.getText(), + ) + + expect(async () => { + await getFormattingResult({ document: testDocument }) + }).rejects.toThrow( + /Shfmt: exited with status 1: :10:1: > must be followed by a word/, + ) + }) }) diff --git a/server/src/shfmt/index.ts b/server/src/shfmt/index.ts index 00304d18..da3ea0a3 100644 --- a/server/src/shfmt/index.ts +++ b/server/src/shfmt/index.ts @@ -41,9 +41,7 @@ export class Formatter { formatOptions?: LSP.FormattingOptions | null, shfmtConfig?: Record | null, ): Promise { - const documentText = document.getText() - - const result = await this.runShfmt(documentText, formatOptions, shfmtConfig) + const result = await this.runShfmt(document, formatOptions, shfmtConfig) if (!this._canFormat) { return [] @@ -61,7 +59,7 @@ export class Formatter { } private async runShfmt( - documentText: string, + document: TextDocument, formatOptions?: LSP.FormattingOptions | null, shfmtConfig?: Record | null, ): Promise { @@ -74,6 +72,12 @@ export class Formatter { if (shfmtConfig?.simplifyCode) args.push('-s') // --simplify if (shfmtConfig?.spaceRedirects) args.push('-sr') // --space-redirects + // If we can determine a local filename, pass that to shfmt to aid language dialect detection + const filePathMatch = document.uri.match(/^file:\/\/(.*)$/) + if (filePathMatch) { + args.push(`--filename=${filePathMatch[1]}`) + } + logger.debug(`Shfmt: running "${this.executablePath} ${args.join(' ')}"`) let out = '' @@ -90,7 +94,7 @@ export class Formatter { // This is solved in Node >= 15.1 by the "on('spawn', ...)" event, but we need to // support earlier versions. }) - proc.stdin.end(documentText) + proc.stdin.end(document.getText()) }) let exit