diff --git a/README.md b/README.md index d52e8845..6af1d277 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,10 @@ To be implemented: ### Dependencies -As a dependency, we recommend that you first install shellcheck [shellcheck][shellcheck] to enable linting: https://github.com/koalaman/shellcheck#installing . If shellcheck is installed, bash-language-server will automatically call it to provide linting and code analysis each time the file is updated (with debounce time of 500ms). +As a dependency, we recommend that you first install [shellcheck][shellcheck] to enable linting: +https://github.com/koalaman/shellcheck#installing . If `shellcheck` is installed, +bash-language-server will automatically call it to provide linting and code analysis each time the +file is updated (with debounce time of 500ms). If you want your shell scripts to be formatted consistently, you can install [shfmt][shfmt]. If `shfmt` is installed then your documents will be formatted whenever you take the 'format document' @@ -185,6 +188,13 @@ Using the built-in `eglot` lsp mode: (bash-ts-mode . eglot-ensure)) ``` +## `shfmt` integration + +The indentation used by `shfmt` is whatever has been configured for the current editor session, so +there is no `shfmt`-specific configuration variable for this. If your editor is configured for +two-space indents then that's what it will use. If you're using tabs for indentation then `shfmt` +will use that. + ## Logging The minimum logging level for the server can be adjusted using the `BASH_IDE_LOG_LEVEL` environment variable diff --git a/server/CHANGELOG.md b/server/CHANGELOG.md index 52f62060..762c3434 100644 --- a/server/CHANGELOG.md +++ b/server/CHANGELOG.md @@ -1,5 +1,9 @@ # Bash Language Server +## 5.3.4 + +- Add additonal shfmt formatting config options https://github.com/bash-lsp/bash-language-server/pull/1168 + ## 5.3.3 - Revert "Add --help fallback for documentation" https://github.com/bash-lsp/bash-language-server/pull/1052 diff --git a/server/package.json b/server/package.json index 73df323e..f7f6541b 100644 --- a/server/package.json +++ b/server/package.json @@ -3,7 +3,7 @@ "description": "A language server for Bash", "author": "Mads Hartmann", "license": "MIT", - "version": "5.3.3", + "version": "5.3.4", "main": "./out/server.js", "typings": "./out/server.d.ts", "bin": { diff --git a/server/src/__tests__/config.test.ts b/server/src/__tests__/config.test.ts index 1d02dcf0..ba927820 100644 --- a/server/src/__tests__/config.test.ts +++ b/server/src/__tests__/config.test.ts @@ -17,7 +17,9 @@ describe('ConfigSchema', () => { "binaryNextLine": false, "caseIndent": false, "funcNextLine": false, + "keepPadding": false, "path": "shfmt", + "simplifyCode": false, "spaceRedirects": false, }, } @@ -36,7 +38,9 @@ describe('ConfigSchema', () => { binaryNextLine: true, caseIndent: true, funcNextLine: true, + keepPadding: true, path: 'myshfmt', + simplifyCode: true, spaceRedirects: true, }, }), @@ -59,7 +63,9 @@ describe('ConfigSchema', () => { "binaryNextLine": true, "caseIndent": true, "funcNextLine": true, + "keepPadding": true, "path": "myshfmt", + "simplifyCode": true, "spaceRedirects": true, }, } @@ -92,7 +98,9 @@ describe('getConfigFromEnvironmentVariables', () => { "binaryNextLine": false, "caseIndent": false, "funcNextLine": false, + "keepPadding": false, "path": "shfmt", + "simplifyCode": false, "spaceRedirects": false, }, } @@ -119,7 +127,9 @@ describe('getConfigFromEnvironmentVariables', () => { "binaryNextLine": false, "caseIndent": false, "funcNextLine": false, + "keepPadding": false, "path": "", + "simplifyCode": false, "spaceRedirects": false, }, } @@ -155,7 +165,9 @@ describe('getConfigFromEnvironmentVariables', () => { "binaryNextLine": false, "caseIndent": true, "funcNextLine": false, + "keepPadding": false, "path": "/path/to/shfmt", + "simplifyCode": false, "spaceRedirects": false, }, } diff --git a/server/src/config.ts b/server/src/config.ts index 1e39903b..26584cfa 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -55,6 +55,12 @@ export const ConfigSchema = z.object({ // Place function opening braces on a separate line. funcNextLine: z.boolean().default(false), + // (Deprecated) Keep column alignment padding. + keepPadding: z.boolean().default(false), + + // Simplify code before formatting. + simplifyCode: z.boolean().default(false), + // Follow redirection operators with a space. spaceRedirects: z.boolean().default(false), }) @@ -81,6 +87,8 @@ export function getConfigFromEnvironmentVariables(): { binaryNextLine: toBoolean(process.env.SHFMT_BINARY_NEXT_LINE), caseIndent: toBoolean(process.env.SHFMT_CASE_INDENT), funcNextLine: toBoolean(process.env.SHFMT_FUNC_NEXT_LINE), + keepPadding: toBoolean(process.env.SHFMT_KEEP_PADDING), + simplifyCode: toBoolean(process.env.SHFMT_SIMPLIFY_CODE), spaceRedirects: toBoolean(process.env.SHFMT_SPACE_REDIRECTS), }, } diff --git a/server/src/shfmt/__tests__/index.test.ts b/server/src/shfmt/__tests__/index.test.ts index d29f98d0..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/, ) }) @@ -83,6 +83,12 @@ describe('formatter', () => { ;; esac + echo one two three + echo four five six + echo seven eight nine + + [[ "$simplify" == "simplify" ]] + echo space redirects >/dev/null function next() { @@ -128,6 +134,12 @@ describe('formatter', () => { ;; esac + echo one two three + echo four five six + echo seven eight nine + + [[ "$simplify" == "simplify" ]] + echo space redirects >/dev/null function next() { @@ -173,6 +185,12 @@ describe('formatter', () => { ;; esac + echo one two three + echo four five six + echo seven eight nine + + [[ "$simplify" == "simplify" ]] + echo space redirects >/dev/null function next() { @@ -219,6 +237,12 @@ describe('formatter', () => { ;; esac + echo one two three + echo four five six + echo seven eight nine + + [[ "$simplify" == "simplify" ]] + echo space redirects >/dev/null function next() { @@ -265,6 +289,12 @@ describe('formatter', () => { ;; esac + echo one two three + echo four five six + echo seven eight nine + + [[ "$simplify" == "simplify" ]] + echo space redirects >/dev/null function next() { @@ -311,6 +341,12 @@ describe('formatter', () => { ;; esac + echo one two three + echo four five six + echo seven eight nine + + [[ "$simplify" == "simplify" ]] + echo space redirects >/dev/null function next() @@ -333,6 +369,110 @@ describe('formatter', () => { `) }) + it('should format with padding kept as-is when keepPadding is true', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 2, insertSpaces: true }, + shfmtConfig: { keepPadding: true }, + }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary && + echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo one two three + echo four five six + echo seven eight nine + + [[ "$simplify" == "simplify" ]] + + echo space redirects >/dev/null + + function next() { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + + it('should format after simplifying the code when simplifyCode is true', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 2, insertSpaces: true }, + shfmtConfig: { simplifyCode: true }, + }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary && + echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo one two three + echo four five six + echo seven eight nine + + [[ $simplify == "simplify" ]] + + echo space redirects >/dev/null + + function next() { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + it('should format with redirect operators followed by a space when spaceRedirects is true', async () => { const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT, @@ -358,6 +498,12 @@ describe('formatter', () => { ;; esac + echo one two three + echo four five six + echo seven eight nine + + [[ "$simplify" == "simplify" ]] + echo space redirects > /dev/null function next() { @@ -387,6 +533,8 @@ describe('formatter', () => { binaryNextLine: true, caseIndent: true, funcNextLine: true, + keepPadding: true, + simplifyCode: true, spaceRedirects: true, }, }) @@ -401,7 +549,7 @@ describe('formatter', () => { fi echo binary \\ - && echo next line + && echo next line case "$arg" in a) @@ -409,10 +557,16 @@ describe('formatter', () => { ;; esac + echo one two three + echo four five six + echo seven eight nine + + [[ $simplify == "simplify" ]] + echo space redirects > /dev/null function next() - { + { echo line } ", @@ -430,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 e039a04d..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 { @@ -70,8 +68,16 @@ export class Formatter { if (shfmtConfig?.binaryNextLine) args.push('-bn') // --binary-next-line if (shfmtConfig?.caseIndent) args.push('-ci') // --case-indent if (shfmtConfig?.funcNextLine) args.push('-fn') // --func-next-line + if (shfmtConfig?.keepPadding) args.push('-kp') // --keep-padding + 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 = '' @@ -88,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 diff --git a/testing/fixtures/shfmt.sh b/testing/fixtures/shfmt.sh index ff14200f..0ab419a7 100644 --- a/testing/fixtures/shfmt.sh +++ b/testing/fixtures/shfmt.sh @@ -14,6 +14,12 @@ echo case indent ;; esac +echo one two three +echo four five six +echo seven eight nine + +[[ "$simplify" == "simplify" ]] + echo space redirects> /dev/null function next(){ diff --git a/vscode-client/package.json b/vscode-client/package.json index 7c164ccf..457653cc 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -98,6 +98,17 @@ "default": false, "description": "Place function opening braces on a separate line." }, + "bashIde.shfmt.keepPadding": { + "type": "boolean", + "default": false, + "description": "(Deprecated) Keep column alignment padding.", + "markdownDescription": "**([Deprecated](https://github.com/mvdan/sh/issues/658))** Keep column alignment padding." + }, + "bashIde.shfmt.simplifyCode": { + "type": "boolean", + "default": false, + "description": "Simplify code before formatting." + }, "bashIde.shfmt.spaceRedirects": { "type": "boolean", "default": false,