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 gatsby-remark-embed-snippet plug-in #3012

Merged
merged 16 commits into from
Nov 28, 2017

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Nov 24, 2017

For a more detailed explanation of this plug-in and its options, refer to the README.

Given a file like this (eg examples/hello-world.js):

// highlight-range{1,4}
ReactDOM.render(
  <h1>Hello, world!</h1>,
  document.getElementById('root')
);

And a markdown snippet like this:

Here is some embedded code:
`embed:hello-world.js`

Create HTML like this:
screen shot 2017-11-23 at 4 26 18 pm

This PR is related to ReactJS.org issue reactjs/react.dev/issues/310 and PR reactjs/react.dev/pull/331

Follow-up Work

I think the highlight marker comments from this PR would be a nice addition to the gatsby-remark-prismjs plugin as well. Defining ranges at the top of the markdown comment is kind of painful, especially for larger blocks or edits. Ideally this plug-in could be simplified to just read files from disk and pass their contents along to a method shared with that plug-in to convert to Prism blocks.

I'd be willing to make this refactor as a follow-up PR if you'd be interested?

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 24, 2017

I kind of hate that "Strings must use backtick quotes" lint rule 😁

@KyleAMathews
Copy link
Contributor

Deploy preview failed.

Built with commit bded111

https://app.netlify.com/sites/using-glamor/deploys/5a17652ba1147749ece5b65f

@gatsbybot
Copy link
Collaborator

gatsbybot commented Nov 24, 2017

Deploy preview ready!

Built with commit 91d0d73

https://deploy-preview-3012--gatsbygram.netlify.com

@gatsbybot
Copy link
Collaborator

gatsbybot commented Nov 24, 2017

Deploy preview ready!

Built with commit 91d0d73

https://deploy-preview-3012--using-drupal.netlify.com

@KyleAMathews
Copy link
Contributor

KyleAMathews commented Nov 24, 2017

Deploy preview failed.

Built with commit aad2f3b

https://app.netlify.com/sites/using-glamor/deploys/5a185c4aa114770604e5b6e7

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 24, 2017

I'm going to add range support here momentarily.

Done. The plug-in now supports highlight-range{...} comments similar to the gatsby-remark-prismjs plugin.

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 25, 2017

I left a note about possible follow-up work on the PR. If you're interested in merging that, let me know and I'll be happy to submit another PR.

If you're not interested, that's okay too. Let me know and I'll just roll my own plug-in for use in reactjs.org 😄

@KyleAMathews
Copy link
Contributor

I kind of hate that "Strings must use backtick quotes" lint rule

This is the best rule ever :-D

const visit = require(`unist-util-visit`)

// HACK: It would be nice to find a better way to share this utility code.
const highlightCode = require(`gatsby-remark-prismjs/highlight-code`)
Copy link
Contributor

Choose a reason for hiding this comment

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

What we've been doing is to just split it out into a new plugin.

@KyleAMathews
Copy link
Contributor

I think the highlight marker comments from this PR would be a nice addition to the gatsby-remark-prismjs plugin as well. Defining ranges at the top of the markdown comment is kind of painful, especially for larger blocks or edits. Ideally this plug-in could be simplified to just read files from disk and pass their contents along to a method shared with that plug-in to convert to Prism blocks.

100% agree! It's super obnoxious haha :-) Inline highlight comments are way better. And yeah, allowing code re-use is always very nice too.

If this is ready to go, I'll merge/publish and wait looking forward to your follow-up PR :-)

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 27, 2017

If this is ready to go, I'll merge/publish and wait looking forward to your follow-up PR :-)

This should be ready to go, yes. I've got a branch of the reactjs.org docs built using this plug-in just to test it out. 😄

I'll send a follow-up PR sometime this week.

@KyleAMathews
Copy link
Contributor

spyOn is a Jest global? It's breaking lint test — so should be added to our .eslintrc globals.

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 27, 2017

spyOn is a Jest global? It's breaking lint test — so should be added to our .eslintrc globals.

Yup it's a global. It's also used here (although that's my fault too I guess 😁 )

@KyleAMathews
Copy link
Contributor

Huh, weird the other place doesn't fail the lint too.

@bvaughn bvaughn changed the title [WIP] New gatsby-remark-embed-snippet plug-in New gatsby-remark-embed-snippet plug-in Nov 28, 2017
@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 28, 2017

Huh, weird the other place doesn't fail the lint too.

Agreed!

I'll add an entry to the ESLint config though.

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 28, 2017

I believe it should be good to go now. yarn lint passes locally at least.

@KyleAMathews
Copy link
Contributor

Cool! This is awesome stuff, looking forward to seeing it rolled out on reactjs.org!

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