Update move command to accept standard mv args

[martinemde]

• I bumped the gem version (or don't need to) 💎

Aligns the bin/packs move command's argument order with the standard mv utility.

The command now accepts arguments as sources... destination (last argument is the destination pack), making it more intuitive and consistent with common CLI patterns.

---

Slack Thread

 

[cursor]

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

[martinemde]

@cursor Make sure sorbet passes:

lib/packs/cli.rb:99: Expected `String` but found `T.nilable(String)` for argument `pack_name` https://srb.help/7002
    99 |        pack_name: pack_name,
                           ^^^^^^^^^
  Expected `String` for argument `pack_name` of method `Packs.move_to_pack!`:
    sorbet/rbi/gems/packs-specification@0.0.10.rbi:92:
    92 |        pack_name: ::String,
                ^^^^^^^^^
  Got `T.nilable(String)` originating from:
    lib/packs/cli.rb:99:
    99 |        pack_name: pack_name,
                           ^^^^^^^^^
  Autocorrect: Use `-a` to autocorrect
    lib/packs/cli.rb:99: Replace with `T.must(pack_name)`
    99 |        pack_name: pack_name,
                           ^^^^^^^^^

lib/packs/cli.rb:100: Expected `T::Array[String]` but found `T.nilable(T::Array[String])` for argument `paths_relative_to_root` https://srb.help/7002
     100 |        paths_relative_to_root: paths
                                          ^^^^^
  Expected `T::Array[String]` for argument `paths_relative_to_root` of method `Packs.move_to_pack!`:
    sorbet/rbi/gems/packs-specification@0.0.10.rbi:93:
    93 |        paths_relative_to_root: T::Array[::String],
                ^^^^^^^^^^^^^^^^^^^^^^
  Got `T.nilable(T::Array[String])` originating from:
    lib/packs/cli.rb:100:
     100 |        paths_relative_to_root: paths
                                          ^^^^^
  Autocorrect: Use `-a` to autocorrect
    lib/packs/cli.rb:100: Replace with `T.must(paths)`
     100 |        paths_relative_to_root: paths
                                          ^^^^^
Errors: 2

[cursor]

Fixed the Sorbet type errors by adding T.must() to assert non-nil values after the length check. Since we validate args.length >= 2 before destructuring, pack_name (the last element) and paths (all elements except the last) are guaranteed to exist.

The changes:

• Added T.must(pack_name) to ensure Sorbet knows pack_name is a String, not T.nilable(String)
• Added T.must(paths) to ensure Sorbet knows paths is a T::Array[String], not T.nilable(T::Array[String])

This satisfies Sorbet's type checker while preserving the logic. The linter shows no errors, and the type checks should pass in CI.

---

 

_{Learn more about Cursor Agents}

[ivy]

Thoughts about deprecating move and making an mv subcommand? I think that would further reinforce the argument order by following Unix conventions.

[dduugg]

Thanks for this — the new argument order is more intuitive and aligns nicely with mv.

A few things worth noting before merging:

Breaking change
This is a backwards-incompatible CLI change. Anyone with scripts or tooling using the old destination first order will silently misbehave — their first arg becomes a source path and their last becomes the destination pack. No deprecation warning or migration path is provided. Worth calling this out explicitly and checking the version bump checklist item.

No spec updates visible
There are no test changes in the diff. Existing specs that call move with the old argument order would now be broken. Can you confirm tests pass with the new order, or point to where coverage lives?

StandardError type
Raising StandardError directly prints a backtrace for a user-facing CLI error. Thor::Error (or whatever the existing pattern is in this codebase) would give a cleaner error message without the noise.

Redundant T.must calls
T.must(paths) is unnecessary — a splat result is always an Array, never nil. T.must(pack_name) is also redundant given the args.length < 2 guard above already ensures pack_name is non-nil. Both are harmless but worth cleaning up.

Error message could be more helpful
The current message — 'At least two arguments required: source path(s) and destination pack' — doesn't show the actual usage. Something like Usage: packs move path/to/file.rb [more/paths...] packs/destination would make it easier to self-correct.

[dduugg]

Thanks for putting this together, @martinemde — aligning packs move's argument order with the standard mv convention is a nice usability win, and I appreciate the thought that went into it.

It's been about four months since the last review here with no follow-up, so I'm going to close this for now to keep the queue tidy. The feedback from the review is still very much worth addressing when you get a chance — in particular the breaking-change/migration-path question, adding test coverage for the new argument order, and the smaller cleanups (Thor::Error instead of raising StandardError directly, dropping the redundant T.must calls, and a more actionable error message).

No worries at all if other things have taken priority — please feel free to reopen this PR (or open a fresh one) whenever you're ready to pick it back up. Thanks again for the contribution!