diff --git a/CHANGELOG.md b/CHANGELOG.md index 62971fa2..fa9b6267 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## [Unreleased] +### Fixed + +- Improve LibreOffice handling in experimental `--pptx-editable` option ([#632](https://github.com/marp-team/marp-cli/pull/632)) + - Detect LibreOffice automatically that was installed in Windows by Scoop ([#631](https://github.com/marp-team/marp-cli/issues/631), [#632](https://github.com/marp-team/marp-cli/pull/632)) + ## v4.1.0 - 2025-01-15 ### Added diff --git a/src/browser/manager.ts b/src/browser/manager.ts index aa94d0de..081c64e9 100644 --- a/src/browser/manager.ts +++ b/src/browser/manager.ts @@ -50,7 +50,7 @@ export class BrowserManager implements AsyncDisposable { this._finderResult.value = undefined // Reset finder result cache } if (config.protocol) { - if (this._conversionBrowser) + if (this._conversionBrowser.value) debugBrowser( 'WARNING: Changing protocol after created browser for conversion is not supported' ) diff --git a/src/soffice/finder.ts b/src/soffice/finder.ts index 964eeefd..5488b04d 100644 --- a/src/soffice/finder.ts +++ b/src/soffice/finder.ts @@ -7,7 +7,8 @@ import { isExecutable, normalizeDarwinAppPath, } from '../utils/finder' -import { getWSL2NetworkingMode } from '../utils/wsl' + +const scoopRestPath = ['scoop', 'apps', 'libreoffice', 'current'] as const const sOffice = (path: string) => ({ path }) as const @@ -28,11 +29,9 @@ export const findSOffice = async ({ return await sOfficeFinderDarwin() case 'win32': return await sOfficeFinderWin32() - case 'wsl1': - return await sOfficeFinderWSL() } - return await sOfficeFinderLinuxOrFallback() + return await sOfficeFinderLinux() })() if (installation) return sOffice(installation) @@ -73,49 +72,19 @@ const sOfficeFinderWin32 = async () => { } } - return await findExecutable( - prefixes.map((prefix) => - path.join(prefix, 'LibreOffice', 'program', 'soffice.exe') - ) - ) -} - -const sOfficeFinderWSL = async () => { - const prefixes: string[] = [] - - const winDriveMatcher = /^\/mnt\/[a-z]\//i - const winPossibleDrives = () => { - const possibleDriveSet = new Set(['c']) - const pathEnvs = process.env.PATH?.split(':') ?? [] - - for (const pathEnv of pathEnvs) { - if (winDriveMatcher.test(pathEnv)) { - possibleDriveSet.add(pathEnv[5].toLowerCase()) - } - } - - return Array.from(possibleDriveSet).sort() + // Scoop + if (process.env.USERPROFILE) { + prefixes.push(path.join(process.env.USERPROFILE, ...scoopRestPath)) } - - for (const drive of winPossibleDrives()) { - prefixes.push(`/mnt/${drive}/Program Files`) - prefixes.push(`/mnt/${drive}/Program Files (x86)`) + if (process.env.ALLUSERSPROFILE) { + prefixes.push(path.join(process.env.ALLUSERSPROFILE, ...scoopRestPath)) } return await findExecutable( prefixes.map((prefix) => - path.posix.join(prefix, 'LibreOffice', 'program', 'soffice.exe') + path.join(prefix, 'LibreOffice', 'program', 'soffice.exe') ) ) } -const sOfficeFinderLinuxOrFallback = async () => { - const ret = await findExecutableBinary(['soffice']) - if (ret) return ret - - // WSL2 Fallback - if ((await getWSL2NetworkingMode()) === 'mirrored') - return await sOfficeFinderWSL() - - return undefined -} +const sOfficeFinderLinux = async () => await findExecutableBinary(['soffice']) diff --git a/src/soffice/soffice.ts b/src/soffice/soffice.ts index f33992d0..1d92e09f 100644 --- a/src/soffice/soffice.ts +++ b/src/soffice/soffice.ts @@ -5,26 +5,26 @@ import path from 'node:path' import { pathToFileURL } from 'node:url' import chalk from 'chalk' import { nanoid } from 'nanoid' -import { error } from '../cli' +import { warn } from '../cli' import { debug } from '../utils/debug' import { createMemoizedPromiseContext } from '../utils/memoized-promise' -import { getWindowsEnv, isWSL, translateWindowsPathToWSL } from '../utils/wsl' import { findSOffice } from './finder' -const wslHostMatcher = /^\/mnt\/[a-z]\// - -let wslTmp: string | undefined - export interface SOfficeOptions { path?: string } +export interface SOfficeProfileDir { + path: string + fileURL: string +} + export class SOffice { preferredPath?: string #profileDirName: string private _path = createMemoizedPromiseContext() - private _profileDir = createMemoizedPromiseContext() + private _profileDir = createMemoizedPromiseContext() private static _spawnQueue: Promise = Promise.resolve() @@ -40,7 +40,7 @@ export class SOffice { ) } - get profileDir(): Promise { + get profileDir(): Promise { return this._profileDir.init(async () => await this.setProfileDir()) } @@ -49,7 +49,7 @@ export class SOffice { SOffice._spawnQueue = SOffice._spawnQueue .then(async () => { const spawnArgs = [ - `-env:UserInstallation=${pathToFileURL(await this.profileDir).toString()}`, + `-env:UserInstallation=${(await this.profileDir).fileURL}`, ...args, ] @@ -67,7 +67,7 @@ export class SOffice { const output = data.toString() debug(`[soffice:stderr] %s`, output) - error(`${chalk.yellow`[soffice]`} ${output.trim()}`, { + warn(`${chalk.yellow`[soffice]`} ${output.trim()}`, { singleLine: true, }) }) @@ -88,36 +88,13 @@ export class SOffice { }) } - private async binaryInWSLHost(): Promise { - return !!(await isWSL()) && wslHostMatcher.test(await this.path) - } - - private async setProfileDir(): Promise { - let needToTranslateWindowsPathToWSL = false - - const dir = await (async () => { - // In WSL environment, Marp CLI may use soffice on Windows. If soffice has - // located in host OS (Windows), we have to specify Windows path. - if (await this.binaryInWSLHost()) { - if (wslTmp === undefined) wslTmp = await getWindowsEnv('TMP') - if (wslTmp !== undefined) { - needToTranslateWindowsPathToWSL = true - return path.win32.resolve(wslTmp, this.#profileDirName) - } - } - return path.resolve(os.tmpdir(), this.#profileDirName) - })() - + private async setProfileDir(): Promise { + const dir = path.resolve(os.tmpdir(), this.#profileDirName) debug(`soffice data directory: %s`, dir) - // Ensure the data directory is created - const mkdirPath = needToTranslateWindowsPathToWSL - ? await translateWindowsPathToWSL(dir) - : dir - - await fs.promises.mkdir(mkdirPath, { recursive: true }) - debug(`Created data directory: %s`, mkdirPath) + await fs.promises.mkdir(dir, { recursive: true }) + debug(`soffice data directory created: %s`, dir) - return dir + return { path: dir, fileURL: pathToFileURL(dir).toString() } } } diff --git a/test/browser/manager.ts b/test/browser/manager.ts index 7fe68ace..f14fd118 100644 --- a/test/browser/manager.ts +++ b/test/browser/manager.ts @@ -1,9 +1,16 @@ +import debug from 'debug' +import { Browser } from '../../src/browser/browser' import { ChromeBrowser } from '../../src/browser/browsers/chrome' import { ChromeCdpBrowser } from '../../src/browser/browsers/chrome-cdp' import { FirefoxBrowser } from '../../src/browser/browsers/firefox' import * as browserFinder from '../../src/browser/finder' import { BrowserManager } from '../../src/browser/manager' +jest.mock('debug', () => { + const debugMock = jest.fn() + return () => debugMock +}) + afterEach(() => { jest.resetAllMocks() jest.restoreAllMocks() @@ -25,6 +32,25 @@ describe('Browser manager', () => { expect(manager._preferredProtocol).toBe('webDriverBiDi') expect(manager.timeout).toBe(12345) }) + + it('cannot update the browser protocol after the browser is created', async () => { + const manager = new BrowserManager({ protocol: 'webDriverBiDi' }) + const debugMock = debug('test') + + const browser = await manager.browserForConversion() + expect(browser).toBeInstanceOf(Browser) + expect(browser.protocol).toBe('webDriverBiDi') + + manager.configure({ protocol: 'cdp' }) + expect(debugMock).toHaveBeenCalledWith( + expect.stringContaining( + 'Changing protocol after created browser for conversion is not supported' + ) + ) + expect((await manager.browserForConversion()).protocol).toBe( + 'webDriverBiDi' + ) + }) }) describe('#browserForConversion', () => { diff --git a/test/soffice/finder.ts b/test/soffice/finder.ts index 13322856..4f37ec14 100644 --- a/test/soffice/finder.ts +++ b/test/soffice/finder.ts @@ -3,7 +3,6 @@ import which from 'which' import { CLIError } from '../../src/error' import { findSOffice } from '../../src/soffice/finder' import * as utils from '../../src/utils/finder' -import * as wsl from '../../src/utils/wsl' jest.mock('which') @@ -17,6 +16,13 @@ afterEach(() => { }) const sofficePathRest = ['LibreOffice', 'program', 'soffice.exe'] +const sofficeScoopPathRest = [ + 'scoop', + 'apps', + 'libreoffice', + 'current', + ...sofficePathRest, +] const itExceptWin = process.platform === 'win32' ? it.skip : it @@ -113,31 +119,6 @@ describe('SOffice finder', () => { await expect(findSOffice({})).rejects.toThrow(CLIError) expect(which).toHaveBeenCalled() }) - - it('fallbacks to WSL resolution if in WSL 2 with mirrored network mode', async () => { - jest.spyOn(wsl, 'getWSL2NetworkingMode').mockResolvedValue('mirrored') - - mockedWhich.mockImplementation(async () => null) - - jest - .spyOn(utils, 'isExecutable') - .mockImplementation( - async (p) => - p === '/mnt/c/Program Files/LibreOffice/program/soffice.exe' - ) - - expect(await findSOffice({})).toStrictEqual({ - path: '/mnt/c/Program Files/LibreOffice/program/soffice.exe', - }) - expect(which).toHaveBeenCalled() - }) - - it('throws error if in WSL 2 with NAT mode', async () => { - jest.spyOn(wsl, 'getWSL2NetworkingMode').mockResolvedValue('nat') - mockedWhich.mockImplementation(async () => null) - - await expect(findSOffice({})).rejects.toThrow(CLIError) - }) }) describe('with macOS', () => { @@ -172,6 +153,8 @@ describe('SOffice finder', () => { describe('with Windows', () => { const winProgramFiles = ['c:', 'Mock', 'Program Files'] const winProgramFilesX86 = ['c:', 'Mock', 'Program Files (x86)'] + const winUserProfile = ['c:', 'Mock', 'Users', 'user'] + const winAllUsersProfile = ['c:', 'Mock', 'ProgramData'] const sofficePath = path.join(...winProgramFiles, ...sofficePathRest) const originalEnv = { ...process.env } @@ -189,6 +172,8 @@ describe('SOffice finder', () => { PATH: undefined, PROGRAMFILES: path.join(...winProgramFiles), 'PROGRAMFILES(X86)': path.join(...winProgramFilesX86), + USERPROFILE: path.join(...winUserProfile), + ALLUSERSPROFILE: path.join(...winAllUsersProfile), } }) @@ -204,6 +189,10 @@ describe('SOffice finder', () => { expect(findExecutableSpy).toHaveBeenCalledWith([ path.join(...winProgramFiles, ...sofficePathRest), path.join(...winProgramFilesX86, ...sofficePathRest), + + // Scoop + path.join(...winUserProfile, ...sofficeScoopPathRest), + path.join(...winAllUsersProfile, ...sofficeScoopPathRest), ]) }) @@ -215,6 +204,8 @@ describe('SOffice finder', () => { expect(findExecutableSpy).toHaveBeenCalledWith([ path.join(...winProgramFiles, ...sofficePathRest), + path.join(...winUserProfile, ...sofficeScoopPathRest), + path.join(...winAllUsersProfile, ...sofficeScoopPathRest), ]) }) @@ -231,12 +222,16 @@ describe('SOffice finder', () => { path.join('d:', ...winProgramFilesX86.slice(1), ...sofficePathRest), path.join('z:', ...winProgramFiles.slice(1), ...sofficePathRest), path.join('z:', ...winProgramFilesX86.slice(1), ...sofficePathRest), + path.join(...winUserProfile, ...sofficeScoopPathRest), + path.join(...winAllUsersProfile, ...sofficeScoopPathRest), ]) }) it('throws error if no executable path is found', async () => { process.env.PROGRAMFILES = '' process.env['PROGRAMFILES(X86)'] = '' + process.env.USERPROFILE = '' + process.env.ALLUSERSPROFILE = '' const findExecutableSpy = jest.spyOn(utils, 'findExecutable') await expect(findSOffice({})).rejects.toThrow(CLIError) @@ -244,60 +239,4 @@ describe('SOffice finder', () => { expect(findExecutableSpy).toHaveBeenCalledWith([]) }) }) - - describe('with WSL1', () => { - const originalEnv = { ...process.env } - - beforeEach(() => { - jest.resetModules() - - jest.spyOn(utils, 'getPlatform').mockResolvedValue('wsl1') - jest - .spyOn(utils, 'isExecutable') - .mockImplementation( - async (p) => - p === '/mnt/c/Program Files/LibreOffice/program/soffice.exe' - ) - - process.env = { ...originalEnv, PATH: undefined } - }) - - afterEach(() => { - process.env = { ...originalEnv } - }) - - it('finds possible executable path and returns the matched path', async () => { - const findExecutableSpy = jest.spyOn(utils, 'findExecutable') - const soffice = await findSOffice({}) - - expect(soffice).toStrictEqual({ - path: '/mnt/c/Program Files/LibreOffice/program/soffice.exe', - }) - expect(findExecutableSpy).toHaveBeenCalledWith([ - path.posix.join('/mnt/c/Program Files', ...sofficePathRest), - path.posix.join('/mnt/c/Program Files (x86)', ...sofficePathRest), - ]) - }) - - it('finds from detected drives when the PATH environment has paths starting with any drive letters', async () => { - process.env.PATH = '/mnt/z/Mock:/mnt/d/Mock:/mnt/d/Mock/Mock' - - const findExecutableSpy = jest.spyOn(utils, 'findExecutable') - await findSOffice({}) - - expect(findExecutableSpy).toHaveBeenCalledWith([ - path.posix.join('/mnt/c/Program Files', ...sofficePathRest), - path.posix.join('/mnt/c/Program Files (x86)', ...sofficePathRest), - path.posix.join('/mnt/d/Program Files', ...sofficePathRest), - path.posix.join('/mnt/d/Program Files (x86)', ...sofficePathRest), - path.posix.join('/mnt/z/Program Files', ...sofficePathRest), - path.posix.join('/mnt/z/Program Files (x86)', ...sofficePathRest), - ]) - }) - - it('throws error if no executable path is found', async () => { - jest.spyOn(utils, 'isExecutable').mockResolvedValue(false) - await expect(findSOffice({})).rejects.toThrow(CLIError) - }) - }) }) diff --git a/test/soffice/soffice.ts b/test/soffice/soffice.ts index 070db0c2..eba96d73 100644 --- a/test/soffice/soffice.ts +++ b/test/soffice/soffice.ts @@ -3,7 +3,6 @@ import EventEmitter from 'node:events' import fs from 'node:fs' import * as cli from '../../src/cli' import { SOffice } from '../../src/soffice/soffice' -import * as wsl from '../../src/utils/wsl' const defaultSpawnSetting = { code: 0, delay: 50 } const spawnSetting = { ...defaultSpawnSetting } @@ -60,7 +59,7 @@ describe('SOffice class', () => { it('throws error if soffice exits with non-zero code', async () => { spawnSetting.code = 123 - jest.spyOn(cli, 'error').mockImplementation() + jest.spyOn(cli, 'warn').mockImplementation() const spawnSpy = jest.spyOn(childProcess, 'spawn') const soffice = new SOffice() @@ -94,7 +93,7 @@ describe('SOffice class', () => { }) describe('get #profileDir', () => { - it('returns the profile directory', async () => { + it('returns the profile directory object', async () => { const mkdir = jest .spyOn(fs.promises, 'mkdir') .mockResolvedValue(undefined) @@ -102,61 +101,10 @@ describe('SOffice class', () => { const soffice = new SOffice() const profileDir = await soffice.profileDir - expect(profileDir).toContain('marp-cli-soffice-') - expect(mkdir).toHaveBeenCalledWith(profileDir, { recursive: true }) - }) - - it('returns the profile directory with Windows TMP if the binary is in WSL host', async () => { - const mkdir = jest - .spyOn(fs.promises, 'mkdir') - .mockResolvedValue(undefined) - - jest.spyOn(wsl, 'getWindowsEnv').mockResolvedValue('C:\\Windows\\Temp') - jest - .spyOn(wsl, 'translateWindowsPathToWSL') - .mockResolvedValue('/mnt/c/Windows/Temp/test') - - const soffice = new SOffice() - jest.spyOn(soffice as any, 'binaryInWSLHost').mockResolvedValue(true) - - const profileDir = await soffice.profileDir - expect(profileDir).toContain('C:\\Windows\\Temp') - expect(profileDir).toContain('marp-cli-soffice-') - expect(mkdir).toHaveBeenCalledWith('/mnt/c/Windows/Temp/test', { - recursive: true, - }) - }) - }) - - describe('private #binaryInWSLHost', () => { - it('always returns false if the current environment is not WSL', async () => { - jest.spyOn(wsl, 'isWSL').mockResolvedValue(0) - - const soffice: any = new SOffice({ - path: '/mnt/c/Program Files/LibreOffice/program/soffice.exe', - }) - - expect(await soffice.binaryInWSLHost()).toBe(false) - }) - - it('returns true if the current environment is WSL and the browser path is located in the host OS', async () => { - jest.spyOn(wsl, 'isWSL').mockResolvedValue(1) - - const soffice: any = new SOffice({ - path: '/mnt/c/Program Files/LibreOffice/program/soffice.exe', - }) - - expect(await soffice.binaryInWSLHost()).toBe(true) - }) - - it('returns false if the current environment is WSL and the browser path is not located in the host OS', async () => { - jest.spyOn(wsl, 'isWSL').mockResolvedValue(1) - - const soffice: any = new SOffice({ - path: '/usr/lib/libreoffice/program/libreoffice', - }) - - expect(await soffice.binaryInWSLHost()).toBe(false) + expect(profileDir.path).toContain('marp-cli-soffice-') + expect(profileDir.fileURL).toContain('marp-cli-soffice-') + expect(profileDir.fileURL.startsWith('file://')).toBe(true) + expect(mkdir).toHaveBeenCalledWith(profileDir.path, { recursive: true }) }) }) })