Update submodules

[sharkdp]

This supersedes #154.

• All tests should pass now
• The modifications in sublimehq/Packages should now be regex syntax-checked.

[sharkdp]

I just found out about syntest. I should mention that there are a lot more failed assertions now:

----
ASP
Ok(FailedAssertions(162, 4077))
----
Batch File
Ok(FailedAssertions(283, 1730))
----
C#
Ok(FailedAssertions(38, 43))
Ok(FailedAssertions(3, 4137))
Ok(FailedAssertions(317, 1890))
Ok(FailedAssertions(8, 321))
----
C++
Ok(FailedAssertions(655, 3048))
----
CSS
Ok(FailedAssertions(151, 3408))
----
D
Ok(FailedAssertions(1, 51))
----
Groovy
Ok(FailedAssertions(441, 757))
----
HTML
Ok(FailedAssertions(129, 1201))
----
Java
Ok(FailedAssertions(391, 409))
Ok(FailedAssertions(186, 243))
Ok(FailedAssertions(3192, 6814))
----
JavaScript
Ok(FailedAssertions(86, 411))
Ok(FailedAssertions(282, 479))
Ok(FailedAssertions(2596, 4423))
----
LaTeX
Ok(FailedAssertions(286, 926))
----
Makefile
Ok(FailedAssertions(7, 612))
----
Markdown
Ok(FailedAssertions(860, 6643))
----
Matlab
Ok(FailedAssertions(56, 122))
----
Objective-C
Ok(FailedAssertions(54, 2892))
----
PHP
Ok(FailedAssertions(489, 4504))
----
Python
Ok(FailedAssertions(64, 1355))
Ok(FailedAssertions(124, 3362))
----
R
Ok(FailedAssertions(3, 501))
----
Rust
Ok(FailedAssertions(58, 3160))
----
ShellScript
Ok(FailedAssertions(907, 2599))
----
TCL
Ok(FailedAssertions(284, 1004))

[keith-hall]

The 104 "Java properties" failures (FAILED testdata/Packages/Java/syntax_test_java_properties.properties: 104 from Travis CI) seem interesting, as it is a small syntax definition. Again it seems to be related to with_prototype - this time in relation to set.
Removing the with_prototype by adding the matches into the contexts manually gives us 0 failures in syntect:

--- Shipped Packages/Java/JavaProperties.sublime-syntax 2018-03-29 12:10:14
+++ Packages/Java/JavaProperties.sublime-syntax 2018-05-29 10:41:34
@@ -28,23 +28,29 @@
       set:
         - match: '[:=]'
           scope: punctuation.separator.java-props
           set: property-value
         - include: property-value
-      with_prototype:
+      #with_prototype:
         - match: (?=\n)
           pop: true
 
   property-value:
     - match: (?=\S)
       set:
         - meta_content_scope: string.unquoted.java-props
         - include: backslash
+    
+    - match: (?=\n)
+      pop: true
 
   backslash:
     - match: (\\)\n
       captures:
         1: punctuation.separator.continuation.java-props
     - match: \\u[[:xdigit:]]{4}
       scope: constant.character.escape.unicode.java-props
     - match: \\[^u]
       scope: constant.character.escape.java-props
+    
+    - match: (?=\n)
+      pop: true

This could answer why JavaDoc was also failing, because set is used (far) under the with_prototype. I'm guessing that Sublime Text keeps the with_prototype active when set is used for nested contexts but syntect doesn't.

[robinst]

I'm guessing that Sublime Text keeps the with_prototype active when set is used for nested contexts but syntect doesn't.

I tried that out with this change:

diff --git a/src/parsing/parser.rs b/src/parsing/parser.rs
index da747bf..7f482e9 100644
--- a/src/parsing/parser.rs
+++ b/src/parsing/parser.rs
@@ -595,11 +595,11 @@ impl ParseState {
 
     /// Returns true if the stack was changed
     fn perform_op(&mut self, line: &str, regions: &Region, pat: &MatchPattern) -> bool {
-        let ctx_refs = match pat.operation {
-            MatchOperation::Push(ref ctx_refs) => ctx_refs,
+        let (ctx_refs, old_proto) = match pat.operation {
+            MatchOperation::Push(ref ctx_refs) => (ctx_refs, None),
             MatchOperation::Set(ref ctx_refs) => {
-                self.stack.pop();
-                ctx_refs
+                let old_proto = self.stack.pop().and_then(|s| s.prototype);
+                (ctx_refs, old_proto)
             }
             MatchOperation::Pop => {
                 self.stack.pop();
@@ -613,7 +613,9 @@ impl ParseState {
             // top most on the stack after all the contexts are pushed - this is also
             // referred to as the "target" of the push by sublimehq - see
             // https://forum.sublimetext.com/t/dev-build-3111/19240/17 for more info
-            let proto = if i == ctx_refs.len() - 1 {
+            let proto = if i == 0 && pat.with_prototype.is_none() {
+                old_proto.clone()
+            } else if i == ctx_refs.len() - 1 {
                 pat.with_prototype.clone()
             } else {
                 None

It fixes syntax_test_java_properties.properties but I get this later:

Testing file testdata/Packages/Markdown/syntax_test_markdown.md
The test file references syntax definition file: Packages/Markdown/Markdown.sublime-syntax
thread 'main' panicked at 'Can only call resolve on linked references: ByScope { scope: <source.javascript>, sub_context: None }', src/parsing/syntax_definition.rs:205:18

I think maybe we need to sit down and systematically go through the prototype logic. By that I mean write a minimal test case in Sublime and put it in the parser test cases and see if we do the right thing. Repeat for all the intersections with other features.

[keith-hall]

It looks like that error isn't caused by your changes, it's a bug in the Markdown syntax referencing a scope/syntax definition that doesn't exist. See sublimehq/Packages#1618. We will need to change syntect to not report missing embeded contexts as errors (and treat it more like include), but as it rewrites embed to the equivalent with_prototype push (EDIT: at YAML load time), and only checks it at link time, it may not be trivial.

So your fix looks correct, it seems to reduce the number of syntest failures slightly. I agree it'd be a great idea to get some more of these test cases in the parser itself.

[Keats]

A new submodule update fails with the following:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParseSyntax(RegexCompileError("\\b(?x)\n(?:attributes|autodie(?:::(?:exception(?:::system)|hints|skip))?|autouse\n  |base|bigint|bignum|bigrat|blib|bytes\n  |charnames|constant\n  |diagnostics\n  |encoding\n  |feature|fields|filetest\n  |if|integer\n  |less|lib|locale\n  |mro\n  |ok|open|ops|overload|overloading\n  |parent\n  |re\n  |sigtrap|sort|strict|subs\n  |threads|threads::shared\n  |utf8\n  |vars|vmsish\n  |warnings|warnings::register)(?!\\w| *::)", Error(-113, target of repeat operator is not specified)))', libcore/result.rs:945:5

which seems to come from https://github.com/sublimehq/Packages/blob/181aa52de5f9f8d7f49be0d9beae90107db3547e/Perl/Perl.sublime-syntax#L247 which had a full refactor merged a few days ago

[keith-hall]

yup, not sure why/how it works in Sublime Text, but I've already reported it at sublimehq/Packages#1564 (comment), so we just have to wait for the new PR to be merged.
EDIT: it works in ST because there is a bug in how it parses the regex differently to Oniguruma: sublimehq/sublime_text#2354

[Enselic]

I'm closing this now to keep the PR inbox clean. Of course feel free to re-open if the change still makes sense to do.