From 0b05639ef1ed5e182a344539709dcdc235dac400 Mon Sep 17 00:00:00 2001 From: Ricardo Aravena Date: Tue, 30 Sep 2025 03:16:18 +0000 Subject: [PATCH 1/2] fix: handle spaces in file paths in Modelfile parsing Fixes issue #207 where Modelfile parsing would fail with 'invalid args' error when file paths contained spaces without quotes. Changes: - Modified parseStringArgs() to join multiple arguments with spaces - This handles unquoted paths like 'CONFIG example workflows/file.json' - Maintains backward compatibility with quoted paths - Updated tests to reflect new behavior - Added comprehensive test coverage for edge cases The fix allows both: - CONFIG "path with spaces/file.json" (quoted - existing behavior) - CONFIG path with spaces/file.json (unquoted - new behavior) Signed-off-by: Ricardo Aravena --- pkg/modelfile/modelfile_test.go | 4 ++-- pkg/modelfile/parser/args_parser.go | 14 ++++++++++---- pkg/modelfile/parser/args_parser_test.go | 5 ++++- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/pkg/modelfile/modelfile_test.go b/pkg/modelfile/modelfile_test.go index 1c4fefb3..1dd301b2 100644 --- a/pkg/modelfile/modelfile_test.go +++ b/pkg/modelfile/modelfile_test.go @@ -1308,8 +1308,8 @@ MODEL model weights.bin CODE inference script.py DOC README file.md `, - expectError: true, - description: "Unquoted paths with spaces should cause parsing errors", + expectError: false, + description: "Unquoted paths with spaces are now handled correctly by joining arguments", }, { name: "quoted_paths_with_spaces", diff --git a/pkg/modelfile/parser/args_parser.go b/pkg/modelfile/parser/args_parser.go index 5f871f22..e89ed007 100644 --- a/pkg/modelfile/parser/args_parser.go +++ b/pkg/modelfile/parser/args_parser.go @@ -18,18 +18,24 @@ package parser import ( "errors" + "strings" ) // parseStringArgs parses the string type of args and returns a Node, for example: // "MODEL foo" args' value is "foo". +// If multiple args are provided (due to unquoted spaces), they are joined with spaces. +// This handles cases like: CONFIG path with spaces/file.json func parseStringArgs(args []string, start, end int) (Node, error) { - if len(args) != 1 { - return nil, errors.New("invalid args") + if len(args) == 0 { + return nil, errors.New("empty args") } - if args[0] == "" { + // Join all arguments with spaces to handle unquoted file paths with spaces + joined := strings.Join(args, " ") + + if joined == "" { return nil, errors.New("empty args") } - return NewNode(args[0], start, end), nil + return NewNode(joined, start, end), nil } diff --git a/pkg/modelfile/parser/args_parser_test.go b/pkg/modelfile/parser/args_parser_test.go index 6cfe112a..6b7fc02e 100644 --- a/pkg/modelfile/parser/args_parser_test.go +++ b/pkg/modelfile/parser/args_parser_test.go @@ -33,8 +33,11 @@ func TestParseStringArgs(t *testing.T) { {[]string{"foo"}, 1, 2, false, "foo"}, {[]string{"bar"}, 3, 4, false, "bar"}, {[]string{}, 5, 6, true, ""}, - {[]string{"foo", "bar"}, 7, 8, true, ""}, + {[]string{"foo", "bar"}, 7, 8, false, "foo bar"}, // Now handles multiple args by joining {[]string{""}, 9, 10, true, ""}, + // Additional test cases for spaces in file paths + {[]string{"path", "with", "spaces/file.json"}, 11, 12, false, "path with spaces/file.json"}, + {[]string{"example", "workflows_Wan2.1/image_to_video_wan_480p_example.json"}, 13, 14, false, "example workflows_Wan2.1/image_to_video_wan_480p_example.json"}, } assert := assert.New(t) From 2f68d8573ff8e38bf13ec78b975c116d80eee92e Mon Sep 17 00:00:00 2001 From: Ricardo Aravena Date: Wed, 1 Oct 2025 00:31:34 +0000 Subject: [PATCH 2/2] refactor: improve whitespace validation and add comprehensive test coverage Address feedback from @gemini-code-assist[bot] review: 1. Enhanced validation in parseStringArgs(): - Use strings.TrimSpace() to reject arguments consisting solely of whitespace - Prevents creating nodes with whitespace-only paths - Maintains preservation of intentional whitespace in valid arguments 2. Added comprehensive test cases for whitespace handling: - Whitespace-only arguments (should be rejected) - Multiple empty string arguments (should be rejected) - Arguments with leading/trailing spaces (should be preserved) - Mixed whitespace scenarios This improves robustness while maintaining backward compatibility and the core functionality of handling unquoted paths with spaces. Signed-off-by: Ricardo Aravena --- pkg/modelfile/parser/args_parser.go | 2 +- pkg/modelfile/parser/args_parser_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/modelfile/parser/args_parser.go b/pkg/modelfile/parser/args_parser.go index e89ed007..4c93c289 100644 --- a/pkg/modelfile/parser/args_parser.go +++ b/pkg/modelfile/parser/args_parser.go @@ -33,7 +33,7 @@ func parseStringArgs(args []string, start, end int) (Node, error) { // Join all arguments with spaces to handle unquoted file paths with spaces joined := strings.Join(args, " ") - if joined == "" { + if strings.TrimSpace(joined) == "" { return nil, errors.New("empty args") } diff --git a/pkg/modelfile/parser/args_parser_test.go b/pkg/modelfile/parser/args_parser_test.go index 6b7fc02e..72cbe182 100644 --- a/pkg/modelfile/parser/args_parser_test.go +++ b/pkg/modelfile/parser/args_parser_test.go @@ -38,6 +38,12 @@ func TestParseStringArgs(t *testing.T) { // Additional test cases for spaces in file paths {[]string{"path", "with", "spaces/file.json"}, 11, 12, false, "path with spaces/file.json"}, {[]string{"example", "workflows_Wan2.1/image_to_video_wan_480p_example.json"}, 13, 14, false, "example workflows_Wan2.1/image_to_video_wan_480p_example.json"}, + // Test cases for whitespace handling + {[]string{" "}, 15, 16, true, ""}, // Whitespace-only argument should be rejected + {[]string{"", ""}, 17, 18, true, ""}, // Multiple empty string arguments should be rejected + {[]string{" a "}, 19, 20, false, " a "}, // Arguments with leading/trailing spaces should be preserved + {[]string{" ", " "}, 21, 22, true, ""}, // Multiple whitespace-only arguments should be rejected + {[]string{" path ", "with", " spaces "}, 23, 24, false, " path with spaces "}, // Mixed whitespace should be preserved } assert := assert.New(t)