fix(cmd): include backslash in path detection for @ file references#38
Open
hsusul wants to merge 1 commit into
Open
fix(cmd): include backslash in path detection for @ file references#38hsusul wants to merge 1 commit into
hsusul wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Observed Behavior
When passing an
@file reference containing backslash path separators (such as@subfolder\\unreadablefileor Windows paths without a file extension) that fails to open,embedFilesValueinpkg/cmd/flagoptions.godid not recognize\\as a path separator. As a result,probablyFile/expectsFileevaluated tofalse, silently falling back to treating the unreadable file path as a raw string literal ("@subfolder\\\\unreadablefile") instead of surfacing a file read error.Root Cause
probablyFileandexpectsFilechecks inpkg/cmd/flagoptions.go(lines 237 and 265) checked for.and/(strings.Contains(filename, ".") || strings.Contains(filename, "/")), but omitted\\(strings.Contains(filename, "\\\\")).Implementation
Updated path detection in
embedFilesValue(pkg/cmd/flagoptions.go) to also check for\\(strings.Contains(filename, "\\\\")), ensuring paths using backslashes are recognized as file references when handling unreadable file errors.Regression Coverage
Added unit test case
"non-existent file with backslash path @ prefix (error)"inpkg/cmd/flagoptions_test.goto assert that unreadable backslash path references properly return a file reading error instead of falling back to a raw string literal.Exact Validation Commands & Results
go test ./pkg/cmd -run TestEmbedFiles-> Passedgo test ./internal/...-> Passedgo vet ./internal/... ./pkg/...-> Passed (clean)git diff --check-> Passed (clean)Limitations or Untested Platforms
None.
Unrelated Changes
No unrelated changes were included.