-
Notifications
You must be signed in to change notification settings - Fork 75
fix: improve version extraction logic to prevent false positives #511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves version extraction logic in the version bump script to prevent false positives when matching version fields. The changes make the pattern matching more specific by ensuring only exact version field matches are replaced, rather than matching any field containing "version" in its name.
Key changes:
- Enhanced regex patterns to match version fields more precisely using
^[[:space:]]*version[[:space:]]*=instead ofversion.*=.*" - Added explicit field name validation with
$1 == "version"check - Improved whitespace handling by preserving original indentation
| /^[[:space:]]*version[[:space:]]*=/ { | ||
| if (in_target_module && $1 == "version") { | ||
| match($0, /^[[:space:]]*/) | ||
| indent = substr($0, 1, RLENGTH) | ||
| print indent "version = \"" new_version "\"" | ||
| in_target_module = 0 | ||
| next |
Copilot
AI
Oct 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition $1 == \"version\" will fail when there's leading whitespace because $1 will be the first whitespace-delimited field. Since the pattern already allows leading whitespace with ^[[:space:]]*, lines with indentation will have $1 as an empty string or the indented content won't split correctly. Consider using a pattern match on the line content instead, or strip leading whitespace before the field check.
| if [ -f "$readme_path" ] && grep -q '^[[:space:]]*version[[:space:]]*=' "$readme_path"; then | ||
| local readme_version | ||
| readme_version=$(grep 'version\s*=\s*"' "$readme_path" | head -1 | sed 's/.*version\s*=\s*"\([^"]*\)".*/\1/') | ||
| readme_version=$(awk '/^[[:space:]]*version[[:space:]]*=/ && $1 == "version" { match($0, /"[^"]*"/); print substr($0, RSTART+1, RLENGTH-2); exit }' "$readme_path") |
Copilot
AI
Oct 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as Comment 1: The condition $1 == \"version\" will not work correctly when there's leading whitespace. The first field $1 will be determined by AWK's default field splitting on whitespace, so indented lines will cause the check to fail. Use a regex match or strip whitespace before comparing fields.
| readme_version=$(awk '/^[[:space:]]*version[[:space:]]*=/ && $1 == "version" { match($0, /"[^"]*"/); print substr($0, RSTART+1, RLENGTH-2); exit }' "$readme_path") | |
| readme_version=$(awk '/^[[:space:]]*version[[:space:]]*=/ { match($0, /"[^"]*"/); print substr($0, RSTART+1, RLENGTH-2); exit }' "$readme_path") |
Description
Makes the version extract and replace logic more specific so it wont replace any field that does is not specifically
fieldunder the detected modules.Type of Change
Testing & Validation
bun test)bun fmt)Related Issues
#510