From 459b3fa20a950a4352ce0d71ae0fbb420a0f8a3d Mon Sep 17 00:00:00 2001 From: Jamy Golden Date: Sat, 17 Aug 2024 01:10:34 +0200 Subject: [PATCH] Fix bug where variables including dots aren't replaced correctly --- Cargo.lock | 4 +- ribboncurls-cli/CHANGELOG.md | 7 ++ ribboncurls-cli/Cargo.toml | 4 +- ribboncurls/CHANGELOG.md | 7 ++ ribboncurls/Cargo.toml | 2 +- ribboncurls/src/lib.rs | 152 +++++++++++++++++------------------ ribboncurls/tests/general.rs | 47 +++++++++++ 7 files changed, 138 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 26378a3..829ca1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -197,7 +197,7 @@ checksum = "7a66a03ae7c801facd77a29370b4faec201768915ac14a721ba36f20bc9c209b" [[package]] name = "ribboncurls" -version = "0.3.0" +version = "0.3.1" dependencies = [ "html-escape", "regex", @@ -208,7 +208,7 @@ dependencies = [ [[package]] name = "ribboncurls-cli" -version = "0.3.1" +version = "0.3.2" dependencies = [ "anyhow", "clap", diff --git a/ribboncurls-cli/CHANGELOG.md b/ribboncurls-cli/CHANGELOG.md index 6dd77e4..336a17a 100644 --- a/ribboncurls-cli/CHANGELOG.md +++ b/ribboncurls-cli/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [0.3.2] - 2024-08-17 + +### Changed + +- Update to latest `ribboncurls` dependency which includes bug fixes + ## [0.3.1] - 2024-07-13 ### Changed @@ -31,6 +37,7 @@ - Initial release +[0.3.2]: https://github.com/tinted-theming/ribboncurls/compare/v0.3.1...v0.3.2 [0.3.1]: https://github.com/tinted-theming/ribboncurls/compare/v0.3.0...v0.3.1 [0.3.0]: https://github.com/tinted-theming/ribboncurls/compare/v0.2.1...v0.3.0 [0.2.1]: https://github.com/tinted-theming/ribboncurls/compare/v0.2.0...v0.2.1 diff --git a/ribboncurls-cli/Cargo.toml b/ribboncurls-cli/Cargo.toml index 54d71ce..09e563a 100644 --- a/ribboncurls-cli/Cargo.toml +++ b/ribboncurls-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ribboncurls-cli" -version = "0.3.1" +version = "0.3.2" edition = "2021" authors = ["Tinted Theming "] license = "MPL-2.0" @@ -19,7 +19,7 @@ serde_yaml = "0.9.33" [dependencies.ribboncurls] path = "../ribboncurls" -version = "0.3.0" +version = "0.3.1" [[bin]] name = "ribboncurls" diff --git a/ribboncurls/CHANGELOG.md b/ribboncurls/CHANGELOG.md index b2bf801..0dc7528 100644 --- a/ribboncurls/CHANGELOG.md +++ b/ribboncurls/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 0.3.1 - 2024-08-17 + +## Fixed + +- Fix bug where data isn't replaced for variables including a `.` + correctly by going up the context stack. + ## 0.3.0 - 2024-06-25 ## Changed diff --git a/ribboncurls/Cargo.toml b/ribboncurls/Cargo.toml index 1e8f8c1..590a858 100644 --- a/ribboncurls/Cargo.toml +++ b/ribboncurls/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ribboncurls" -version = "0.3.0" +version = "0.3.1" edition = "2021" build = "build.rs" authors = ["Tinted Theming "] diff --git a/ribboncurls/src/lib.rs b/ribboncurls/src/lib.rs index 640de54..a8e95bf 100644 --- a/ribboncurls/src/lib.rs +++ b/ribboncurls/src/lib.rs @@ -119,12 +119,12 @@ fn render_syntax_tree( } } SyntaxItem::EscapedVariable(content) => { - if let Some(value) = get_context_value(ctx, content.as_str())? { + if let Some(value) = get_value_from_context(ctx, content.as_str()) { output.push_str(&escape_html(&serde_yaml_value_to_string(value))); } } SyntaxItem::Variable(content) => { - if let Some(value) = get_context_value(ctx, content.as_str())? { + if let Some(value) = get_value_from_context(ctx, content.as_str()) { output.push_str(&serde_yaml_value_to_string(value)); } } @@ -180,31 +180,29 @@ fn render_syntax_tree( // ------------------ ctx.section_path.push(name.to_string()); - let mut section_value_option = None; + let mut section_context_option = None; let mut is_mutating_context_stack = false; let mut iterator_option: Option = None; // Add section context to the ctx.data_stack - if let Some(section_value) = - get_context_value(ctx, &ctx.section_path.join("."))? - { - section_value_option = Some(section_value.clone()); - if matches!(section_value, Value::Mapping(_)) { - ctx.data_stack.push(section_value.clone()); + if let Some(section_context) = get_value_from_context(ctx, name) { + section_context_option = Some(section_context.clone()); + if matches!(section_context, Value::Mapping(_)) { + ctx.data_stack.push(section_context.clone()); is_mutating_context_stack = true; - } else if matches!(section_value, Value::Sequence(_)) { - iterator_option = Some(section_value.clone()); + } else if matches!(section_context, Value::Sequence(_)) { + iterator_option = Some(section_context.clone()); } } // Iterate and render over the sequence match (iterator_option, is_inverted) { - (Some(Value::Sequence(section_value)), false) => { - for item in section_value { + (Some(Value::Sequence(section_context)), false) => { + for item in section_context { ctx.data_stack.push(item); - match (§ion_value_option, is_inverted) { + match (§ion_context_option, is_inverted) { (Some(value), false) => { if is_value_truthy(value) { let section_output = render_syntax_tree(items, ctx)?; @@ -232,7 +230,7 @@ fn render_syntax_tree( } } // Otherwise render without iteration - _ => match (section_value_option, is_inverted) { + _ => match (section_context_option, is_inverted) { (Some(value), false) => { if is_value_truthy(&value) { let section_output = render_syntax_tree(items, ctx)?; @@ -284,88 +282,82 @@ fn serde_yaml_value_to_string(value: &Value) -> String { } } -fn get_context_value<'a>( - ctx: &'a RenderCtx, - path: &str, -) -> Result, RibboncurlsError> { - let context_stack = &ctx.data_stack; - - if path.is_empty() || ctx.data_stack.is_empty() { - return Ok(None); +fn get_value_from_context<'a>(ctx: &'a RenderCtx, path: &str) -> Option<&'a Value> { + let section_path = &ctx.section_path; + let data_stack_len = &ctx.data_stack.len(); + if path.is_empty() { + return None; } - // Return context for "." variables + + // Return context for "." implicit iterator variables if path == "." { - return match (ctx.section_path.last(), context_stack.last()) { - (Some(current_section), Some(context)) => { - let value_option = context.get(current_section); + let current_section_option = section_path.last(); + let latest_context_option = ctx.data_stack.last(); + + return match (current_section_option, latest_context_option) { + (Some(current_section), Some(latest_context)) => { + let value_option = latest_context.get(current_section); if value_option.is_some() { - return Ok(value_option); + return value_option; } - Ok(Some(context)) + Some(latest_context) } - (None, Some(context)) => Ok(Some(context)), - _ => Err(RibboncurlsError::MissingData), - }; - } - - let parts: Vec<&str> = if path == "." { - ctx.section_path.iter().map(|s| s.as_str()).collect() - } else { - path.split('.').collect::>() - }; - - // context_stack index at which the root path value begins - let mut context_stack_start_index: usize = 0; - - // Does root path exist? - for (index, context) in context_stack.iter().enumerate().rev() { - if let Value::Mapping(_) = context { - let value_option = get_value(context, &parts.join(".")); - - match (parts.first(), value_option) { - (Some(first_part), Some(_)) => { - if get_value(context, first_part).is_some() { - context_stack_start_index = index; - break; - } - } - (None, Some(_)) | (None, None) => { - return Err(RibboncurlsError::BadTag); - } - (Some(_), None) => { - continue; + (None, Some(latest_context)) => { + if *data_stack_len == 1 && !matches!(latest_context, Value::Mapping(_)) { + return Some(latest_context); } + None } - } + (Some(_), None) | (None, None) => None, + }; } - // Check for partial path matches on property names - for context in context_stack[context_stack_start_index..].iter().rev() { - // Return if there is a perfect match - if let Some(value) = get_value(context, &parts.join(".")) { - return Ok(Some(value)); + // if `path`'s `a` in `a.b.c.d` doesn't exist in latest context, search up the context stack. + // If it doesn't exist anywhere, assume `"a.b"` is the property name and repeat. Once `a.b` + // property is found, assume `c` in `c.d` is a Mapping, if nothing is found assume `"c.d"` is a + // property name and search + if !ctx.data_stack.is_empty() { + let path_vec = path.split('.'); + let mut possible_path_list = Vec::default(); + let mut path_item_prefix = String::default(); + + for path_item in path_vec { + let new_path_item = if path_item_prefix.is_empty() { + path_item.to_string() + } else { + format!("{}.{}", path_item_prefix, path_item) + }; + possible_path_list.push(new_path_item.clone()); + path_item_prefix = new_path_item; } - // Search for values from the top of the section stack back to the bottom - for index_outer in (0..parts.len()).rev() { - let value_option = get_value(context, parts[index_outer]); - match value_option { - Some(Value::Mapping(_)) | Some(Value::Sequence(_)) => { - return Ok(value_option); - } - _ => { - for index_inner in (index_outer + 1..parts.len()).rev() { - if let Some(value) = context.get(parts[index_inner..].join(".")) { - return Ok(Some(value)); + for possible_path in &possible_path_list { + for context in ctx.data_stack.iter().rev() { + let value_option = context.get(possible_path); + + if let Some(value) = value_option { + if let Some(target_property_name) = + &path.strip_prefix(&format!("{}.", possible_path)) + { + let result = get_value(value, target_property_name); + + if result.is_none() { + return value.get(target_property_name); + } else { + return result; } + } else { + return get_value(context, path); } } } } - } - Ok(None) + None + } else { + None + } } fn get_value<'a>(data: &'a Value, path: &str) -> Option<&'a Value> { @@ -380,7 +372,7 @@ fn get_value<'a>(data: &'a Value, path: &str) -> Option<&'a Value> { return data.get(path); } - // Match if property `a.b.c.d` exists + // Match if property `a` or `a.b.c.d` exists if let Some(data) = data.get(path) { return data.get(path); } diff --git a/ribboncurls/tests/general.rs b/ribboncurls/tests/general.rs index 7db1e5d..eb7294a 100644 --- a/ribboncurls/tests/general.rs +++ b/ribboncurls/tests/general.rs @@ -200,3 +200,50 @@ fn data_with_nested_sections() { assert_eq!(template, "Tinted Theming!"); } + +#[test] +fn prefix_variable_with_dots_is_string() { + let template = "{{#a}}{{#b}}{{c.d.e.f}}{{/b}}{{/a}}"; + let data = r#" + a: + c.d: + e: + f: Tinted Theming! + b: true + "#; + let template = ribboncurls::render(template, data, None).unwrap(); + + assert_eq!(template, "Tinted Theming!"); +} + +#[test] +fn prefix_variable_with_dots_is_string_and_suffix_with_dots_is_string() { + let template = "{{#a}}{{#b}}{{c.d.e.f}}{{/b}}{{/a}}"; + let data = r#" + a: + c.d: + e.f: Tinted Theming! + b: true + "#; + let template = ribboncurls::render(template, data, None).unwrap(); + + assert_eq!(template, "Tinted Theming!"); +} + +#[test] +fn data_exists_in_parent_sections() { + let template = "{{#a}}{{#b}}{{data.exists}}{{moredata.doesntexist}}{{/b}}{{/a}}"; + let data = r#" + a: + data: + exists: true + b: + moredata: + invalid: false + moredata: + true + "#; + let template = ribboncurls::render(template, data, None).unwrap(); + + assert_eq!(template, "true"); +}