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: properly remove block comments from templates #10701

Merged
merged 2 commits into from
Apr 19, 2021

Conversation

denis-anisimov
Copy link
Contributor

fixes #10673

Comment on lines 68 to 91
@Test
public void removeComments_commentsWithAsterisksInside_commentIsRemoved() {
String result = StringUtil.removeComments(
// @formatter:off
"import { html, LitElement } from 'lit-element';\n"
+ "\n"
+ "export class HelloLit extends LitElement {\n"
+ " /* ******************************************************\n"
+ " * comment\n"
+ " * ******************************************************/\n"
+ "\n"
+ " render() {\n"
+ " return html` <div>Some content</div>`;\n"
+ " }\n"
+ "}\n"
+ "\n"
+ "customElements.define('hello-lit', HelloLit);");
// @formatter:on
MatcherAssert.assertThat(result, CoreMatchers.allOf(
CoreMatchers.containsString("<div>Some content</div>"),
CoreMatchers.containsString(
"customElements.define('hello-lit', HelloLit);"),
CoreMatchers.not(CoreMatchers.containsString("comment"))));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though this exhibits the error it's not clear why this would fail when the other block tests do not.
The actual problem comes when there is a n%2 amount of asterisks before the / as the check eats up one character after each asterisk.

Also the test didn't show in the failing assertion that the actual problem is that after the block comment there is no string left.

A clearer test would be:

    @Test
    public void endCommentCheck_shouldNotEatCharacters(){
        String blockComment = StringUtil
            .removeComments("return html'/* comment **/';");
        Assert.assertEquals("return html'';", blockComment);
    }

Which on failure will give a clearer exception where one can see that the whole end of the string is missing.

org.junit.ComparisonFailure: 
Expected :return html'Code';
Actual   :return html'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with this.

@denis-anisimov denis-anisimov merged commit 6b5fc3a into master Apr 19, 2021
@denis-anisimov denis-anisimov deleted the 10673-comments-in-templates branch April 19, 2021 06:49
@vaadin-bot
Copy link
Collaborator

Hi @denis-anisimov , this commit cannot be picked to 6.0 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick 6b5fc3a
error: could not apply 6b5fc3a... fix: properly remove block comments from templates (#10701)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

@vaadin-bot
Copy link
Collaborator

Hi @denis-anisimov , this commit cannot be picked to 2.6 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick 6b5fc3a
error: could not apply 6b5fc3a... fix: properly remove block comments from templates (#10701)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

@vaadin-bot
Copy link
Collaborator

Hi @denis-anisimov , this commit cannot be picked to 2.5 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick 6b5fc3a
error: could not apply 6b5fc3a... fix: properly remove block comments from templates (#10701)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

denis-anisimov pushed a commit that referenced this pull request Apr 19, 2021
* fix: properly remove block comments from templates

fixes #10673
denis-anisimov pushed a commit that referenced this pull request Apr 19, 2021
* fix: properly remove block comments from templates

fixes #10673
denis-anisimov pushed a commit that referenced this pull request Apr 19, 2021
* fix: properly remove block comments from templates

fixes #10673
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Comments in LitTemplate can produce an invalid warning
3 participants