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

(jsx) multiple self-closing tags with attributes highlighting broken #1915

Closed
leafOfTree opened this issue Nov 28, 2018 · 19 comments · Fixed by #2322
Closed

(jsx) multiple self-closing tags with attributes highlighting broken #1915

leafOfTree opened this issue Nov 28, 2018 · 19 comments · Fixed by #2322
Labels
bug help welcome Could use help from community

Comments

@leafOfTree
Copy link

leafOfTree commented Nov 28, 2018

Given jsx code:

class App extends Component {
  render() {
    return (
      <BrowserRouter>
        <div>
          <Route path="/about" component={About} />
          <Route path="/contact" component={Contact} />
        </div>
      </BrowserRouter>
    );
  }
}

2018-11-28_114855

It seems that the XML code is not highlighted because of the second Route tag with attributes.
When I tried to modify src/language/javascript.js

          { // E4X / JSX
            begin: /</, end: /(\/\w+|\w+\/)>/,
            subLanguage: 'xml',
            contains: [
              {begin: /<\w+\s*\/>/, skip: true},
              {
-               begin: /<\w+/, end: /(\/\w+|\w+\/)>/, skip: true,
+               begin: /<\w+/, end: /(\/\w+[^>]*|\w+\S*\s*\/)>/, skip: true,

                contains: [
                  {begin: /<\w+\s*\/>/, skip: true},
                  'self'
                ]
              }
            ]
          }

The highlighting works as expected.

2018-11-28_115530

@JeromeLin
Copy link

JeromeLin commented Dec 26, 2018

@leafOfTree use your config have wrong still. e.g

import { Radio, Cell } from 'zarm';

class Demo extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      radio: '0',
    };
  }

  render() {
    return (
      <div>
        <Cell
          description={
            <Radio.Group
              type="button"
              value={this.state.radio}
              onChange={value => console.log(`radio to ${value}`)}
            >
              <Radio value="0">选项一</Radio>
              <Radio value="1">选项二</Radio>
              <Radio value="2">选项三</Radio>
            </Radio.Group>
          }
        >
          普通
        </Cell>

        <Cell
          description={
            <Radio.Group type="button" defaultValue="1">
              <Radio value="0">选项一</Radio>
              <Radio value="1">选项二</Radio>
              <Radio value="2">选项三</Radio>
            </Radio.Group>
          }
        >
          指定默认值
        </Cell>

        <Cell
          description={
            <Radio.Group type="button">
              <Radio value="0">选项一</Radio>
              <Radio value="1">选项二</Radio>
              <Radio value="2" disabled>选项三</Radio>
            </Radio.Group>
          }
        >
          禁用指定项
        </Cell>

        <Cell
          description={
            <Radio.Group type="button" shape="radius">
              <Radio value="0">选项一</Radio>
              <Radio value="1">选项二</Radio>
              <Radio value="2">选项三</Radio>
            </Radio.Group>
          }
        >
          圆角
        </Cell>

        <Cell
          description={
            <Radio.Group type="button" shape="round">
              <Radio value="0">选项一</Radio>
              <Radio value="1">选项二</Radio>
              <Radio value="2">选项三</Radio>
            </Radio.Group>
          }
        >
          椭圆角
        </Cell>
      </div>
    )
  }
}

ReactDOM.render(<Demo />, mountNode);

wechatimg1174

@leafOfTree
Copy link
Author

leafOfTree commented Dec 27, 2018

@JeromeLin Regexp needs to be updated:

          { // E4X / JSX
            begin: /</, end: /(\/\w+|\w+\/)>/,
            subLanguage: 'xml',
            contains: [
              {begin: /<\w+\s*\/>/, skip: true},
              {
-               begin: /<\w+/, end: /(\/\w+|\w+\/)>/, skip: true,
-               begin: /<\w+/, end: /(\/\w+|\w+\S*\s*\/)>/, skip: true,
+               begin: /<\w+/, end: /(\/\w+\S*|\w+\S*\s*\/)>/, skip: true,

                contains: [
                  {begin: /<\w+\s*\/>/, skip: true},
                  'self'
                ]
              }
            ]
          }

Then the code is highlighted correctly:
2018-12-27_115922

@JeromeLin
Copy link

JeromeLin commented Dec 28, 2018

@leafOfTree
image

@leafOfTree
Copy link
Author

@JeromeLin

          { // E4X / JSX
            begin: /</, end: /(\/\w+|\w+\/)>/,
            subLanguage: 'xml',
            contains: [
              {begin: /<\w+\s*\/>/, skip: true},
              {
-               begin: /<\w+/, end: /(\/\w+|\w+\/)>/, skip: true,
+               begin: /<\w+/, end: /(\/\w+[^>]*|\w+\S*\s*\/)>/, skip: true,

                contains: [
                  {begin: /<\w+\s*\/>/, skip: true},
                  'self'
                ]
              }
            ]
          }

@JeromeLin
Copy link

JeromeLin commented Jan 2, 2019

@leafOfTree

1546411817617

import { SwipeAction, Button, Cell } from 'zarm';

class Demo extends React.Component {
  render() {
    return (
      <div>
        <SwipeAction
          right={[
            <Button size="lg" theme="primary" onClick={() => console.log('右按钮1')}>右按钮1</Button>,
            <Button size="lg" theme="error" onClick={() => console.log('右按钮2')}>右按钮2</Button>,
          ]}
        >
          <Cell>左滑看看</Cell>
        </SwipeAction>

        <SwipeAction
          left={[
            <Button size="lg" theme="primary" onClick={() => console.log('左按钮1')}>左按钮1</Button>,
            <Button size="lg" theme="error" onClick={() => console.log('左按钮2')}>左按钮2</Button>,
          ]}
        >
          <Cell>右滑看看</Cell>
        </SwipeAction>

        <SwipeAction
          autoClose
          left={[
            <Button size="lg" theme="primary" onClick={() => console.log('左按钮1')}>左按钮1</Button>,
            <Button size="lg" theme="warning" onClick={() => console.log('左按钮2')}>左按钮2</Button>,
          ]}
          right={[
            <Button size="lg" theme="error" onClick={() => console.log('右按钮1')}>右按钮2</Button>,
          ]}
          onOpen={() => console.log('open')}
          onClose={() => console.log('close')}
        >
          <Cell>左右都能滑动(自动关闭)</Cell>
        </SwipeAction>
      </div>
    )
  }
}

ReactDOM.render(<Demo />, mountNode);

@leafOfTree
Copy link
Author

@JeromeLin JSX attr is handled by XML syntax, so src/language/xml.js can be modified like below to get JSX attr expression syntax highlight

  var TAG_INTERNALS = {
    endsWithParent: true,
    illegal: /</,
    relevance: 0,
    contains: [
+      {
+        className: 'jsx-attr',
+        begin: /={/,
+        end: /}/,
+        subLanguage: 'xml',
+      },
      {
        className: 'attr',
        begin: XML_IDENT_RE,
        relevance: 0
      },

2019-01-08_144027

@joshgoebel
Copy link
Member

I just commented over in the other thread:

If someone wants to take a swing at a PR you want to focus on:

https://github.com/highlightjs/highlight.js/blob/master/src/languages/javascript.js#L155

The reuse of sublanguage xml also sounds like a nifty idea. (though that might be doing a little TOO much, honestly)

@joshgoebel
Copy link
Member

#1625 reminded me that it's not JUST HTML inside these blocks, but you can have JS style comments, template variables, etc...

The more I read the more it seems a separate JSX grammar might be the right choice for dealing with all of this.

@joshgoebel joshgoebel changed the title React JSX multiple self-colsing tags with attributes highlighting React JSX multiple self-closing tags with attributes highlighting Oct 7, 2019
@joshgoebel joshgoebel changed the title React JSX multiple self-closing tags with attributes highlighting React JSX multiple self-closing tags with attributes highlighting broken Oct 7, 2019
@joshgoebel joshgoebel added bug help welcome Could use help from community labels Oct 7, 2019
@joshgoebel joshgoebel changed the title React JSX multiple self-closing tags with attributes highlighting broken (jsx) multiple self-closing tags with attributes highlighting broken Oct 13, 2019
@zwhitchcox
Copy link

I am also having this issue. How exactly would I go about fixing this? Is there a contribution guide?

I cloned the repo, but there is not start script or obvious way to set up the development environment.

@zwhitchcox
Copy link

zwhitchcox commented Dec 13, 2019

Also, I don't know if this is useful, but this is a minimum reproduction of the bug

const n = () => <X />
const m = () => <X x="" />

If you remove the x="" attribute, it works fine. Also, if you remove the first element, it works fine.

@joshgoebel
Copy link
Member

joshgoebel commented Dec 13, 2019

Well it's all Node.js... you can build a new library with:

node ./tools/build.js -t node

Run tests:

npm run test

I'd start by adding a new expects test in tests/markup/javascript (look at other languages for example)... to get a failing test case. Then you'd have to work on the language grammar until you fix the bug.

@joshgoebel
Copy link
Member

The JSX stuff is around line 190 or so in the grammar.

@joshgoebel
Copy link
Member

So it looks like we're only trying to find the beginning and end of the tags and then let XML do the highlighting... which makes sense... I haven't looked at this closely before so I"m not sure what is going wrong here. These regexes are way more complex than they need to be though at first glance.

@joshgoebel
Copy link
Member

If anyone wants to test that, that'd be great.

@joshgoebel
Copy link
Member

joshgoebel commented Dec 13, 2019

Really we should make sure the begin and end tags are actually matching, but we can't do that yet...

@joshgoebel
Copy link
Member

joshgoebel commented Dec 14, 2019

@leafOfTree

+      {
+        className: 'jsx-attr',
+        begin: /={/,
+        end: /}/,
+        subLanguage: 'xml',
+      },

Actually technically isn't the sublanguage we should use javascript? The {} just allowed you to include javascript (event handlers, etc) which could also be more JSX code... correct?

And can't {} truly be used anywhere, not just for attributes?

@joshgoebel
Copy link
Member

I've added both these as test cases (and they pass):

const n = () => <X />
const m = () => <X x="" />
class App extends Component {
  render() {
    return (
      <BrowserRouter>
        <div>
          <Route path="/about" component={About} />
          <Route path="/contact" component={Contact} />
        </div>
      </BrowserRouter>
    );
  }
}

They both are good examples of the tag closing issue I think. The other larger example has more to do with {} and inlining code than tags - and we need to treat that as a separate (more complex) issue.

The larger example is likely parsed better than before, but it still gets confused about the attribute highlighting since it doesn't understand the inline {} stuff.

@joshgoebel
Copy link
Member

Opening a new issue to track the issues with {} separate from the issues with tag closing.

@leafOfTree
Copy link
Author

leafOfTree commented Dec 16, 2019

Thanks for tracking these jsx issues. @yyyc514

@leafOfTree

+      {
+        className: 'jsx-attr',
+        begin: /={/,
+        end: /}/,
+        subLanguage: 'xml',
+      },

Actually technically isn't the sublanguage we should use javascript? The {} just allowed you to include javascript (event handlers, etc) which could also be more JSX code... correct?

And can't {} truly be used anywhere, not just for attributes?

You are right about the {}. I know a little about highlight.js and I used xml as sublanguage beacuse src/language/javascript.js uses sublanguage: xml for jsx and it seems to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help welcome Could use help from community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants