diff --git a/wdl-ast/CHANGELOG.md b/wdl-ast/CHANGELOG.md index 84824276..b861b3ff 100644 --- a/wdl-ast/CHANGELOG.md +++ b/wdl-ast/CHANGELOG.md @@ -19,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Reduced allocations in stripping whitespace from commands and multiline strings and provided unescaping of escape sequences ([#265](https://github.com/stjude-rust-labs/wdl/pull/265)). +### Fixed +* Fixed bug in `strip_whitespace()` that erroneously stripped characters from the first line when it had content. + Closed [issue #268](https://github.com/stjude-rust-labs/wdl/issues/268) ([#271](https://github.com/stjude-rust-labs/wdl/pull/271)). + ## 0.9.0 - 10-22-2024 ### Changed diff --git a/wdl-ast/src/v1/task.rs b/wdl-ast/src/v1/task.rs index fdbaf6ea..cd4b7938 100644 --- a/wdl-ast/src/v1/task.rs +++ b/wdl-ast/src/v1/task.rs @@ -942,10 +942,12 @@ impl CommandSection { } // Trim the first line + let mut whole_first_line_trimmed = false; if let Some(StrippedCommandPart::Text(text)) = result.first_mut() { - let end = text.find('\n').map(|p| p + 1).unwrap_or(text.len()); - let line = &text[..end]; + let end_of_first_line = text.find('\n').map(|p| p + 1).unwrap_or(text.len()); + let line = &text[..end_of_first_line]; let len = line.len() - line.trim_start().len(); + whole_first_line_trimmed = len == line.len(); text.replace_range(..len, ""); } @@ -987,7 +989,7 @@ impl CommandSection { // Finally, strip the leading whitespace on each line // This is done in place using the `replace_range` method; the method will // internally do moves without allocations - let mut strip_leading_whitespace = true; + let mut strip_leading_whitespace = whole_first_line_trimmed; for part in &mut result { match part { StrippedCommandPart::Text(text) => { @@ -2461,4 +2463,51 @@ task test { "echo \"hello\"\n echo \"world\"\necho \\\n \"goodbye\"" ); } + + /// Regression test for issue [#268](https://github.com/stjude-rust-labs/wdl/issues/268). + #[test] + fn whitespace_stripping_with_content_on_first_line() { + let (document, diagnostics) = Document::parse( + r#" +version 1.2 + +task test { + command <<< weird stuff $firstlinelint + # other weird whitespace + somecommand.py $line120 ~{placeholder} + >>> + }"#, + ); + + assert!(diagnostics.is_empty()); + let ast = document.ast(); + let ast = ast.as_v1().expect("should be a V1 AST"); + let tasks: Vec<_> = ast.tasks().collect(); + assert_eq!(tasks.len(), 1); + + let command = tasks[0].command().expect("should have a command section"); + + let stripped = command.strip_whitespace().unwrap(); + assert_eq!(stripped.len(), 3); + let text = match &stripped[0] { + StrippedCommandPart::Text(text) => text, + _ => panic!("expected text"), + }; + assert_eq!( + text, + "weird stuff $firstlinelint\n # other weird whitespace\nsomecommand.py $line120 " + ); + + let _placeholder = match &stripped[1] { + StrippedCommandPart::Placeholder(p) => p, + _ => panic!("expected placeholder"), + }; + // not testing anything with the placeholder, just making sure it's there + + let text = match &stripped[2] { + StrippedCommandPart::Text(text) => text, + _ => panic!("expected text"), + }; + assert_eq!(text, ""); + } }