Merge pull request #1168 from chris-reeves/additional-shfmt-options

Additional shfmt options
bash-lsp · Jun 2, 2024 · fe93389 · fe93389
2 parents 04a2cb3 + 3a5ac07
commit fe93389
Show file tree

Hide file tree

Showing 9 changed files with 239 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/,
     )
   })
 
@@ -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,18 +549,24 @@ describe('formatter', () => {
       fi
 
       echo binary \\
-        && echo next line
+                 && 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
       }
       ",
@@ -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: <standard input>:10:1: > must be followed by a word/,
+    )
+  })
 })