Skip to content

Conversation

@jonathannorris
Copy link
Member

@jonathannorris jonathannorris commented Aug 13, 2025

Add positional arguments support to get commands

Implements FDN-561 to allow positional arguments as shorthand for the --keys flag in get commands.

What's Changed

Added support for positional arguments in:

  • dvc variables get
  • dvc features get
  • dvc environments get

New Usage Patterns

# Before (still works)
dvc variables get --keys=var-one,var-two

# After (new options)
dvc variables get var-one var-two        # space-separated
dvc variables get var-one,var-two        # comma-separated
dvc features get feature-one feature-two # works for all commands

Key Features

Backward compatible - existing --keys flag usage unchanged
Precedence handling - positional args take priority over --keys
Flexible input - supports both space and comma separation
Updated help - new examples in command documentation
DRY implementation - shared utility function for consistency

Testing

  • Added focused test coverage for new functionality
  • All existing tests continue to pass
  • 187 tests passing with 0 failures

This comment was marked as outdated.

@jonathannorris jonathannorris requested a review from Copilot August 14, 2025 15:11
@jonathannorris jonathannorris force-pushed the feat-fdn-561-positional-args-for-get-commands branch from ac5d7b6 to 2dd0759 Compare August 14, 2025 15:17

This comment was marked as outdated.

@jonathannorris jonathannorris requested a review from Copilot August 14, 2025 15:23
@jonathannorris jonathannorris merged commit 5f991ee into main Aug 14, 2025
8 checks passed
@jonathannorris jonathannorris deleted the feat-fdn-561-positional-args-for-get-commands branch August 14, 2025 15:27
Copy link

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 adds positional argument support to get commands for variables, features, and environments, allowing users to specify keys as space-separated or comma-separated arguments instead of only using the --keys flag.

  • Added a shared utility function parseKeysFromArgs to handle argument parsing with proper precedence
  • Updated three get commands to support positional arguments while maintaining backward compatibility
  • Added comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/utils/parseKeysFromArgs.ts New utility function for parsing keys from various argument formats
src/utils/parseKeysFromArgs.test.ts Test coverage for the utility function
src/commands/variables/get.ts Updated to support positional arguments and new examples
src/commands/variables/get.test.ts Added test for positional argument functionality
src/commands/features/get.ts Updated to support positional arguments and new examples
src/commands/features/get.test.ts Added test for positional argument functionality
src/commands/environments/get.ts Updated to support positional arguments and new examples
src/commands/environments/get.test.ts New test file with comprehensive test coverage
oclif.manifest.json Updated manifest with new command configurations

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

}

if (args.keys) {
// Handle the first positional argument if provided
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

The comment is misleading. The code handles args.keys which is not necessarily the first positional argument, but rather a named argument. Consider updating the comment to accurately reflect what this code does.

Suggested change
// Handle the first positional argument if provided
// Handle the named argument 'keys' if provided

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants