Skip to content

Comments

Upgrade urfave/cli from v2 to v3 to support flexible flag placement#545

Merged
aooohan merged 23 commits intomainfrom
copilot/fix-4bf672c1-1036-41a1-bee8-7ca355a5bca9
Sep 30, 2025
Merged

Upgrade urfave/cli from v2 to v3 to support flexible flag placement#545
aooohan merged 23 commits intomainfrom
copilot/fix-4bf672c1-1036-41a1-bee8-7ca355a5bca9

Conversation

Copy link
Contributor

Copilot AI commented Sep 30, 2025

Upgrade urfave/cli from v2 to v3

This PR upgrades the CLI library from v2 to v3 to enable flags to be placed after arguments (e.g., vfox use java@21 --global).

Progress Checklist

  • Upgrade urfave/cli dependency from v2.27.1 to v3.4.1
  • Update import paths from github.com/urfave/cli/v2 to github.com/urfave/cli/v3
  • Fix API breaking changes from v2 to v3:
    • Changed function signatures from func(*cli.Context) error to func(context.Context, *cli.Command) error
    • Replaced cli.App with cli.Command
    • Updated BoolFlag Action signature to include bool parameter
    • Updated all command files to use new API
  • Build and test all changes - all tests pass
  • Verify the fix works for both command patterns:
    • vfox use --global java@21 (old pattern, still works)
    • vfox use java@21 --global (new pattern, now works)
  • Add tests for use command scope selection and validation
  • Update documentation (English and Chinese) to reflect flexible flag placement
  • Fix Windows build error - removed unused context import

What Changed

Files Modified (24 total):

  • 18 command files updated with new v3 API
  • 2 dependency files (go.mod, go.sum)
  • 2 documentation files (English & Chinese)
  • 1 test file added
  • 1 internal file (manager.go - unchanged)

Key API Changes:

// Function signatures
func cmdFunc(ctx context.Context, cmd *cli.Command) error

// App → Command
app := &cli.Command{}
app.Run(context.Background(), args)

// BoolFlag actions
Action: func(ctx context.Context, cmd *cli.Command, b bool) error

Why This Matters

The v3 library allows flags to be placed either before or after arguments, fixing the reported issue where vfox use java@21 --global didn't work.

User Impact:

  • ✅ More intuitive CLI usage
  • ✅ Backward compatible (old patterns still work)
  • ✅ Better UX aligning with common CLI tools

Verification

All verification tests pass:

# ✓ Build successful (including Windows)
# ✓ All command tests pass (including new use command tests)  
# ✓ Manual testing confirms both patterns work:

# NEW pattern (flags after arguments) - NOW WORKS ✓
$ vfox use java@21 --global
$ vfox unuse java --global

# OLD pattern (flags before arguments) - STILL WORKS ✓  
$ vfox use --global java@21
$ vfox unuse --global java

Testing

  • All existing tests continue to pass
  • New test file cmd/commands/use_test.go adds coverage for use command
  • Manual verification confirms both flag placement patterns work correctly
  • Build succeeds with no warnings or errors on all platforms

Documentation

Updated both English and Chinese documentation to show users can place flags either before or after arguments, with examples of both patterns.

Original prompt

This section details on the original issue you should resolve

<issue_title>[BUG]: global option</issue_title>
<issue_description>Version
The version you are currently using
OS
macOS

Describe the bug

vfox use java@21 --global 

执行后没有报错,只是当前会话生效
必须使用下面这个才可以

vfox use --global java@21 

Screenshots[optional]
If applicable, add screenshots to help explain your problem.
</issue_description>

<agent_instructions>请将cli/v2 升级为 cli/v3. 注意兼容性测试。</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@aooohan 感谢反馈!

vfox使用的是urfave/cli库, 这个是库的问题. 只能命令后面紧接着跟flag才能生效.</comment_new>
<comment_new>@bytemain
urfave/cli support this feature on v3.0

urfave/cli#1568

just add a property to the flag to make it work

CleanShot 2024-03-08 at 13 49 31@2x

