From 91ad45ad392ba0ed28ce0b275c437f17619ab537 Mon Sep 17 00:00:00 2001 From: Keith Hall Date: Wed, 17 Oct 2018 14:58:47 +0300 Subject: [PATCH 1/7] correctly apply multiple with_prototypes when a context is set --- src/parsing/parser.rs | 63 ++++++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/src/parsing/parser.rs b/src/parsing/parser.rs index a5e54960..ff5501e3 100644 --- a/src/parsing/parser.rs +++ b/src/parsing/parser.rs @@ -36,7 +36,7 @@ pub struct ParseState { #[derive(Debug, Clone, Eq, PartialEq)] struct StateLevel { context: ContextId, - prototype: Option, + prototype: Vec, captures: Option<(Region, String)>, } @@ -150,7 +150,7 @@ impl ParseState { pub fn new(syntax: &SyntaxReference) -> ParseState { let start_state = StateLevel { context: syntax.contexts["__start"].clone(), - prototype: None, + prototype: Vec::new(), captures: None, }; ParseState { @@ -309,7 +309,7 @@ impl ParseState { let context_chain = { let proto_start = self.proto_starts.last().cloned().unwrap_or(0); // Sublime applies with_prototypes from bottom to top - let with_prototypes = self.stack[proto_start..].iter().filter_map(|lvl| lvl.prototype.as_ref().map(|ctx| (true, ctx, lvl.captures.as_ref()))); + let with_prototypes = self.stack[proto_start..].iter().flat_map(|lvl| lvl.prototype.iter().map(|ctx| (true, ctx, None))); // TODO: replace None with lvl.captures.as_ref() let cur_prototype = prototype.into_iter().map(|ctx| (false, ctx, None)); let cur_context = Some((false, &cur_level.context, cur_level.captures.as_ref())).into_iter(); with_prototypes.chain(cur_prototype).chain(cur_context) @@ -604,11 +604,13 @@ impl ParseState { pat: &MatchPattern, syntax_set: &SyntaxSet ) -> bool { - let (ctx_refs, old_proto_id) = match pat.operation { + let (ctx_refs, old_proto_ids) = match pat.operation { MatchOperation::Push(ref ctx_refs) => (ctx_refs, None), MatchOperation::Set(ref ctx_refs) => { - let old_proto_id = self.stack.pop().and_then(|s| s.prototype); - (ctx_refs, old_proto_id) + // a `with_prototype` stays active when the context is `set` + // until the context layer in the stack (where the `with_prototype` + // was initially applied) is popped off. + (ctx_refs, self.stack.pop().map(|s| s.prototype)) } MatchOperation::Pop => { self.stack.pop(); @@ -617,29 +619,23 @@ impl ParseState { MatchOperation::None => return false, }; for (i, r) in ctx_refs.iter().enumerate() { - let proto_id = if i == ctx_refs.len() - 1 { - // a `with_prototype` stays active when the context is `set` - // until the context layer in the stack (where the `with_prototype` - // was initially applied) is popped off. + let mut proto_ids = old_proto_ids.clone().unwrap_or(Vec::new()); + if i == ctx_refs.len() - 1 { // if a with_prototype was specified, and multiple contexts were pushed, // then the with_prototype applies only to the last context pushed, i.e. // 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 if let Some(ref p) = pat.with_prototype { - Some(p.id()) - } else { - old_proto_id + proto_ids.push(p.id().clone()); } - } else { - None - }; + } let context_id = r.id(); let context = syntax_set.get_context(&context_id); let captures = { let mut uses_backrefs = context.uses_backrefs; - if let Some(ref proto_id) = proto_id { - uses_backrefs = uses_backrefs || syntax_set.get_context(proto_id).uses_backrefs; + if !proto_ids.is_empty() { + uses_backrefs = uses_backrefs || proto_ids.iter().any(|id| syntax_set.get_context(id).uses_backrefs); } if uses_backrefs { Some((regions.clone(), line.to_owned())) @@ -649,7 +645,7 @@ impl ParseState { }; self.stack.push(StateLevel { context: context_id, - prototype: proto_id, + prototype: proto_ids, captures, }); } @@ -1404,6 +1400,35 @@ contexts: expect_scope_stacks_with_syntax("testfoo", &["", /*"",*/ "", ""], syntax); } + #[test] + fn can_parse_two_with_prototypes_at_same_stack_level() { + let syntax = r#" +%YAML 1.2 +--- +# See http://www.sublimetext.com/docs/3/syntax.html +scope: source.example-wp +contexts: + main: + - match: a + scope: a + push: + - match: b + scope: b + set: + - match: c + scope: c + with_prototype: + - match: '2' + scope: '2' + with_prototype: + - match: '1' + scope: '1' +"#; + + let syntax = SyntaxDefinition::load_from_str(&syntax, true, None).unwrap(); + expect_scope_stacks_with_syntax("abc12", &["<1>", "<2>"], syntax); + } + fn expect_scope_stacks(line_without_newline: &str, expect: &[&str], syntax: &str) { println!("Parsing with newlines"); let line_with_newline = format!("{}\n", line_without_newline); From 546c516d2bd209e4c7774d7a2854223b1da2a785 Mon Sep 17 00:00:00 2001 From: Keith Hall Date: Wed, 17 Oct 2018 15:42:32 +0300 Subject: [PATCH 2/7] remove ASP from known syntest failures --- testdata/known_syntest_failures.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/testdata/known_syntest_failures.txt b/testdata/known_syntest_failures.txt index aadb0783..f55cff60 100644 --- a/testdata/known_syntest_failures.txt +++ b/testdata/known_syntest_failures.txt @@ -1,5 +1,4 @@ loading syntax definitions from testdata/Packages -FAILED testdata/Packages/ASP/syntax_test_asp.asp: 162 FAILED testdata/Packages/C#/tests/syntax_test_Strings.cs: 38 FAILED testdata/Packages/LaTeX/syntax_test_latex.tex: 1 FAILED testdata/Packages/Makefile/syntax_test_makefile.mak: 6 From 588530467d3064e2664dd2444a65d417b509e63d Mon Sep 17 00:00:00 2001 From: Keith Hall Date: Wed, 17 Oct 2018 17:13:52 +0300 Subject: [PATCH 3/7] add additional test case for capture group changes on set --- src/parsing/parser.rs | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/parsing/parser.rs b/src/parsing/parser.rs index ff5501e3..e6be6513 100644 --- a/src/parsing/parser.rs +++ b/src/parsing/parser.rs @@ -1402,7 +1402,7 @@ contexts: #[test] fn can_parse_two_with_prototypes_at_same_stack_level() { - let syntax = r#" + let syntax_yamlstr = r#" %YAML 1.2 --- # See http://www.sublimetext.com/docs/3/syntax.html @@ -1425,10 +1425,41 @@ contexts: scope: '1' "#; - let syntax = SyntaxDefinition::load_from_str(&syntax, true, None).unwrap(); + let syntax = SyntaxDefinition::load_from_str(&syntax_yamlstr, true, None).unwrap(); expect_scope_stacks_with_syntax("abc12", &["<1>", "<2>"], syntax); } + #[test] + fn can_parse_two_with_prototypes_at_same_stack_level_updated_captures() { + let syntax_yamlstr = r#" +%YAML 1.2 +--- +# See http://www.sublimetext.com/docs/3/syntax.html +scope: source.example-wp +contexts: + main: + - match: (a) + scope: a + push: + - match: (b) + scope: b + set: + - match: c + scope: c + with_prototype: + - match: d + scope: d + with_prototype: + - match: \1 + scope: '1' + pop: true +"#; + + let syntax = SyntaxDefinition::load_from_str(&syntax_yamlstr, true, None).unwrap(); + expect_scope_stacks_with_syntax("aa", &["", "<1>"], syntax.clone()); + expect_scope_stacks_with_syntax("abcdb", &["", "", "", "", "<1>"], syntax); + } + fn expect_scope_stacks(line_without_newline: &str, expect: &[&str], syntax: &str) { println!("Parsing with newlines"); let line_with_newline = format!("{}\n", line_without_newline); From 910827163f2fd3bcfba3502bb2fa920cd832132e Mon Sep 17 00:00:00 2001 From: Keith Hall Date: Wed, 17 Oct 2018 17:35:41 +0300 Subject: [PATCH 4/7] added additional test case that will have to pass --- src/parsing/parser.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/parsing/parser.rs b/src/parsing/parser.rs index e6be6513..3d442e81 100644 --- a/src/parsing/parser.rs +++ b/src/parsing/parser.rs @@ -1460,6 +1460,40 @@ contexts: expect_scope_stacks_with_syntax("abcdb", &["", "", "", "", "<1>"], syntax); } + #[test] + fn can_parse_two_with_prototypes_at_same_stack_level_updated_captures_ignore_unexisting() { + let syntax_yamlstr = r#" +%YAML 1.2 +--- +# See http://www.sublimetext.com/docs/3/syntax.html +scope: source.example-wp +contexts: + main: + - match: (a)(-) + scope: a + push: + - match: (b) + scope: b + set: + - match: c + scope: c + with_prototype: + - match: d + scope: d + with_prototype: + - match: \2 + scope: '2' + pop: true + - match: \1 + scope: '1' + pop: true +"#; + + let syntax = SyntaxDefinition::load_from_str(&syntax_yamlstr, true, None).unwrap(); + expect_scope_stacks_with_syntax("a--", &["", "<2>"], syntax.clone()); + expect_scope_stacks_with_syntax("a-bcdba-", &["", ""], syntax); // TODO: it seems that when ST encounters a non existing pop backreference, it just pops back to the with_prototype's original parent context - i.e. cdb is unscoped + } + fn expect_scope_stacks(line_without_newline: &str, expect: &[&str], syntax: &str) { println!("Parsing with newlines"); let line_with_newline = format!("{}\n", line_without_newline); From d4e3b78e869475bd9d15bc71d13f9e669e4eac17 Mon Sep 17 00:00:00 2001 From: Keith Hall Date: Thu, 18 Oct 2018 09:01:35 +0300 Subject: [PATCH 5/7] move closure to allow keeping capture details --- src/parsing/parser.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/parsing/parser.rs b/src/parsing/parser.rs index 3d442e81..e41fbc1c 100644 --- a/src/parsing/parser.rs +++ b/src/parsing/parser.rs @@ -36,7 +36,7 @@ pub struct ParseState { #[derive(Debug, Clone, Eq, PartialEq)] struct StateLevel { context: ContextId, - prototype: Vec, + prototypes: Vec, captures: Option<(Region, String)>, } @@ -150,7 +150,7 @@ impl ParseState { pub fn new(syntax: &SyntaxReference) -> ParseState { let start_state = StateLevel { context: syntax.contexts["__start"].clone(), - prototype: Vec::new(), + prototypes: Vec::new(), captures: None, }; ParseState { @@ -309,7 +309,7 @@ impl ParseState { let context_chain = { let proto_start = self.proto_starts.last().cloned().unwrap_or(0); // Sublime applies with_prototypes from bottom to top - let with_prototypes = self.stack[proto_start..].iter().flat_map(|lvl| lvl.prototype.iter().map(|ctx| (true, ctx, None))); // TODO: replace None with lvl.captures.as_ref() + let with_prototypes = self.stack[proto_start..].iter().flat_map(|lvl| lvl.prototypes.iter().map(move |ctx| (true, ctx, lvl.captures.as_ref()))); let cur_prototype = prototype.into_iter().map(|ctx| (false, ctx, None)); let cur_context = Some((false, &cur_level.context, cur_level.captures.as_ref())).into_iter(); with_prototypes.chain(cur_prototype).chain(cur_context) @@ -610,7 +610,7 @@ impl ParseState { // a `with_prototype` stays active when the context is `set` // until the context layer in the stack (where the `with_prototype` // was initially applied) is popped off. - (ctx_refs, self.stack.pop().map(|s| s.prototype)) + (ctx_refs, self.stack.pop().map(|s| s.prototypes)) } MatchOperation::Pop => { self.stack.pop(); @@ -619,7 +619,7 @@ impl ParseState { MatchOperation::None => return false, }; for (i, r) in ctx_refs.iter().enumerate() { - let mut proto_ids = old_proto_ids.clone().unwrap_or(Vec::new()); + let mut proto_ids = old_proto_ids.clone().unwrap_or_else(|| Vec::new()); if i == ctx_refs.len() - 1 { // if a with_prototype was specified, and multiple contexts were pushed, // then the with_prototype applies only to the last context pushed, i.e. @@ -645,7 +645,7 @@ impl ParseState { }; self.stack.push(StateLevel { context: context_id, - prototype: proto_ids, + prototypes: proto_ids, captures, }); } @@ -1491,7 +1491,9 @@ contexts: let syntax = SyntaxDefinition::load_from_str(&syntax_yamlstr, true, None).unwrap(); expect_scope_stacks_with_syntax("a--", &["", "<2>"], syntax.clone()); - expect_scope_stacks_with_syntax("a-bcdba-", &["", ""], syntax); // TODO: it seems that when ST encounters a non existing pop backreference, it just pops back to the with_prototype's original parent context - i.e. cdb is unscoped + // it seems that when ST encounters a non existing pop backreference, it just pops back to the with_prototype's original parent context - i.e. cdb is unscoped + // TODO: it would be useful to have syntest functionality available here for easier testing and clarity + expect_scope_stacks_with_syntax("a-bcdba-", &["", ""], syntax); } fn expect_scope_stacks(line_without_newline: &str, expect: &[&str], syntax: &str) { From a3362a95fe1083ed6f551f7698a1d590c5a30994 Mon Sep 17 00:00:00 2001 From: Keith Hall Date: Mon, 29 Oct 2018 06:40:22 +0200 Subject: [PATCH 6/7] don't duplicate old prototype for all pushed frames on set --- src/parsing/parser.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/parsing/parser.rs b/src/parsing/parser.rs index 7b7833f6..39790677 100644 --- a/src/parsing/parser.rs +++ b/src/parsing/parser.rs @@ -619,7 +619,13 @@ impl ParseState { MatchOperation::None => return false, }; for (i, r) in ctx_refs.iter().enumerate() { - let mut proto_ids = old_proto_ids.clone().unwrap_or_else(|| Vec::new()); + let mut proto_ids = if i == 0 { + // it is only necessary to preserve the old prototypes + // at the first stack frame pushed + old_proto_ids.clone().unwrap_or_else(|| Vec::new()) + } else { + Vec::new() + }; if i == ctx_refs.len() - 1 { // if a with_prototype was specified, and multiple contexts were pushed, // then the with_prototype applies only to the last context pushed, i.e. From fedcd89495d1a35edf5573b0b60bbceaf854915f Mon Sep 17 00:00:00 2001 From: Keith Hall Date: Mon, 29 Oct 2018 09:14:51 +0200 Subject: [PATCH 7/7] add test for with_prototype behavior when using set with multiple contexts --- src/parsing/parser.rs | 50 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/parsing/parser.rs b/src/parsing/parser.rs index 39790677..9bd00f6d 100644 --- a/src/parsing/parser.rs +++ b/src/parsing/parser.rs @@ -1435,6 +1435,56 @@ contexts: expect_scope_stacks_with_syntax("abc12", &["<1>", "<2>"], syntax); } + #[test] + fn can_parse_two_with_prototypes_at_same_stack_level_set_multiple() { + let syntax_yamlstr = r#" +%YAML 1.2 +--- +# See http://www.sublimetext.com/docs/3/syntax.html +scope: source.example-wp +contexts: + main: + - match: a + scope: a + push: + - match: b + scope: b + set: [context1, context2, context3] + with_prototype: + - match: '2' + scope: '2' + with_prototype: + - match: '1' + scope: '1' + - match: '1' + scope: digit1 + - match: '2' + scope: digit2 + context1: + - match: e + scope: e + pop: true + - match: '2' + scope: digit2 + context2: + - match: d + scope: d + pop: true + - match: '2' + scope: digit2 + context3: + - match: c + scope: c + pop: true +"#; + + let syntax = SyntaxDefinition::load_from_str(&syntax_yamlstr, true, None).unwrap(); + expect_scope_stacks_with_syntax("ab12", &["<1>", "<2>"], syntax.clone()); + expect_scope_stacks_with_syntax("abc12", &["<1>", ""], syntax.clone()); + expect_scope_stacks_with_syntax("abcd12", &["<1>", ""], syntax.clone()); + expect_scope_stacks_with_syntax("abcde12", &["", ""], syntax.clone()); + } + #[test] fn can_parse_two_with_prototypes_at_same_stack_level_updated_captures() { let syntax_yamlstr = r#"