Skip to content

Commit

Permalink
Pass --filename to shfmt when available
Browse files Browse the repository at this point in the history
This will aid in language dialect detection.
  • Loading branch information
chris-reeves committed Jun 1, 2024
1 parent 2d81228 commit 773a61c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 6 deletions.
20 changes: 19 additions & 1 deletion server/src/shfmt/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('formatter', () => {
expect(async () => {
await getFormattingResult({ document: FIXTURE_DOCUMENT.PARSE_PROBLEMS })
}).rejects.toThrow(
'Shfmt: exited with status 1: <standard input>: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/,
)
})

Expand Down Expand Up @@ -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: <standard input>:10:1: > must be followed by a word/,
)
})
})
14 changes: 9 additions & 5 deletions server/src/shfmt/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ export class Formatter {
formatOptions?: LSP.FormattingOptions | null,
shfmtConfig?: Record<string, string | boolean> | null,
): Promise<TextEdit[]> {
const documentText = document.getText()

const result = await this.runShfmt(documentText, formatOptions, shfmtConfig)
const result = await this.runShfmt(document, formatOptions, shfmtConfig)

if (!this._canFormat) {
return []
Expand All @@ -61,7 +59,7 @@ export class Formatter {
}

private async runShfmt(
documentText: string,
document: TextDocument,
formatOptions?: LSP.FormattingOptions | null,
shfmtConfig?: Record<string, string | boolean> | null,
): Promise<string> {
Expand All @@ -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 = ''
Expand All @@ -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
Expand Down

0 comments on commit 773a61c

Please sign in to comment.