Skip to content

Handle non-zero exit status when formatting using shfmt #1163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions server/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Bash Language Server

## 5.3.2

- Handle non-zero exit status when formatting using shfmt https://github.com/bash-lsp/bash-language-server/pull/1163

## 5.3.1

- Clear diagnostics when closing document https://github.com/bash-lsp/bash-language-server/pull/1135
Expand Down
2 changes: 1 addition & 1 deletion server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"description": "A language server for Bash",
"author": "Mads Hartmann",
"license": "MIT",
"version": "5.3.1",
"version": "5.3.2",
"main": "./out/server.js",
"typings": "./out/server.d.ts",
"bin": {
Expand Down
10 changes: 9 additions & 1 deletion server/src/shfmt/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('formatter', () => {
expect(new Formatter({ executablePath: 'foo' }).canFormat).toBe(true)
})

it('should set canFormat to false when formatting fails', async () => {
it('should set canFormat to false when the executable cannot be found', async () => {
const [result, formatter] = await getFormattingResult({
document: textToDoc(''),
executablePath: 'foo',
Expand All @@ -54,6 +54,14 @@ describe('formatter', () => {
)
})

it('should throw when formatting fails', async () => {
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',
)
})

it('should format when shfmt is present', async () => {
const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT })
expect(result).toMatchInlineSnapshot(`
Expand Down
10 changes: 5 additions & 5 deletions server/src/shfmt/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,10 @@ export class Formatter {
proc.stdin.end(documentText)
})

// NOTE: do we care about exit code? 0 means "ok", 1 possibly means "errors",
// but the presence of parseable errors in the output is also sufficient to
// distinguish.
let exit
try {
exit = await proc
} catch (e) {
// TODO: we could do this up front?
if ((e as any).code === 'ENOENT') {
// shfmt path wasn't found, don't try to format any more:
logger.warn(
Expand All @@ -107,7 +103,11 @@ export class Formatter {
this._canFormat = false
return ''
}
throw new Error(`Shfmt: failed with code ${exit}: ${e}\nout:\n${out}\nerr:\n${err}`)
throw new Error(`Shfmt: child process error: ${e}`)
}

if (exit != 0) {
throw new Error(`Shfmt: exited with status ${exit}: ${err}`)
}

return out
Expand Down
Loading