Skip to content
This repository has been archived by the owner on Apr 30, 2024. It is now read-only.

Review current emd language configuration and determine why it overwrites built-in .md Open Preview command in vscode #9

Closed
RandomFractals opened this issue May 22, 2023 · 7 comments
Labels
bug Something isn't working markdown Markdown syntax and features support review Requires additional testing and review

Comments

@RandomFractals
Copy link
Contributor

RandomFractals commented May 22, 2023

Evidence extension maps keybindings for the built-in markdown document Preview commands in package.json. This hides access to the .md Preview for the standard markdown documents.

https://github.com/evidence-dev/vscode/blob/main/package.json#L109

keybindings": [
      {
        "command": "markdown.showPreview",
        "key": "shift+ctrl+v",
        "mac": "shift+cmd+v",
        "when": "!notebookEditorFocused && editorLangId == 'emd'"
      },
      {
        "command": "markdown.showPreviewToSide",
        "key": "ctrl+k v",
        "mac": "cmd+k v",
        "when": "!notebookEditorFocused && editorLangId == 'emd'"
      }
    ]

Current emd language configuration also marks all .md files as Evidence markdown documents.

We need to resolve this issue to avoid negative reviews in vscode marketplace. Evidence markdown should not hijack normal .md handling in vscode.

Recommended solution used by all custom markdown language flavors in vscode is to use a custom filename extension (.emd) for the Evidence app page files.

See related thread in vscode channel in Evidence dev slack.

@RandomFractals RandomFractals added the bug Something isn't working label May 22, 2023
@archiewood
Copy link
Member

Alternative would be to identify .md documents as Evidence using some other method.

Eg

---
md_flavour: Evidence
---

etc

Regarding rendering on Github and other platforms, there is an interesting thread about R markdown here, which might be instructive: https://yihui.org/en/2018/10/rmd-github/

@RandomFractals RandomFractals added the review Requires additional testing and review label May 29, 2023
@RandomFractals
Copy link
Contributor Author

RandomFractals commented May 29, 2023

@archiewood, we can certainly identify when we load emd markdown document.

The problem with the current emd language config is that it overwrites default markdown document Preview, Outline and rendering when .md documents are created or open in VS Code.

I outlined the solution we'll pursue to resolve this issue in this slack thread.

Brief notes:

  1. VS Code uses markdown-it engine.
  2. In order to tap into it, we'd need to create evidence markdown-it plugin.
  3. Then we need to enable custom markdown plugins in package.json by adding markdown.markdownItPlugins ext. contribution point.
  4. Then we should create our own custom .md preview that will render your app content via new Preview in simple browser added in sprint 1, or via markdown.showPreview if there is no custom emd content in the loaded .md doc.

Then and only then we can make everything work without breaking default markdown Preview in vscode. Example of hooking up custom markdown-it plugin in vscode extension:

import * as vscode from 'vscode';
import * as path from 'path';
import * as fs from 'fs';

export function activate(context: vscode.ExtensionContext) {
  const md = require('markdown-it')();
  const extPath = context.extensionPath;
  const customPluginPath = path.join(extPath, 'customPlugin.js');
  const customPlugin = fs.readFileSync(customPluginPath, 'utf8');

  md.use(require(customPlugin).extendMarkdownIt);

  context.subscriptions.push(
    vscode.commands.registerCommand('extension.previewMarkdown', () => {
      const editor = vscode.window.activeTextEditor;
      if (editor) {
        const document = editor.document;
        const uri = document.uri;
        if (uri.scheme === 'file') {
          vscode.commands.executeCommand('markdown.showPreview', uri);
        }
      }
    })
  );
}

@mcrascal
Copy link
Member

Then we should create our own custom .md preview that will render your app content via new Preview in simple browser added in sprint 1, or via markdown.showPreview if there is no custom emd content in the loaded .md doc.

@RandomFractals

  1. How do we detect "custom emd content"?
  2. Is there a particular reason to offer the markdown.showPreview option? If I am working on an evidence project, I think I would always want to see the preview in a browser.

@RandomFractals
Copy link
Contributor Author

RandomFractals commented May 30, 2023

@mcrascal even the template Evidence project has README.md which should show up as Markdown document not Evidence Markdown and the built-in markdown.showPreview command should be available to all .md files independent of the open project. Current emd config hides it for all standard .md docs and needs to be resolved.

image

@RandomFractals
Copy link
Contributor Author

Partial patch for the built-in markdown previews was added in #29.

We should also remove your custom keybdings remapping those built-in commands.

Extensions should not change built-in shortcuts or map built-in commands to new shortcuts. We can only use them as was added in #29.

RandomFractals added a commit that referenced this issue May 30, 2023
…de to the editor title context menu for emd docs (#9 and #29)
@RandomFractals
Copy link
Contributor Author

RandomFractals commented May 30, 2023

The official vscode markdown Preview documentation for more info:

https://code.visualstudio.com/Docs/languages/markdown#_markdown-preview

@RandomFractals
Copy link
Contributor Author

RandomFractals commented May 30, 2023

Closing this ticket. The missing markdown Outline will be added in #33.

@RandomFractals RandomFractals added the markdown Markdown syntax and features support label Jun 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working markdown Markdown syntax and features support review Requires additional testing and review
Projects
None yet
Development

No branches or pull requests

3 participants