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

New Code Snippet Syntax and features with tests #1336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fabioaanthony
Copy link
Contributor

Summary
Code Import Snippet v2 Proposal

The motivation for this proposal is based on a prior PR (mine) #1313, along with community discussions #538 (comment), #1189

Feature
Added support for:

  • Specify your language
  • Transclusion
  • Line highlighting

New syntax examples (inspired by #538 (comment))

@[code](@/docs/code.js)
@[code lang=ruby transclude={1-1}](@/docs/code.rb)
@[code highlight={1-6} transcludeTag=style](@/docs/code.vue)
@[code highlight={4,9,11-16} transcludeWith=:::](@/docs/code.vue)

Bugfix
Along with the new features, I also fixed the tests which were providing false positives because the snapshots were just recording "File not found".

@yyx990803 @znck @ulivz this issue has been discussed for almost year without any movement which is why I just made the PR. Hopefully it is well received by you all and the community.

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:

If changing the UI of default theme, please provide the before/after screenshot:

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

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The old code snippet import syntax would need to be updated unless this is added as second more advanced way of importing code snippets. Also documentation would need to updated (which I'm willing to do if the PR is merged).

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number) please see numerous examples above^

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

  • Chrome
  • Firefox
  • 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 (no but willing to update if merged)
  • 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:

@fabioaanthony
Copy link
Contributor Author

\Update

I decided to make this a markdown-it plugin for now because it seems like you guys aren't ready to settle on a syntax for this just yet (no pressure). Also I know that the syntax change would require a significant amount of refactoring. Having this as plugin allows you to use both Code Snippet syntaxs side by side.

If there's high demand for the plugin then reconsider the PR later (I'll leave it open). I'll share any feedback I get and I'll make upgrades to the plugin based on personal and community needs.

If you're interested, you can grab it here: https://www.npmjs.com/package/markdown-it-vuepress-code-snippet-enhanced

Cheers!

P.S. Vue is awesome, please continue the great work!

@greghroberts
Copy link

Hey @fabbballin . I just ran into this same issue. Your package looks great. +1 from me to add better support to VuePress . Weird that you can't select parts of a file (line numbers at least). The main reason I see for this is to reference parts of your source quickly. Not having the right formatting is a non-starter too. I'll go with your package, but didn't see GitHub link. You plan on integrating or keeping the external plugin?

@fabioaanthony
Copy link
Contributor Author

fabioaanthony commented Mar 6, 2019

Hi @greghroberts thanks for the feedback! It will remain as an external plugin for now and may be included into vuepress down the line. You can see the source here
https://github.com/fabbballin/markdown-it-vuepress-code-snippet-enhanced

@timaschew
Copy link
Contributor

@fabbballin your plugin is awesome!

But I didn't work for me because I was running this command:

node_modules/.bin/vuepress dev vuepressed/

And this line in your plugin: const _root = options && options.root ? options.root : process.cwd() doesn't work. I know I can pass the root, but it should not be required to pass it when I invoke the CLI the way it's designed.
I suggest to fallback to process.argv[3] which is the correct directory. It seems to be not super safe for future, so @ulivz what do you think to expose the targetDir somehow?

@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

i will have a look about it and then push to others thanks @fabbballin and sorry if it take time we just created the core team. It will take a bit time for us to have everything in mind

@flozero flozero self-assigned this Sep 5, 2019
@flozero flozero added complexity: medium Medium complexity effort: low Around a day or less status: core team assigned Core team member assigned topic: markdown Relates to VuePress markdown type: enhancement Request to enhance an existing feature version: 1.x Relates to version 1 of VuePress labels Sep 5, 2019
@meteorlxy
Copy link
Member

meteorlxy commented Nov 28, 2019

Hey, just notice this PR.

Seems that this PR totally replaced current code snippet syntax, which introducing a breaking change.

It would be OK in alpha stage at that time, but as we have bumped to a stable version, merging this PR means a major version bump. So it's better to keep compatible with current syntax.

BTW, what if extracting this feature into a VuePress Plugin (which could be a wrapper of your markdown-it plugin)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: medium Medium complexity effort: low Around a day or less status: core team assigned Core team member assigned topic: markdown Relates to VuePress markdown type: enhancement Request to enhance an existing feature version: 1.x Relates to version 1 of VuePress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants