Added template matching following innerHTML #441
Conversation
|
How common is |
|
Uhm, extremely common... |
| @@ -1360,6 +1360,30 @@ | |||
| ] | |||
| } | |||
| { | |||
| 'begin': '(?<=innerHTML)\\s*(=)\\s*(`)' | |||
There was a problem hiding this comment.
May wanna make this (\\+?=) instead, to accommodate innerHTML +=.
There was a problem hiding this comment.
Completely right. I've fixed that.
|
|
|
Yeah, sorry for that comment. I thought we were still dealing with template string functions and completely forgot about the actual method. This will need some tests. |
| @@ -1360,6 +1360,28 @@ | |||
| ] | |||
| } | |||
| { | |||
| 'begin': '(?<=innerHTML\\s*(\\+?=)\\s*)(`)' | |||
There was a problem hiding this comment.
Lookbehinds only support fixed-width regexes, so \\s* and stuff won't work. I would suggest going back to the previous regex and trying to make that one work.
There was a problem hiding this comment.
Yeah. Aside: is there a way that I could set up a local testing environment so that I don't have to wait for Travis to finish each time I tweak something?
There was a problem hiding this comment.
Run the package specs: View -> Developer -> Run Package Specs.
There was a problem hiding this comment.
Thanks. I'm dumb.
|
Hey @50Wliu can this be merged? I am waiting for this very badly. Writing es6 class based components, I really need this. |
|
I didn't get notifications for the two latest commits that fixed the specs. Thanks for the heads up. |
|
So, I realize now that I didn't look closely enough at the specs for the last HTML-related PR before merging. If you notice, none of the HTML tags are being scoped as HTML. To fix that, I would recommend grouping all the HTML-related specs under a single beforeEach ->
waitsForPromise ->
atom.packages.activatePackage("language-html") |
|
That breaks all of the HTML-related specs. I've excerpted that section of the file as I have it in a gist. The error message is wordy enough that I've put it in a gist, too. If this is a problem with all of the related specs, I suggest merging this PR and opening another one to fix the specs in general. The grammar portion of the PR is fully functional and as of the latest commit, the spec I added functions at the same level as the pre-existing ones. |
| expect(tokens[0]).toEqual value: 'text', scopes: [ 'source.js', 'variable.other.object.js' ] | ||
| expect(tokens[1]).toEqual value: '.', scopes: [ 'source.js', 'meta.delimiter.property.period.js' ] | ||
| expect(tokens[2]).toEqual value: 'innerHTML', scopes: [ 'source.js', 'variable.other.property.js' ] | ||
| expect(tokens[3]).toEqual value: ' ', scopes: [ 'source.js', 'string.quoted.template.html.js' ] |
There was a problem hiding this comment.
These scopes are incorrect. string.quoted should only start at the actual backtick, not before it. You can probably fix that by removing the capturing portions of begin and making it a lookbehind followed by a lookahead. Then change name to contentName and move what was previously captured in begin to the patterns section. In addition end needs to be changed to a lookbehind.
There was a problem hiding this comment.
Okay, I've fixed the scopes and updated the grammar so that the output is correct.
|
Thanks! |
|
Awesome thanks @DamnedScholar & @50Wliu. When will this be released? |
|
Current ETA is Atom 1.13. |
|
Hey @50Wliu, |
|
1.13. |
|
Oh noo, this will be so long, any idea when it will be released? Aboutish? |
|
1.13-beta will be out in the next few weeks or so, and 1.13 stable should be released sometime in December. |
|
okay, thanks for the info |
|
@lukasoppermann You can always override the core package until this version is released in core. Just |
|
My apologies - this did not make it into Atom 1.13 (the release came sooner than I expected and it didn't make the cutoff). Sorry for the inconvenience, and I would follow @DamnedScholar's steps for now. |
A forum thread has raised the fact that template strings can sometimes be used to assign
innerHTMLvalues and that the developer might not want to involve thehtmltemplate handler. This PR proposes to detect template strings followinginnerHTML =as HTML templates and includetext.html.basicjust like the template handler does.