feat(gatsby-remark-embed-snippet): Add the ability to embed named snippets

[BobWall23]

Description

Adds a feature to the gatsby-remark-embed-snippet plugin that makes it possible for someone to add inline comments to a source file to delineate one or more named snippets, and to embed snippets by name.

Documentation

The new feature is documented in the README.md for the gatsby-remark-embed-snippet plugin.

Related Issues

[pieh]

This looks awesome!

I do however would want to discuss a bit way in which we specify snippet names to be used. Doing embed:file#SNsnippetName will work but I do wonder if we should try to take hint out of gatsby-remark-prismjs book and instead have something like embed:file{ snippet: "snippetName" } - for users not familiar with #SN<snippetName> syntax I think this will be easier to understand (and also give us room to implement other options/attributes in the future without overloading # (which can be reserved for line numbers)

[pieh]

Another thing is about marking snippets - using them with comments is fine, i just think we should use conventions we already have for line highlighting - so (potentially) // START SNIPPET bar -> // start-snippet{bar} - this is just so API is consistent and parts of it feel like they go well together.

Let me know what do you think

[BobWall23]

Thanks for the suggestions - I think it is good to make this fit in better. I didn't look far enough outside this plugin to see how some of the others worked. I will update to match the prismjs syntax better, using the syntax you suggested.

[pieh]

Those were just examples - we might end up with them, but I'd like to encourage you to take a peek into what syntax conventions are used currently and maybe see if you can come up with better ones. You could take a look in both in gatsby-remark-embed-snippet and gatsby-remark-prismjs (because those 2 are pretty close together and both are used for similar results - showing some code blocks).

[BobWall23]

Those suggestions do seem to be in line with the syntax used in the prismjs plugin. The only option syntax in the embed-snippet plugin is the #L parsing.

I think it is pretty unfortunate that prismjs used {optionName : value} instead of {"optionName" : value}, to facilitate standard JSON parsing. I can continue with similar syntax (no double quotes around the option names) or use standard JSON conventions. I prefer the latter, in spite of the slight mismatch in syntax.

[BobWall23]

I went with a change that doesn't require double quotes around the snippet keyword - it would be a trivial change plus update to the docs to switch over to valid JSON format. I think that would be more robust if new options were added.

I wonder if I blew the commits by not using the conventional commit standard? I was thinking I just needed to worry about the pull request, not the individual commits to my fork. Hopefully that is correct.

[pieh]

I wonder if I blew the commits by not using the conventional commit standard? I was thinking I just needed to worry about the pull request, not the individual commits to my fork. Hopefully that is correct.

This is correct - we are "squashing and merging", so single commit will end up in master - and if there is more than 1 commit in PR - PR title is used

[pieh]

minor inconsistencies to fix up and one extra question

[BobWall23]

I notice that the lint check is failing due to an extra space in docs/blog/2020-05-22-happy-fifth-bday-gatsby/index.md. If I fix that, the lint passes - should I commit that change into this PR?

[pieh]

I notice that the lint check is failing due to an extra space in docs/blog/2020-05-22-happy-fifth-bday-gatsby/index.md. If I fix that, the lint passes - should I commit that change into this PR?

You can merge master branch of gatsbyjs/gatsby into your branch and push it (lint was fixed in master already).

Assuming origin = gatsbyjs/gatsby:

git fetch origin master
git merge origin/master
git push <your_fork> packages/add-named-snippets

should fix lint problems

Alternatively - you can go ahead and fix the lint separately in your branch - because it was fixed already in master - this change should not show in PR view

[pieh]

All right, code has sign off from me - it looks great and tests are great! Now just need to review for docs changes

comments were addressed

[Ekwuno]

Thanks for sending this in. I left a few comments.

[AishaBlake]

This looks good! Thank you for your patience, @BobWall23. And thanks to the other team members who have already reviewed! I've fixed a couple of merge conflicts and will merge this once tests pass.

notes addressed

[AishaBlake]

Note: this says that we still need a Code owner review from the core team but pieh signed off on this a while back!

[gatsbot]

Holy buckets, @BobWall23 — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!