XML syntax definition problem

[alexr00]

From @omochi on January 19, 2019 17:0

I may found bug of syntax.

https://github.com/Microsoft/vscode/blob/10a1d2a50a2882f5ae85bdb51eb04d3064fb9de9/extensions/xml/syntaxes/xml.tmLanguage.json#L354-L363

It should be:

 { 
 	"begin": "<%--", 
 	"captures": { 
 		"0": { 
 			"name": "punctuation.definition.comment.xml" 
 		}
	}, 
 	"end": "--%>", 
 	"name": "comment.block.xml"  
 }, 

Copied from original issue: microsoft/vscode#66776

[alexr00]

From @omochi on January 19, 2019 17:24

Oh, I found more.

https://github.com/Microsoft/vscode/blob/10a1d2a50a2882f5ae85bdb51eb04d3064fb9de9/extensions/xml/syntaxes/xml.tmLanguage.json#L375-L380

should be:

"match": "--(?!>)",
"captures": {
    "0": {
        "name": "invalid.illegal.bad-comments-or-CDATA.xml"
    }
}

[rsese]

Thanks @alexr00 @omochi - can either of you share a specific code snippet that demonstrates the problem?

[omochi]

I recorded video.

https://www.youtube.com/watch?v=UeCwolx7oW0&feature=youtu.be

First issue:
begin and end are not paired.
So punctuation.definition.comment.xml scope does not constructed.

Second issue:
invalid.illegal.bad-comments-or-CDATA.xml is single token and not scope.
So match rule is better than begin.

VSCode process begin without end as match.
So no user level issue is. It is just for technically quality.
Because textmate syntax file is kinda interchangeable among some text editors.

[rsese]

Thanks @omochi -

First issue:
begin and end are not paired.
So punctuation.definition.comment.xml scope does not constructed.

It looks like your example is:

<%-- --%>

I think those are JSP comments, not XML comments? In Atom, if I set the language to JSP the closing --%> is recognized correctly:

x-ref: #87 and 0a04e02

Let me know if I'm misunderstanding something here.

Second issue:
invalid.illegal.bad-comments-or-CDATA.xml is single token and not scope.
So match rule is better than begin.

Your example here is:

<!-- -- -->

Related to the PR above, I believe this is already reported here: #91

[omochi]

@rsese I just reports about syntax of grammar file.

report 1

See this current code.

      {
        'begin': '<%--'
        'captures':
          '0':
            'name': 'punctuation.definition.comment.xml'
          'end': '--%>'
          'name': 'comment.block.xml'
      }

language-xml/grammars/xml.cson

Lines 404 to 411 in 7bc75df

	{
	'begin': '<%--'
	'captures':
	'0':
	'name': 'punctuation.definition.comment.xml'
	'end': '--%>'
	'name': 'comment.block.xml'
	}

end parameter is in object for captures key.
It is wrong.
end should be in same object with begin key.

So it should be this.

 { 
    "begin": "<%--", 
    "captures": { 
        "0": { 
            "name": "punctuation.definition.comment.xml" 
        }
    }, 
    "end": "--%>", 
    "name": "comment.block.xml"  
 }, 

Because JSP mode works correctly, JSP syntax is written as so.

  'comment':
    'begin': '<%--'
    'captures':
      '0':
        'name': 'punctuation.definition.comment.jsp'
    'end': '--%>'
    'name': 'comment.block.jsp'

https://github.com/atom/language-java/blob/3f78faa86e8f84786e69715e1911a028fe99035c/grammars/java%20server%20pages%20(jsp).cson#L55-L61

report 2

current definition is.

          {
            'begin': '--(?!>)'
            'captures':
              '0':
                'name': 'invalid.illegal.bad-comments-or-CDATA.xml'
          }

language-xml/grammars/xml.cson

Lines 420 to 425 in 7bc75df

	{
	'begin': '--(?!>)'
	'captures':
	'0':
	'name': 'invalid.illegal.bad-comments-or-CDATA.xml'
	}

I think that to replace begin to match is better.
So it should be this.

          {
            'match': '--(?!>)'
            'captures':
              '0':
                'name': 'invalid.illegal.bad-comments-or-CDATA.xml'
          }

Because this block is intended to match -- inside comment.
It is single token match and not scope creation.
If it works in atom and VSCode, it is by their flexible fallback that convert begin to match if it has no corresponded end.
Changing match is good for compatibility of this syntax file among other text editors.

[omochi]

More information about report (2).

I reported same concept for PHP syntax.

microsoft/vscode#66809

They accept it and make PR to patch this.

atom/language-php#352

This PR is also reviewed by @rsese . I say same things.

[no-response]

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.