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

feat(markdown): snippet partial import #2225

Merged
merged 3 commits into from
May 8, 2020

Conversation

nweldev
Copy link
Contributor

@nweldev nweldev commented Mar 17, 2020

Only import a portion of a code file delimited by VS Code custom regions markers, based on the region key you set via a fragment identifier (#):

/docs/hello-world.md

# Hello World

<<< @/src/hello-world.js#function-def{1-2}

/src/hello-world.js:

// #region function-def
function helloWorld () {
  // ..
}
// #endregion function-def

export default helloWorld

Motivation

Contents presented with Vuepress (documentation, tutorials, etc.) may (often?) be related to some "real" projects. Ensuring the code snippets in the doc and the "real" code is very important for this use case. Yet, it can be difficult, or even impossible, to split each code sample in a dedicated file. Therefore, some misleading code lines may be shown in the code snippet only because the author needs it in the project.

For example, if I want to show a function in the doc, but I need an export in order to use this function in my project, I would have to show it in the code snippet:

function foo () {
  // ...
}

export default foo;

Solution

This solution add support for:

  • VS Code custom region markers
    • clearly indicate a region to import
    • why: a lot of web developers (and therefore, I guess, Vuepress users) use VS Code and should be familiar with this syntax
  • fragment identifier (path#region)
    • indicate the name of the custom region to import
    • why: same logic as the one defined in the URL standard (see W3C doc)

For more details, see the new documentation and tests.

Compatibility

Does not introduce any breaking change: result stay exactly the same if no fragment identifier is set (no region name is set by default).
Yet, the new syntax isn't backward compatible: adding a fragment identifier to the path will lead to a "cannot find path @/path/file.ext#region" with the current version of Vuepress.

If you want to use this feature before this PR is merged, you can use patch-package as follow:

  1. download patches.zip
  2. extract this zip file in your project root (should create patches/@vuepress+markdown+1.3.1.patch)
  3. add patch-package to your dev dependencies:
    npm i -D patch-package
    
    or
    yarn add --dev patch-package postinstall-postinstall
    
  4. add patch-package in your postinstall script
    "scripts": {
    +  "postinstall": "patch-package"
    }
    

Summary

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

The PR fulfills these requirements:

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome 80.0.3987.132 Linux
  • Firefox 74 Linux
  • Safari
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information

This feature is already used on a daily basis for a pretty heavy (but private) project, alongside a related Pandoc filter which provides this feature: pandoc-import-code.

@nweldev nweldev force-pushed the md-snippet-region branch from a90b2a2 to 037aa14 Compare March 17, 2020 10:04
@kefranabg
Copy link
Collaborator

Hi @noelmace, thanks for opening a PR. I discussed with the team and that's definitely something we could add in the next release 👍 I'll do a review ASAP

nlm-pro added 2 commits March 24, 2020 00:25
Only import a code file region (VS Code `#region <name>` comments,
instead of the whole file content) using a new `#region` parameter:
`<<< @/path/file.ext#region{1-2}`
permit nesting regions for partial code snippets import
@nweldev nweldev force-pushed the md-snippet-region branch from 6f58672 to da909e8 Compare March 23, 2020 23:50
Copy link
Collaborator

@kefranabg kefranabg left a comment

Choose a reason for hiding this comment

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

Hi @noelmace, line highlighting seems not to work as expected when using a line number > 10.

<<< @/docs/.vuepress/config.js{11} results in:

Capture d’écran 2020-03-28 à 10 57 18

Could you fix this and add a new test to cover this case? Then I'll continue the review! Thanks for your time 👍

A bad regexp was introducing a regression, as only one digit was
accepted for code highlight. This error permitted to identify that when
an invalid path is given, "src" falls down to CWD. This was leading to
an ESDIR "invalid operation on a directory" error.
@nweldev
Copy link
Contributor Author

nweldev commented Mar 31, 2020

Thx @kefranabg. I fixed this in the last commit and added a new test as requested.

@nweldev nweldev requested a review from kefranabg April 5, 2020 10:37
nweldev referenced this pull request in nlm-pro/pandoc-import-code Apr 5, 2020
Copy link
Collaborator

@kefranabg kefranabg left a comment

Choose a reason for hiding this comment

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

LGTM!

@wll8
Copy link

wll8 commented Nov 11, 2020

The @ symbol is always relative to the directory where the command is executed. And @alias cannot be used

nweldev referenced this pull request in nlm-pro/pandoc-import-code Aug 6, 2023
nweldev referenced this pull request in nlm-pro/pandoc-import-code Aug 6, 2023
nlm-pro referenced this pull request in nlm-pro/pandoc-import-code Aug 20, 2023
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