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

Add option for using a custom Marp CLI config #123

Closed
wants to merge 10 commits into from

Conversation

zhangchuck
Copy link

@zhangchuck zhangchuck commented Feb 9, 2020

Proposal

Allow marp-vscode power users to take advantage of all features (current and future) in Marp CLI by allowing them to specify a custom config file (instead of using the default config provided by marp-vscode).

If this option is specified, the output may be different than the version seen in vscode's markdown preview. This is why the option will be by default turned off.

Related to: #85, #86, #121

Steps

  • Add an option in vscode to toggle the option customConfig
    • Default should be false
      - [ ] Write logic to load custom config
      - [ ] Search for default config filenames in the root of the workspace directory (e.g., .marprc, marp.config.js
      - [ ] Load the configurations and add/overwrite the default config
  • Check customConfig option when exporting to file to determine whether to create a default config or load a custom config
    • Allow marp cli to search markdown document's path
      - [x] Fall-back on creating a custom config

Just copying the `createConfigFile()` function for now, because we want it to return the same output.
Add a try-except-finally block.

In the try block, check if the `customConfig` option is specified and load the default config file (still working on this). Else, call `createConfigFile()`)

If there is an error (e.g., no file, invalid config file), revert to creating the Config File.
Added comments to propose an implementation, but my typescript is inadequate.
@yhatt
Copy link
Member

yhatt commented Feb 9, 2020

I prefer to leave resolution of configuration file to Marp CLI rather than find it by VS Code extension. Marp CLI will find the appropriate config file from current working directory (process.cwd()) through cosmiconfig. When added option is true, I think it's better to execute CLI with changing current directory to the workspace directory (via process.chdir()).

Allow Marp CLI to find file by changing the process directory to the directory of the markdown file.
@zhangchuck
Copy link
Author

Made edits as suggested; not sure exactly why the tests are failing.

@yhatt yhatt marked this pull request as ready for review February 14, 2020 09:39
@yhatt
Copy link
Member

yhatt commented Feb 14, 2020

@zhangchuck CI failed due to the incorrect Prettier formatting. Could you run yarn format:write?

*/
const currentDir = path.cwd()
process.chdir(path.dirname(document.uri.fsPath))
await marpCli(input.path, '-o', uri.fsPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some options might have to override in CLI arguments for export the document as VS Code user expected. For example:

  • Disable extra conversion modes such as watch, server, and preview mode (These bring looking like hang up conversion process in VS Code UI). --no-server, --no-watch and --no-preview will work.
  • Prefer export type selected in the save dialog rather than specified in configuration file. (Require to pass specific flag: --pdf, --pptx, and --image [png|jpeg])

@yomybaby
Copy link

yomybaby commented Mar 7, 2020

👍 This is what I want.

@rawkode
Copy link

rawkode commented Sep 11, 2020

What's the latest on this? I'd like to be able to provide my own engine when using the vscode plugin; to enable the markdown-it-include plugin.

I have this working for CLI, but I can't find a way to do this for VSCode. Does this PR address that?

@yhatt
Copy link
Member

yhatt commented Sep 12, 2020

That's correct. But Marp team is not leading this PR because an implicit JS execution might make a security hole. If enabled this option in general, a malicious workspace may execute any evil script. A classic Marp app had burned out by that weakness so we are passive for supporting this PR. See also: #135 (comment)

@yhatt
Copy link
Member

yhatt commented Dec 5, 2020

Closed this PR because has become too outdated. But anyone can produce another PR based on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants