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

(javscript/typescript) improve JSX comment highlighting #1625

Open
brigand opened this issue Nov 2, 2017 · 6 comments
Open

(javscript/typescript) improve JSX comment highlighting #1625

brigand opened this issue Nov 2, 2017 · 6 comments
Assignees
Labels
enhancement An enhancement or new feature help welcome Could use help from community

Comments

@brigand
Copy link

brigand commented Nov 2, 2017

Live example on the npm website.

react-ck5 readme on npmjs.com

  state = { editorState: null }
  render() {
    return (
      <div>
        <ClassicBasic
          value={/* ... */}
          onChange={/* ... */}
          editorState={this.state.editorState}
          onStateChange={editorState => this.setState({ editorState })}
        />
        <button
          // prevent editor losing focus
          onMouseDown={e => e.preventDefault()}
          onClick={() => {
            this.setState(s => update(s, 'editorState.bold.value', value => !value));
          }}
        >
          Toggle bold
        </button>
      </div>
    );
  }

Expected behavior

Comments in jsx attribute positions should look like comments. The spec isn't explicit about it, but babel, acorn, typescript, and flow all support the syntax.


Thanks!

original issue

@nickserv
Copy link

nickserv commented Dec 2, 2017

I'm pretty sure this is illegal JSX syntax, the spec doesn't mention using // comments and this might be typo or something specific to your JSX transpiler, but I don't think it's standard.

Edit: Apparently it is legal but only between <>, not between ><.

@joshgoebel
Copy link
Member

Edit: Apparently it is legal but only between <>, not between ><.

So in that case it acts like a Javascript style comment... that probably also pours a little rainstorm on the idea of trying to use "html/xml" as the internal parser for these blocks though.

Seeming more and more like we need a dedicated jsx grammar.

@joshgoebel joshgoebel added the enhancement An enhancement or new feature label Oct 7, 2019
@joshgoebel joshgoebel changed the title JSX Comment Highlighting (jsx) improve comment highlighting Oct 13, 2019
@joshgoebel joshgoebel added the help welcome Could use help from community label Dec 14, 2019
@joshgoebel joshgoebel changed the title (jsx) improve comment highlighting (javscript/typescript/jsx) improve comment highlighting Feb 19, 2020
@joshgoebel joshgoebel changed the title (javscript/typescript/jsx) improve comment highlighting (javscript/typescript) improve JSX comment highlighting Feb 19, 2020
@joshgoebel joshgoebel added this to the 10.1 milestone Apr 21, 2020
@joshgoebel joshgoebel modified the milestones: 10.1, 10.2 Jun 11, 2020
@joshgoebel joshgoebel modified the milestones: 10.2, 10.3 Aug 17, 2020
@joshgoebel
Copy link
Member

Screen Shot 2020-10-01 at 2 25 57 AM

What I have working so far built on top of the new consolidated JS/TS stuff... wasn't that hard either.

@joshgoebel
Copy link
Member

joshgoebel commented Oct 1, 2020

@allejo @egor-rogov

There is a weirdness here though... we're inside "tag" (which has it's own styling) and when we hit a {} pair then we have embedded JS... which should go back to the default "unhighlighted" color... not the tag color... so to do this we really need a new css class... reset or default or something that is pinned to whatever .hljs is... to allow content inside of a tag to do a "reset" to get back to the "parent" style...

Any thoughts on this or naming?

@joshgoebel
Copy link
Member

joshgoebel commented Oct 1, 2020

Some dark magic:

            contains: [
              HTML_TAG,
              XML_ENTITIES,
              {
                begin: /[^<]+/, // eat everything up until the next <
                end: /./, // to avoid 0-width match error
                // always return, this rule is not intended to consume
                // anything, just bookend the JSX/HTML segment
                returnEnd: true,
                returnBegin: true,
                endsParent:true, // when we've paired all HTML tags, we endParent
                "on:begin": (_, resp) => {
                  if (nestedTagCount !== 0) resp.ignoreMatch();
                }
              }
            ]

It's likely we just need a response.endsParent() that could be called from HTML_TAG's on:end rule (to allow modes to conditionally end the parent)... but still it's cool this seems to work great. :-)

@joshgoebel joshgoebel modified the milestones: 10.3, 10.4 Oct 14, 2020
@allejo
Copy link
Member

allejo commented Oct 21, 2020

@allejo @egor-rogov

There is a weirdness here though... we're inside "tag" (which has it's own styling) and when we hit a {} pair then we have embedded JS... which should go back to the default "unhighlighted" color... not the tag color... so to do this we really need a new css class... reset or default or something that is pinned to whatever .hljs is... to allow content inside of a tag to do a "reset" to get back to the "parent" style...

Any thoughts on this or naming?

What about hljs-unset following CSS's unset keyword?

@joshgoebel joshgoebel modified the milestones: 10.4, 10.5 Nov 10, 2020
@joshgoebel joshgoebel removed this from the 10.5 milestone Dec 23, 2020
@joshgoebel joshgoebel self-assigned this Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature help welcome Could use help from community
Projects
None yet
Development

No branches or pull requests

4 participants