refactor: Refactor compare-ver common custom command #126
Conversation
WalkthroughThis pull request updates the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant CV as compare-ver
participant PV as parse-ver
C->>CV: Call compare-ver(v1, v2)
CV->>PV: Parse v1 string
PV-->>CV: Return [int,...] for v1
CV->>PV: Parse v2 string
PV-->>CV: Return [int,...] for v2
CV->>CV: Compare version segments (pad missing with 0)
CV-->>C: Return comparison result (1, 0, -1)
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Code Review AnalysisIssues Identified:
Recommendations:
Example Improved Code:export def compare-ver [v1: string, v2: string] {
# Parse the version number: remove pre-release and build information,
# only take the main version part, and convert it to a list of numbers
def parse-ver [v: string] {
$v | str downcase | str trim -c v | str trim
| split row - | first | split row . | each { into int }
}
let a = parse-ver $v1
let b = parse-ver $v2
# Compare the major, minor, patch, and any additional parts
# Fill in the missing parts with 0
for i in 0..([($a | length) ($b | length)] | math max) {
let x = $a | get -i $i | default 0
let y = $b | get -i $i | default 0
if $x > $y { return 1 }
if $x < $y { return -1 }
}
# If all parts are equal, return 0
0
}This improved version addresses the identified issues and provides a more robust solution for version comparison. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nu/common.nu (1)
34-53: LGTM! The refactoring improves modularity and readability.The new implementation is more maintainable with a separate
parse-verhelper function and handles version comparison effectively.Consider these improvements:
- Add documentation about expected version format and examples:
# Compare two version number, return `1` if first one is higher than second one, # Return `0` if they are equal, otherwise return `-1` +# Examples: +# compare-ver "1.2.3" "1.2.0" # Returns 1 +# compare-ver "2.0.0" "2.0.0" # Returns 0 +# compare-ver "1.9.9" "2.0.0" # Returns -1 +# Format: Expects semantic version strings (major.minor.patch) +# - Optional 'v' prefix +# - Pre-release suffixes (-beta, -rc, etc.) are ignored +# - Missing segments default to 0
- Add input validation for malformed versions:
def parse-ver [v: string] { + # Validate version format + let is_valid = ($v | str downcase | str trim -c v | str trim + | parse --regex '^(\d+\.)*\d+(-.*)?$' + | length) > 0 + if (not $is_valid) { + error make {msg: $"Invalid version format: ($v)"} + } $v | str downcase | str trim -c v | str trim | split row - | first | split row . | each { into int } }
refactor: Refactor compare-ver common custom command
Summary by CodeRabbit