remarkjs / remark-github

remark plugin to link references to commits, issues, pull-requests, and users, like on GitHub
https://remark.js.org
MIT License
179 stars 22 forks source link

Noop if earlier plugin adds new nodes to tree #19

Closed vweevers closed 5 years ago

vweevers commented 5 years ago

Subject of the issue

Because remark-github extends the remark Parser to do its work, it doesn't process nodes that were added by an earlier plugin (after parsing). For example:

remark()
  .use(require('remark-changelog'), { fix: true })
  .use(require('remark-github'))

Is there something that other plugins can do to get their new (text) nodes processed by the parser and thus remark-github?

Your environment

wooorm commented 5 years ago

What kind of syntax / nodes are you talking about?

vweevers commented 5 years ago

Any new node (or an entirely new tree) that contains text like #23 and the other supported references.

wooorm commented 5 years ago

Right! Is that text coming from git commit messages? Could text also include other Markdown syntax? Such as: Fix `inlineCode` handling?

vweevers commented 5 years ago

Is that text coming from git commit messages?

Not always. I also detect merge commits that have the message "Merge pull request #n" and add that PR number to commits. I chose not to insert links to GitHub myself, because I figured remark-github could do that.

Could text also include other Markdown syntax

That hadn't crossed my mind! But parsing that (not sure I want to) would be a separate feature, not one that's related to remark-github.

wooorm commented 5 years ago

remark-github does two things: it parses stuff, and it transforms stuff. The parsing is for things like #2, the transforms are for links. (such as if you link to a commit, https://github.com/remarkjs/remark-github/commit/3cf0e0b4a4046c5cba1f67a55b53c5e762751816, it’ll change the link content: https://github.com/remarkjs/remark-github/commit/3cf0e0b4a4046c5cba1f67a55b53c5e762751816). It’s a mix and it’ll be separated better in the future with micromark.

That hadn't crossed my mind! But parsing that (not sure I want to) would be a separate feature, not one that's related to remark-github.

Where I’m going with the above questions is that see them as exactly the same thing. I see the parsing extensions in this package as syntax extensions, they’re a separate parsing step. If someone were to inject *foo* in the syntax tree, it would output *foo* instead of foo, doing that with the syntax supported in this plugin is handled the same. We can’t keep on parsing again and again on every change.

If you want to inject syntax into the tree, you need to create nodes yourself. E.g., with unified.use(remarkParse).use(remarkGitHub)!

vweevers commented 5 years ago

I see the parsing extensions in this package as syntax extensions

I see. It's a bit painful in practice though, because any plugin that creates nodes will itself have to include remark-parse and remark-github, which breaks the single responsibility principle. Could a plugin perhaps use this.Parser somehow?

This is now more of a support request than bug report, so it's OK to close, I'll figure something out!

wooorm commented 5 years ago

I see. It's a bit painful in practice though, because any plugin that creates nodes will itself have to include remark-parse and remark-github, which breaks the single responsibility principle. Could a plugin perhaps use this.Parser somehow?

Yeah I see that. A similar example is remark-usage, which parses comments in an example.js file, parses them as Markdown, and embeds them in the processed file. Previously, we used this.Parser for that, indeed. However there are cases where that doesn’t work:

An alternative example is that you write a module M that uses Babel to parse JS. Someone uses that in their project P. Do you pick up on the config that the user has in their project P? Maybe they use JSX, but you don’t support that.