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

Unable to render \${...} in code block #902

Closed
yangshun opened this issue Jan 11, 2020 · 7 comments · Fixed by #991
Closed

Unable to render \${...} in code block #902

yangshun opened this issue Jan 11, 2020 · 7 comments · Fixed by #991
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🐛 type/bug This is a problem

Comments

@yangshun
Copy link

yangshun commented Jan 11, 2020

Subject of the issue

Feel free to close if this is not a bug (or bug of some other library).

I'm trying to render the following

```
python command subdir=\${model.nb_layers}
```

but it crashes because MDX tries to evaluate the contents of the ${...}. I suspect some double escaping occurs and the problem (if indeed a problem) lies in this line. Or it could be a remark issue. Not entirely sure which.

I also posted on Spectrum here because I wasn't sure whether this was a bug.

Your environment

  • OS: macOS 10.15.1
  • Packages: @mdx-js/mdx@1.15.1
  • Env: Node 8.16.2, Yarn 1.16.0

Steps to reproduce

Repo here - https://github.com/yangshun/hydra/blame/master/website/docs/configure_hydra/workdir.md#L67-L69

Expected behaviour

Renders fine. It renders ok for GFM.

Actual behaviour

MDX tries to evaluate the contents of the ${...}. I suspect some double escaping occurs.

@yangshun yangshun added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Jan 11, 2020
@ChristianMurphy
Copy link
Member

It is escaping

```
python command subdir=\${model.nb_layers}
```

results in

/* @jsx mdx */
const makeShortcode = name => function MDXDefaultShortcode(props) {
  console.warn("Component " + name + " was not imported, exported, or provided by MDXProvider as global scope")
  return <div {...props}/>
};
const layoutProps = {
  
};
const MDXLayout = "wrapper"
function MDXContent({
  components,
  ...props
}) {
  return <MDXLayout {...layoutProps} {...props} components={components} mdxType="MDXLayout">
    <pre><code parentName="pre" {...{}}>{`python command subdir=\\${model.nb_layers}
`}</code></pre>
    </MDXLayout>;
}
;
MDXContent.isMDXComponent = true;

while

```
python command subdir=${model.nb_layers}
```

results in

/* @jsx mdx */
const makeShortcode = name => function MDXDefaultShortcode(props) {
  console.warn("Component " + name + " was not imported, exported, or provided by MDXProvider as global scope")
  return <div {...props}/>
};
const layoutProps = {
  
};
const MDXLayout = "wrapper"
function MDXContent({
  components,
  ...props
}) {
  return <MDXLayout {...layoutProps} {...props} components={components} mdxType="MDXLayout">
    <pre><code parentName="pre" {...{}}>{`python command subdir=\${model.nb_layers}
`}</code></pre>
    </MDXLayout>;
}
;
MDXContent.isMDXComponent = true;

The question is, should it?
From the ongoing discussion at #628, maybe?
Interpolated content can be interleaved in markdown, this is one edge cases that can happen with interleaving.

@ChristianMurphy ChristianMurphy added 💬 type/discussion This is a request for comments 🗄 area/interface This affects the public interface 🙉 open/needs-info This needs some more info and removed 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Jan 11, 2020
@yangshun
Copy link
Author

@ChristianMurphy to be clear, I want to render the \ also - I also tried \\\${...} but it becomes \\\\${...} and \ escapes the \ in front of the $.

@omry
Copy link

omry commented Jan 12, 2020

@ChristianMurphy, the desired output to show to the user \${model.nb_layers} in a code block.
(Just like GitHub is rendering it now).
The escaping before the $ is a part of the documentation.

@ChristianMurphy ChristianMurphy added 🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on and removed 💬 type/discussion This is a request for comments 🙉 open/needs-info This needs some more info labels Jan 16, 2020
@ChristianMurphy
Copy link
Member

Thanks for the clarification @yangshun and @omry!
That makes sense, PRs are welcome!

@bravo-kernel
Copy link

I placed a $100 bounty for the person whose pull request for fixing this issue gets merged. Add to or claim the bounty here https://www.bountysource.com/issues/86803859-unable-to-render-in-code-block.

@lex111
Copy link
Contributor

lex111 commented Mar 23, 2020

I created PR #991 for fix this. Take a look please.

@bravo-kernel
Copy link

@lex111 awesome and nice to see that you are going after it. As far as the PR goes I cannot judge it so we/you are at the mercy of the mdx crew. I have subscribed to the PR so I will keep an eye on it 🤞

@wooorm wooorm added ⛵️ status/released and removed help wanted 🙏 This could use your insight or help 🙆 yes/confirmed This is confirmed and ready to be worked on labels Jul 23, 2020
@wooorm wooorm added the 💪 phase/solved Post is done label Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🐛 type/bug This is a problem
Development

Successfully merging a pull request may close this issue.

6 participants