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

Fix myst sphinx warnings #3035

Merged
merged 21 commits into from
Feb 6, 2022
Merged

Conversation

stevepiercy
Copy link
Collaborator

@stevepiercy stevepiercy commented Feb 5, 2022

This PR uses the branch add-myst-sphinx-config to build the docs for testing. This PR should be merged into #3032 before the other is merged into plone6-docs

Remove comments that are obsolete
Add html_meta values
Add html_meta values
Remove redundant directory listing
Add html_meta values
Remove `./`
Fix syntax highlighting and emphasis
Fix broken references
Add html_meta values
Add html_meta values
@stevepiercy
Copy link
Collaborator Author

Quick syntax question for y'all. I'm working on fixing the jsx Sphinx highlighting, and the parser is super picky. This does not parse:

<SidebarPortal selected={this.props.selected}>
  <InlineForm
    icon={<Icon size="24px" name={nameSVG} />}
    schema={schema}
    title={schema.title}
    headerActions={<button onClick={() => {}}>Action</button>}
    footer={<div>I'm footer</div>}
    onChangeField={(id, value) => {
      this.props.onChangeBlock(this.props.block, {
        ...this.props.data,
        [id]: value,
      });
    }}
    formData={this.props.data}
  />
</SidebarPortal>;

...but if I quote the HTML values, then it does parse:

<SidebarPortal selected={this.props.selected}>
  <InlineForm
    icon={"<Icon size="24px" name={nameSVG} />"}
    schema={schema}
    title={schema.title}
    headerActions={"<button onClick={() => {}}>Action</button>"}
    footer={"<div>I'm footer</div>"}
    onChangeField={(id, value) => {
      this.props.onChangeBlock(this.props.block, {
        ...this.props.data,
        [id]: value,
      });
    }}
    formData={this.props.data}
  />
</SidebarPortal>;

My question is whether the latter form is valid syntax when actually used as code? It would be great if someone could run a quick test for me, perhaps a simple return <h1>hello</h1> and return "<h1>hello</h1>" to compare?

To go along with this, we need to make a choice about sytnax highlighting for jsx.

  1. Quote HTML values
  2. Leave as is, and ignore lack of syntax highlighting for many jsx code blocks
  3. Submit a PR to fix the parser

Add html_meta values
Add html_meta values
Add html_meta values
Minor grammar fixes
Add html_meta values
Remove unused lexer parameter
Add html_meta values
Add html_meta values
Add html_meta values
Add html_meta values
Add html_meta values
Fix reference to openobjectbrowser-handler-api-label
Add html_meta values
@stevepiercy stevepiercy marked this pull request as ready for review February 5, 2022 10:41
@tiberiuichim
Copy link
Contributor

@stevepiercy no, something={"myfunc()"} binds the something parameter to a string, it doesn't execute myfunc(). Maybe we can use simple js syntax highlighting? Would it work? IMHO we could even live without highlighting, if there's no other way.

@stevepiercy
Copy link
Collaborator Author

I reverted those incorrect "fixes".

I opened an issue here #3038 and fcurella/jsx-lexer#15

js as the lexer also fails.

We can do without syntax highlighting for now, but someone who understands the lexer and jsx syntax should submit a PR to the project.

@stevepiercy
Copy link
Collaborator Author

It turns out that jsx-lexer does not like ' within HTML tags. sheesh!

@stevepiercy
Copy link
Collaborator Author

This PR is now ready for final review. Any takers?

@stevepiercy stevepiercy merged commit 1ea02f4 into add-myst-sphinx-config Feb 6, 2022
@stevepiercy stevepiercy deleted the fix-myst-sphinx-warnings branch February 6, 2022 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants