plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
486 stars 663 forks source link

Improve jsx syntax highlighting in MyST/Sphinx/Pygments #3038

Open stevepiercy opened 2 years ago

stevepiercy commented 2 years ago

When I attempt to highlight the following code snippet using Sphinx, MyST, and jsx-lexer, Sphinx returns a warning:

WARNING: Could not lex literal_block as "jsx". Highlighting skipped.

```jsx
import React from 'react';

const View = props => {
  return <div>I'm the Block view component!</div>;
};

export default View;

Here is how it should lex, as demonstrated in GitHub.

```jsx
import React from 'react';

const View = props => {
  return <div>I'm the Block view component!</div>;
};

export default View;

FWIW, if I try js as the lexer, it also does not lex.

We need to make a choice about sytnax highlighting for jsx.

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

Originally posted by @stevepiercy in https://github.com/plone/volto/issues/3035#issuecomment-1030588839

Filed ticket upstream on 2024-11-07 in https://github.com/pygments/pygments/issues/2816.

stevepiercy commented 2 years ago

Here's another one. If I remove aria-label="Cancel", then it gets lexed.

```jsx
<button className="cancel" aria-label="Cancel" onClick={() => this.onCancel()}>
  <Icon
    name={clearSVG}
    className="circled"
    size="30px"
    title={this.props.intl.formatMessage(messages.cancel)}
  />
</button>
tiberiuichim commented 2 years ago

@stevepiercy that's because it's broken syntax. In react the kebab-case becomes camelCase, so it should be ariaLabel="Cancel"

stevepiercy commented 2 years ago

@tiberiuichim thank you! Fixed in #3077.

sneridagh commented 2 years ago

I think not... in fact I had problems in the past if used the CamelCase:

https://reactjs.org/docs/accessibility.html#wai-aria

tiberiuichim commented 2 years ago

@sneridagh ok, so it's not broken syntax, just the lexer being unaware of things. Ok, TIL

stevepiercy commented 2 years ago

A new release of jsx-lexer might have fixed this issue. I updated the lexer to lex aria-* attributes.

https://github.com/fcurella/jsx-lexer/releases/tag/v2.0.0

stevepiercy commented 2 years ago

I need to double-check for strings that contain '.

stevepiercy commented 2 years ago

I gave it a shot to check for ' as described at the top of the issue, but I have no idea how to modify the lexer to accept it in this context.