@aooohan > `urfave/cli` support this feature on v3.0 > > [urfave/cli#1568](https://github.com/urfave/cli/pull/1568) > > just add a property to the flag to make it work >

Wow, I'll fix this later. 👍

@aooohan Oh damn, we are currently using v2, and it seems like a lot of things need to be changed when upgrading to v3, which is still alpha.

So I suggest that we wait until v3 is released before fixing this issue.</comment_new>

Fixes #88

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits September 30, 2025 06:08
Co-authored-by: aooohan <40265686+aooohan@users.noreply.github.com>
Co-authored-by: aooohan <40265686+aooohan@users.noreply.github.com>
…commands

Co-authored-by: aooohan <40265686+aooohan@users.noreply.github.com>
Copilot AI changed the title [WIP] [BUG]: global option Upgrade urfave/cli from v2 to v3 to support flexible flag placement Sep 30, 2025
Copilot AI requested a review from aooohan September 30, 2025 06:19
@aooohan aooohan marked this pull request as ready for review September 30, 2025 06:20
Copilot AI review requested due to automatic review settings September 30, 2025 06:20
Copy link
Contributor

Copilot AI left a 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 upgrades the urfave/cli library from v2 to v3 to enable flexible flag placement, addressing a usability issue where flags had to be placed before arguments. The upgrade allows natural command patterns like vfox use java@21 --global while maintaining backward compatibility.

  • Upgraded urfave/cli dependency from v2.27.1 to v3.4.1
  • Updated function signatures across all command handlers to match v3 API
  • Added test coverage for use command scope selection and validation

Reviewed Changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 17 comments.

Show a summary per file
File Description
go.mod Updated dependency from urfave/cli/v2 to v3 with version bump and removed indirect dependencies
internal/manager.go Updated import statement to use v3
cmd/cmd.go Updated main CLI app configuration for v3 API changes
cmd/commands/*.go Migrated all 18 command handlers to new v3 function signatures
cmd/commands/use_test.go Added comprehensive test coverage for use command functionality
docs/*.md Updated documentation to reflect flexible flag placement support

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

aooohan and others added 17 commits September 30, 2025 14:24
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Sep 30, 2025

Codecov Report

❌ Patch coverage is 0% with 74 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.11%. Comparing base (f685b43) to head (60e889e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/commands/info.go 0.00% 11 Missing ⚠️
cmd/cmd.go 0.00% 8 Missing ⚠️
cmd/commands/env.go 0.00% 8 Missing ⚠️
cmd/commands/config.go 0.00% 5 Missing ⚠️
cmd/commands/activate.go 0.00% 4 Missing ⚠️
cmd/commands/add.go 0.00% 4 Missing ⚠️
cmd/commands/cd.go 0.00% 4 Missing ⚠️
cmd/commands/install.go 0.00% 4 Missing ⚠️
cmd/commands/unuse.go 0.00% 4 Missing ⚠️
cmd/commands/use.go 0.00% 4 Missing ⚠️
... and 8 more
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #545   +/-   ##
=======================================
  Coverage   21.11%   21.11%           
=======================================
  Files          68       68           
  Lines        5740     5740           
=======================================
  Hits         1212     1212           
  Misses       4439     4439           
  Partials       89       89           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aooohan
Copy link
Member

aooohan commented Sep 30, 2025

@copilot Run go build .
go build .
shell: C:\Program Files\PowerShell\7\pwsh.EXE -command ". '{0}'"

github.com/version-fox/vfox/cmd/commands

Error: cmd\commands\upgrade_win.go:22:2: "context" imported and not used
Error: Process completed with exit code 1.

Co-authored-by: aooohan <40265686+aooohan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 30, 2025

@copilot Run go build .
go build .
shell: C:\Program Files\PowerShell\7\pwsh.EXE -command ". '{0}'"
...

Fixed in 60e889e. The context import was added automatically but not used in the Windows-specific upgrade_win.go file. Removed the unused import and verified the build succeeds.

Copy link
Member

@aooohan aooohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@aooohan aooohan merged commit cac622d into main Sep 30, 2025
4 checks passed
@aooohan aooohan deleted the copilot/fix-4bf672c1-1036-41a1-bee8-7ca355a5bca9 branch September 30, 2025 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: global option

2 participants