Merge pull request #478 from marp-team/browser-path-setting

[v3] `markdown.marp.browser` and `markdown.marp.browserPath` settings
marp-team · Dec 31, 2024 · 440f3ed · 440f3ed
2 parents c1d3cab + 11e5df1
commit 440f3ed
Show file tree

Hide file tree

Showing 10 changed files with 294 additions and 112 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,16 +13,20 @@
 ### Added
 
 - `markdown.marp.html` setting to control rendering HTML within Marp Markdown ([#476](https://github.com/marp-team/marp-vscode/pull/476))
-
-### Deprecated
-
-- `markdown.marp.enableHtml` setting ([#476](https://github.com/marp-team/marp-vscode/pull/476))
+- `markdown.marp.browser` and `markdown.marp.browserPath` settings to control internally using browser to export ([#478](https://github.com/marp-team/marp-vscode/pull/478))
+- Support Firefox as a browser for exporting ([#473](https://github.com/marp-team/marp-vscode/pull/473), [#474](https://github.com/marp-team/marp-vscode/pull/474), [#478](https://github.com/marp-team/marp-vscode/pull/478))
 
 ### Changed
 
+- Several allowed HTML elements through Marp Core are enabled by default ([#472](https://github.com/marp-team/marp-vscode/pull/472), [#474](https://github.com/marp-team/marp-vscode/pull/474), [#476](https://github.com/marp-team/marp-vscode/pull/476))
 - Upgrade development Node.js and dependent packages to the latest version ([#474](https://github.com/marp-team/marp-vscode/pull/474))
 - Migrate ESLint config to flat config ([#475](https://github.com/marp-team/marp-vscode/pull/475))
 
+### Deprecated
+
+- Deprecated `markdown.marp.enableHtml` setting in favor of `markdown.marp.html` ([#476](https://github.com/marp-team/marp-vscode/pull/476))
+- Deprecated `markdown.marp.chromePath` setting in favor of `markdown.marp.browserPath` ([#478](https://github.com/marp-team/marp-vscode/pull/478))
+
 ## v2.8.0 - 2023-10-28
 
 ### Changed

diff --git a/README.md b/README.md
@@ -128,9 +128,13 @@ You can also execute command from the Command Palette (<kbd>F1</kbd> or <kbd>Ctr
 - **PNG** (_First slide only)_
 - **JPEG** (_First slide only)_
 
-Default file type can choose by `markdown.marp.exportType` preference.
+Default file type can choose by the `markdown.marp.exportType` setting.
 
-> ⚠️ Export except HTML requires to install any one of [Google Chrome](https://www.google.com/chrome/), [Chromium](https://www.chromium.org/), or [Microsoft Edge](https://www.microsoft.com/edge). You may also specify the custom path for Chrome / Chromium-based browser by preference `markdown.marp.chromePath`.
+> [!IMPORTANT]
+> Exporting PDF, PPTX, and image formats requires to install any one of [Google Chrome](https://www.google.com/chrome/), [Chromium](https://www.chromium.org/), [Microsoft Edge](https://www.microsoft.com/edge), or [Firefox](https://www.mozilla.org/firefox/). You may control using browser and the custom path for the browser by `markdown.marp.browser` and `markdown.marp.browserPath` settings.
+
+> [!NOTE]
+> A legacy setting `markdown.marp.chromePath` is deprecated since v2. Please use `markdown.marp.browserPath` instead.
 
 ### Use custom theme CSS 🛡️
 
@@ -175,7 +179,7 @@ Markdown preview will reload updated theme CSS automatically when you edited the
 
 ### Outline extension
 
-When Marp Markdown is enabled, you can use the extended [outline view](https://code.visualstudio.com/docs/languages/markdown#_outline-view) like following. They are enabled by default but you may disable by `markdown.marp.outlineExtension` preference.
+When Marp Markdown is enabled, you can use the extended [outline view](https://code.visualstudio.com/docs/languages/markdown#_outline-view) like following. They are enabled by default but you may disable by the `markdown.marp.outlineExtension` setting.
 
 #### Outline view for each slide
 

diff --git a/package.json b/package.json
@@ -115,10 +115,27 @@
             "Use inherited setting from `#markdown.preview.breaks#`."
           ]
         },
-        "markdown.marp.chromePath": {
+        "markdown.marp.browser": {
+          "type": "string",
+          "enum": [
+            "auto",
+            "chrome",
+            "edge",
+            "firefox"
+          ],
+          "default": "auto",
+          "description": "Controls the installed browser using internally to export PDF, PPTX, and the image.",
+          "enumDescriptions": [
+            "Automatically detect Chrome, Chromium, Edge, or Firefox.",
+            "Use Google Chrome.",
+            "Use Microsoft Edge.",
+            "Use Mozilla Firefox."
+          ]
+        },
+        "markdown.marp.browserPath": {
           "type": "string",
           "default": "",
-          "description": "Sets the custom path for Chrome or Chromium-based browser to export PDF, PPTX, and image. If it's empty, Marp will find out the installed Google Chrome / Chromium / Microsoft Edge."
+          "markdownDescription": "Configure the custom path for the installed browser using internally to export PDF, PPTX, and the image. The kind of browser is determined by `#markdown.marp.browser#`. When set to empty, Marp will find out a suitable installed browser automatically."
         },
         "markdown.marp.html": {
           "type": "string",
@@ -214,6 +231,12 @@
           "default": false,
           "description": "Enables all HTML elements in Marp Markdown. This setting is working only in the trusted workspace.",
           "deprecationMessage": "The setting \"markdown.marp.enableHtml\" is deprecated. Please use \"markdown.marp.html\" instead."
+        },
+        "markdown.marp.chromePath": {
+          "type": "string",
+          "default": "",
+          "description": "Sets the custom path for Chrome or Chromium-based browser to export PDF, PPTX, and image. If it's empty, Marp will find out the installed Google Chrome / Chromium / Microsoft Edge.",
+          "deprecationMessage": "The setting \"markdown.marp.chromePath\" is deprecated. Please use \"markdown.marp.browserPath\" instead."
         }
       }
     },

diff --git a/src/__mocks__/vscode.ts b/src/__mocks__/vscode.ts
@@ -5,14 +5,18 @@ type MockedConf = Record<string, any>
 const defaultVSCodeVersion = 'v1.62.1'
 const defaultConf: MockedConf = {
   'markdown.marp.breaks': 'on',
-  'markdown.marp.chromePath': '',
-  'markdown.marp.enableHtml': false,
+  'markdown.marp.browser': 'auto',
+  'markdown.marp.browserPath': '',
   'markdown.marp.html': 'default',
   'markdown.marp.exportType': 'pdf',
   'markdown.marp.outlineExtension': true,
   'markdown.marp.pdf.noteAnnotations': false,
   'markdown.marp.pdf.outlines': 'off',
   'window.zoomLevel': 0,
+
+  // Legacy
+  'markdown.marp.chromePath': '',
+  'markdown.marp.enableHtml': false,
 }
 
 let currentConf: MockedConf = {}

diff --git a/src/commands/export.test.ts b/src/commands/export.test.ts
@@ -1,3 +1,4 @@
+import * as marpCliModule from '@marp-team/marp-cli'
 import { commands, env, window, workspace } from 'vscode'
 import * as marpCli from '../marp-cli'
 import * as option from '../option'
@@ -317,6 +318,65 @@ describe('#doExport', () => {
     })
   })
 
+  describe('when CLI was thrown CLIError with BROWSER_NOT_FOUND error code', () => {
+    it.each`
+      browser      | platform    | expected
+      ${'auto'}    | ${'win32'}  | ${['Google Chrome', 'Microsoft Edge', 'Firefox']}
+      ${'auto'}    | ${'darwin'} | ${['Google Chrome', 'Microsoft Edge', 'Firefox']}
+      ${'auto'}    | ${'linux'}  | ${['Google Chrome', 'Chromium', 'Microsoft Edge', 'Firefox']}
+      ${'chrome'}  | ${'win32'}  | ${['Google Chrome']}
+      ${'chrome'}  | ${'darwin'} | ${['Google Chrome']}
+      ${'chrome'}  | ${'linux'}  | ${['Google Chrome', 'Chromium']}
+      ${'edge'}    | ${'win32'}  | ${['Microsoft Edge']}
+      ${'edge'}    | ${'darwin'} | ${['Microsoft Edge']}
+      ${'edge'}    | ${'linux'}  | ${['Microsoft Edge']}
+      ${'firefox'} | ${'win32'}  | ${['Firefox']}
+      ${'firefox'} | ${'darwin'} | ${['Firefox']}
+      ${'firefox'} | ${'linux'}  | ${['Firefox']}
+    `(
+      'throws MarpCLIError with the message contains $expected to suggest browsers when running on $platform with browser option as $browser',
+      async ({ browser, platform, expected }) => {
+        expect.assertions(expected.length + 1)
+        setConfiguration({ 'markdown.marp.browser': browser })
+
+        const { platform: originalPlatform } = process
+
+        try {
+          Object.defineProperty(process, 'platform', { value: platform })
+
+          const runMarpCLI = jest
+            .spyOn(marpCli, 'default')
+            .mockImplementation(async (_, __, opts) => {
+              opts?.onCLIError?.({
+                error: new marpCliModule.CLIError(
+                  'mocked error',
+                  marpCliModule.CLIErrorCode.NOT_FOUND_BROWSER,
+                ),
+                codes: marpCliModule.CLIErrorCode,
+              })
+            })
+
+          try {
+            await exportModule.doExport(saveURI(), document)
+            expect(window.showErrorMessage).toHaveBeenCalledTimes(1)
+
+            for (const fragment of expected) {
+              expect(window.showErrorMessage).toHaveBeenCalledWith(
+                expect.stringContaining(fragment),
+              )
+            }
+          } finally {
+            runMarpCLI.mockRestore()
+          }
+        } finally {
+          Object.defineProperty(process, 'platform', {
+            value: originalPlatform,
+          })
+        }
+      },
+    )
+  })
+
   describe('when the save path has non-file scheme', () => {
     it('exports the document into temporally path and copy it to the save path', async () => {
       const marpCliMock = jest.spyOn(marpCli, 'default').mockImplementation()

diff --git a/src/commands/export.ts b/src/commands/export.ts
@@ -45,6 +45,13 @@ const descriptions = {
   [Types.jpeg]: 'JPEG image (first slide only)' as const,
 }
 
+const browsers = {
+  chrome: '[Google Chrome](https://www.google.com/chrome/)',
+  chromium: '[Chromium](https://www.chromium.org/)',
+  edge: '[Microsoft Edge](https://www.microsoft.com/edge)',
+  firefox: '[Mozilla Firefox](https://www.mozilla.org/firefox/)',
+} as const
+
 export const ITEM_CONTINUE_TO_EXPORT = 'Continue to export...'
 export const ITEM_MANAGE_WORKSPACE_TRUST = 'Manage Workspace Trust...'
 
@@ -140,9 +147,51 @@ export const doExport = async (uri: Uri, document: TextDocument) => {
       })
 
       try {
-        await marpCli(['-c', conf.path, input.path, '-o', outputPath], {
-          baseUrl,
-        })
+        await marpCli(
+          ['-c', conf.path, input.path, '-o', outputPath],
+          { baseUrl },
+          {
+            onCLIError: ({ error, codes }) => {
+              if (error.errorCode === codes.NOT_FOUND_BROWSER) {
+                // Throw error with user-friendly instructions based on the current configuration
+                const browserOption = marpConfiguration().get<string>('browser')
+                const suggestBrowsers: string[] = []
+
+                switch (browserOption) {
+                  case 'chrome':
+                    suggestBrowsers.push(
+                      ...[
+                        browsers.chrome,
+                        process.platform === 'linux' ? browsers.chromium : '',
+                      ].filter((b) => !!b),
+                    )
+                    break
+                  case 'edge':
+                    suggestBrowsers.push(browsers.edge)
+                    break
+                  case 'firefox':
+                    suggestBrowsers.push(browsers.firefox)
+                    break
+                  default:
+                    suggestBrowsers.push(
+                      ...[
+                        browsers.chrome,
+                        process.platform === 'linux' ? browsers.chromium : '',
+                        browsers.edge,
+                        browsers.firefox,
+                      ].filter((b) => !!b),
+                    )
+                }
+
+                throw new MarpCLIError(
+                  `It requires to install a suitable browser, ${suggestBrowsers
+                    .join(', ')
+                    .replace(/, ([^,]*)$/, ' or $1')} for exporting.`,
+                )
+              }
+            },
+          },
+        )
 
         if (outputToLocalFS) {
           env.openExternal(uri)

diff --git a/src/marp-cli.test.ts b/src/marp-cli.test.ts
@@ -7,10 +7,6 @@ import { textEncoder } from './utils'
 
 jest.mock('vscode')
 
-const setConfiguration: (conf?: Record<string, unknown>) => void = (
-  workspace as any
-)._setConfiguration
-
 describe('Marp CLI integration', () => {
   const runMarpCli = marpCli.default
 
@@ -31,7 +27,7 @@ describe('Marp CLI integration', () => {
   })
 
   it('runs Marp CLI with passed args', async () => {
-    const marpCliSpy = jest.spyOn(marpCliModule, 'marpCli')
+    const marpCliSpy = jest.spyOn(marpCliModule, 'marpCli').mockResolvedValue(0)
     await runMarpCli(['--version'])
 
     expect(marpCliSpy).toHaveBeenCalledWith(['--version'], undefined)
@@ -50,67 +46,6 @@ describe('Marp CLI integration', () => {
       marpCliMock.mockRestore()
     }
   })
-
-  it.each`
-    platform    | expected
-    ${'win32'}  | ${[/Google Chrome/, /Microsoft Edge/]}
-    ${'darwin'} | ${[/Google Chrome/, /Microsoft Edge/]}
-    ${'linux'}  | ${[/Google Chrome/, /Chromium/]}
-  `(
-    'contains $expected to suggested browsers in error message when running on $platform',
-    async ({ platform, expected }) => {
-      expect.assertions(expected.length)
-
-      const originalPlatform = process.platform
-
-      try {
-        Object.defineProperty(process, 'platform', { value: platform })
-
-        const marpCliMock = jest
-          .spyOn(marpCliModule, 'marpCli')
-          .mockRejectedValue(
-            new marpCliModule.CLIError(
-              'mocked error',
-              marpCliModule.CLIErrorCode.NOT_FOUND_CHROMIUM,
-            ),
-          )
-
-        try {
-          for (const fragment of expected) {
-            await expect(runMarpCli(['--version'])).rejects.toThrow(fragment)
-          }
-        } finally {
-          marpCliMock.mockRestore()
-        }
-      } finally {
-        Object.defineProperty(process, 'platform', { value: originalPlatform })
-      }
-    },
-  )
-
-  describe('with markdown.marp.chromePath preference', () => {
-    it('runs Marp CLI with overridden CHROME_PATH environment', async () => {
-      const { CHROME_PATH } = process.env
-      expect(process.env.CHROME_PATH).toBe(CHROME_PATH)
-
-      setConfiguration({ 'markdown.marp.chromePath': __filename })
-
-      const marpCliMock = jest
-        .spyOn(marpCliModule, 'marpCli')
-        .mockImplementation(async () => {
-          expect(process.env.CHROME_PATH).toBe(__filename)
-          return 0
-        })
-
-      try {
-        await runMarpCli(['--version'])
-        expect(marpCliMock).toHaveBeenCalled()
-        expect(process.env.CHROME_PATH).toBe(CHROME_PATH)
-      } finally {
-        marpCliMock.mockRestore()
-      }
-    })
-  })
 })
 
 describe('#createWorkFile', () => {