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

Fix the default export name for untitled Markdown #285

Merged
merged 3 commits into from
Sep 3, 2021
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

- [Early preview] Set up build for Web extension such as [github.dev](https://github.dev) ([#281](https://github.com/marp-team/marp-vscode/issues/281), [#283](https://github.com/marp-team/marp-vscode/pull/283))

### Fixed

- Fix the default export name for untitled Markdown ([#280](https://github.com/marp-team/marp-vscode/issues/280), [#285](https://github.com/marp-team/marp-vscode/pull/285))

### Changed

- Upgrade Marp CLI to [v1.4.0](https://github.com/marp-team/marp-cli/releases/tag/v1.4.0) ([#282](https://github.com/marp-team/marp-vscode/pull/282))
Expand Down
22 changes: 21 additions & 1 deletion src/commands/export.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ describe('#saveDialog', () => {
)
})

it('opens save dialog with default name "untitled" if the document is untitled', async () => {
const document: any = { uri: { fsPath: '' }, isUntitled: true }

await exportModule.saveDialog(document)

expect(window.showSaveDialog).toHaveBeenCalledWith(
expect.objectContaining({
defaultUri: expect.objectContaining({ fsPath: 'untitled.pdf' }),
})
)
})

it('opens save dialog with configured default type', async () => {
setConfiguration({ 'markdown.marp.exportType': 'pptx' })

Expand Down Expand Up @@ -197,13 +209,21 @@ describe('#doExport', () => {
expect.stringContaining('[Error] ERROR')
)

// Unknown error
// Unknown error (via toString())
marpCliSpy.mockRejectedValueOnce('UNKNOWN ERROR!')
await exportModule.doExport(saveURI(), document)

expect(window.showErrorMessage).toHaveBeenCalledWith(
expect.stringContaining('UNKNOWN ERROR!')
)

// WTF
marpCliSpy.mockRejectedValueOnce(null)
await exportModule.doExport(saveURI(), document)

expect(window.showErrorMessage).toHaveBeenCalledWith(
'Failure to export by unknown error.'
)
})

describe('when enabled markdown.marp.pdf.noteAnnotations', () => {
Expand Down
11 changes: 8 additions & 3 deletions src/commands/export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import marpCli, {
createWorkFile,
MarpCLIError,
} from '../marp-cli'
import { marpConfiguration, unlink } from '../utils'
import { marpConfiguration, unlink, hasToString } from '../utils'
import {
createWorkspaceProxyServer,
WorkspaceProxyServer,
Expand Down Expand Up @@ -152,8 +152,9 @@ export const doExport = async (uri: Uri, document: TextDocument) => {
`Failure to export${(() => {
if (e instanceof MarpCLIError) return `. ${e.message}`
if (e instanceof Error) return `: [${e.name}] ${e.message}`
if (hasToString(e)) return `. ${e.toString()}`

return `. ${e.toString()}`
return ' by unknown error.'
})()}`
)
} finally {
Expand All @@ -174,7 +175,11 @@ export const saveDialog = async (document: TextDocument) => {
const types = [...new Set<string>([defaultType, ...baseTypes])]

const saveURI = await window.showSaveDialog({
defaultUri: Uri.file(fsPath.slice(0, -path.extname(fsPath).length) + ext),
defaultUri: Uri.file(
(document.isUntitled
? 'untitled'
: fsPath.slice(0, -path.extname(fsPath).length)) + ext
),
filters: types.reduce((f, t) => {
if (baseTypes.includes(t)) f[descriptions[t]] = extensions[t]
return f
Expand Down
20 changes: 20 additions & 0 deletions src/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,24 @@ describe('Utilities', () => {
jest.advanceTimersByTime(timeout)
}))
})

describe('#hasToString', () => {
it('returns true if the object has a toString method', () => {
expect(utils.hasToString({ toString: () => 'test' })).toBe(true)
expect(utils.hasToString({})).toBe(true)
expect(utils.hasToString(Object.create(null))).toBe(false)
})

it('returns true if the literal has a toString method', () => {
expect(utils.hasToString(0)).toBe(true)
expect(utils.hasToString(BigInt(1))).toBe(true)
expect(utils.hasToString('string')).toBe(true)
expect(utils.hasToString(Symbol())).toBe(true)
expect(utils.hasToString(utils.hasToString)).toBe(true)
expect(utils.hasToString(false)).toBe(true)
expect(utils.hasToString(true)).toBe(true)
expect(utils.hasToString(null)).toBe(false)
expect(utils.hasToString(undefined)).toBe(false)
})
})
})
17 changes: 17 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,20 @@ export const writeFile = (target: Uri, text: string) =>

export const unlink = (target: Uri) =>
workspace.fs.delete(target, { useTrash: false })

export const hasToString = (
target: unknown
): target is { toString(): string } => {
switch (typeof target) {
case 'object':
return typeof target?.toString === 'function'
case 'bigint':
case 'boolean':
case 'function':
case 'number':
case 'string':
case 'symbol':
return true
}
return false
}