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: Add support to import files as code fence #538

Merged
merged 3 commits into from
Jun 2, 2018

Conversation

znck
Copy link
Member

@znck znck commented Jun 1, 2018

Add rule to import code snippets:

...
## Minimal

rollup-plugin-vue ships as zero config solution to package `.vue` files.

<<< @/cookbook/minimal/rollup.config.js{1,10}

screenshot-2018-6-2 rollup plugin for vue cookbook

@@ -0,0 +1,43 @@
const fs = require('fs')

module.exports = function codeFrame(md, options = {}) {
Copy link
Member

Choose a reason for hiding this comment

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

snippet?

Copy link
Member

@ulivz ulivz left a comment

Choose a reason for hiding this comment

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

LGTM, would you please also add the tests for that?

return true
}

md.block.ruler.before('fence', 'code-frame', parser)
Copy link
Member

Choose a reason for hiding this comment

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

same here

@znck znck force-pushed the feat/import-code-snippets branch from 29978bf to b73bdc5 Compare June 1, 2018 21:46
@ulivz
Copy link
Member

ulivz commented Jun 2, 2018

Final thing, could you also document for this feature? thanks!

@znck
Copy link
Member Author

znck commented Jun 2, 2018

I feel it should support importing given lines from large a file but that can be a different PR.

@ulivz
Copy link
Member

ulivz commented Jun 2, 2018

Awesome! welcome to open any PRs for good stuffs like this.

@ulivz ulivz merged commit 26ecff7 into vuejs:master Jun 2, 2018
@znck znck deleted the feat/import-code-snippets branch June 2, 2018 15:46
@octref
Copy link
Member

octref commented Jun 2, 2018

I wish we could do a syntax more inline with markdown and Vue loaders, for example:

# My Config

<code src="@/cookbook/minimal/rollup.config.js"></code>
<code src="@/cookbook/minimal/rollup.config.js" start="1" end="10"></code>

This is both valid markdown, and benefits include:

  • Editor can support syntax highlight easily
  • Editor might be able to linkify the src and offer jump-to-definition
  • The code is now semantic. <<< is much less straight-forward and readable than <code>
  • Following spec is always a good thing.

@ulivz
Copy link
Member

ulivz commented Jun 2, 2018

@octref Looks good to me, but for now there are some restrictions for us to implement above syntax:

Since our md's process pipeline is markdown loader => vue loader, and markdown loader is responsible for highlight and other code-related transpliation, so all code must be included before it was thrown to Vue Loader.

So for the above grammar, we need to implement a simple XML parser at markdown's side which looks too complicated.

I just have an idea, which is simple, semantic enough and also offer jump-to-definition:

[code{1-2}](../app.js)

@octref
Copy link
Member

octref commented Jun 2, 2018

@ulivz That's actually quite a nice idea. I was about to suggest https://github.com/iainc/Markdown-Content-Blocks which would solve the problem of file transcultion in general.

One problem is code there actually don't mean anything. The other problem (also with the original proposal) is that {1-2} or {1, 10} is very misleading...I thought it was open/end but it was actually lines that need highlight. But I don't have a good idea now either.

ulivz added a commit that referenced this pull request Jun 2, 2018
@znck
Copy link
Member Author

znck commented Jun 3, 2018

@octref's concerns are valid. Let's give this problem another shot.

What do we need:

  • Transclusion (full or partial)
  • Line highlighting
  • Performed statically & within scope of markdown loader

@yyx990803 Any suggestions here?


https://talk.commonmark.org/t/transclusion-or-including-sub-documents-for-reuse/270
(It's listed on proposed commonmark extensions)

@znck
Copy link
Member Author

znck commented Jun 5, 2018

Something like https://github.com/posva/markdown-it-custom-block could be better here.

:[code](../somefile.js)
:[code {1,4}](../somefile.js)
:[code js{1,4}](../somefile.js)
:[code js{1,4}](../somefile.js "1-10") <!-- Partial Transclusion -->

It would support explicit language, line highlighting and with click jumping editor support.

@octref WDYT?

@ulivz
Copy link
Member

ulivz commented Jun 5, 2018

@znck I like it. but how about this syntax for partial transclusion?

@[code{1-10} js{1,4}](../somefile.js)

It prevent the links from being changed and still offer jump-to-definition.

@znck
Copy link
Member Author

znck commented Jun 5, 2018

I'll send a PR by EOD.

@octref
Copy link
Member

octref commented Jun 5, 2018

I think the highlight line syntax need more thought. Can you do things like {2-5,8-10} for ranges?
On the other hand I wouldn’t find partial transclution very useful or ergonomic. When I go to the transcluded file I don’t know which part is included without looking at the importing file. Also if I only want line 2-5 why don’t I delete line 1? If Line 1 is necessary why not just highlight line 2-5?

@znck
Copy link
Member Author

znck commented Jun 5, 2018

If you have a large file and you want to import only couple of lines. Example importing only script from SFC.

@octref
Copy link
Member

octref commented Jun 5, 2018

Well you can also remove other lines in the source file, if they are not relevant.
Maybe you are thinking in one project, you can import in /doc/index.md something from /src/Component.vue, etc. But those are very implicit dependencies and I don't think that's a real need.

@daggerok
Copy link

daggerok commented Sep 10, 2019

Hey, so can anybody tell me if some time in a future we will have possibility to include partially code snippets or it wont be possible for some religion reasons?

Write really good documentation which is updating together with code is very important. let's say I have some class and important business method in it. So It would be awesome to tell vuepress include code only (for example) from comment with start-lable to comment with ending label.
Asciidoctor has that possibility and it's very handy. Here example how I can include only interested code range from file (snippet-1 label) right from real source code file (not from it's copy, snippet for documentation needs):

codeWontBeIncluded();
// tag::snippet-1
myBusinessMethod();
myOtherImportantCode();
// end::snippet-1
otherCodeWontBeIncluded();

so If I will add some more important code in between myBusinessMethod() and myOtherImportantCode() my documentation will be successfully updated automatically. But if I will be using separate snippet-files I can even forget about it and documentation will be inconsistent and developers should do more effort to keep documentation updated. As you can see, @octref it's not useless case

Guys, please add such possibility, we really needed it
for now we are using this awesome md enhancer: #1336 (comment) but it's really what we need

Thanks


Regards,
Maksim

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.

5 participants