Skip to content
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

Follow up for experimental --pptx-editable in Windows environment #632

Merged
merged 4 commits into from
Jan 19, 2025
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/browser/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
)
Expand Down
51 changes: 10 additions & 41 deletions src/soffice/finder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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<string>(['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'])
53 changes: 15 additions & 38 deletions src/soffice/soffice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>()
private _profileDir = createMemoizedPromiseContext<string>()
private _profileDir = createMemoizedPromiseContext<SOfficeProfileDir>()

private static _spawnQueue: Promise<void> = Promise.resolve()

Expand All @@ -40,7 +40,7 @@ export class SOffice {
)
}

get profileDir(): Promise<string> {
get profileDir(): Promise<SOfficeProfileDir> {
return this._profileDir.init(async () => await this.setProfileDir())
}

Expand All @@ -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,
]

Expand All @@ -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,
})
})
Expand All @@ -88,36 +88,13 @@ export class SOffice {
})
}

private async binaryInWSLHost(): Promise<boolean> {
return !!(await isWSL()) && wslHostMatcher.test(await this.path)
}

private async setProfileDir(): Promise<string> {
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<SOfficeProfileDir> {
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() }
}
}
26 changes: 26 additions & 0 deletions test/browser/manager.ts
Original file line number Diff line number Diff line change
@@ -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()
Expand All @@ -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', () => {
Expand Down
Loading