FEDX-812 Improve CLI ergonomics

CONTRIBUTING.md

@@ -0,0 +1,22 @@
+# Contributing
+
+## Supported Dart Versions
+Currently, `dpx` is written to support Dart 2.19 and Dart 3. We will eventually
+drop support for Dart 2, but until then, all dependency ranges need to take this
+into account and all CI checks (static analysis and tests) should pass on both
+major versions of Dart. Additionally, language features that require Dart 3+ are
+not yet used.
+
+## Running Locally
+
+- Install dependencies: `dart pub get`
+- Analysis: `dart analyze`
+- Tests: `dart test`
+- Running:
+  - `dart bin/dpx.dart`
+  - `dart pub global activate -spath .` to activate and `dart pub global run dpx`
+    to run (or just `dpx` if global Dart executables are added to your path).
+- Debugging:
+  - In VS Code, use the `DPX CLI` launch configuration. This will start `dpx` in
+    interactive mode and open a terminal where you can enter the args. This will
+    let you use the debugger and set breakpoints.

README.md

@@ -2,9 +2,8 @@
 
 ## Installation
 
-Until this is published to pub, you'll have to install via Git:
 ```bash
-dart pub global activate -sgit [email protected]:Workiva/dpx.git
+dart pub global activate dpx
 ```
 
 For ease of use, [follow these instructions][dart run from path] to add the
@@ -13,48 +12,33 @@ system cache `bin` directory to your path so that you can run `dpx` directly.
 ## Usage
 
 ```bash
-# Execute a command from <pkg> with the same name as <pkg>
-dpx <pkg> [args...]
-
-# Execute <cmd> from <pkg>.
-# Use if there are multiple executables or if the executable name is different.
-dpx --package=<pkg> <cmd> [args...]
+dpx <package-spec>[:<package-executable>] [-e <executable>] [args...]
 ```
 
-## Command Running
-
-Once the necessary package is installed, dpx will attempt to run the command.
+First, dpx will globally activate the package specified by `<package-spec>`.
+Then it will run a command.
 
-First, it tries to run the command directly, assuming that it is available as an
-executable in the PATH. This works for Dart packages that declare an
-[executable in the pubspec][pubspec executable].
-
-```yaml
-# pubspec.yaml
-name: webdev
-executables:
-  webdev:
-```
+If neither `:<package-executable>` nor `-e <executable>` are specified, dpx will
+run the default package executable from `<package>`. This is equivalent to:
 
 ```bash
-# Installs and runs `webdev` executable in PATH
-dpx webdev
+dart pub global run <package> [args...]
 ```
 
-If that fails, dpx falls back to running the command with `dart pub global run`.
-The expected format of a command run this way is `<pkg>:<cmd>`, where `<pkg>` is
-the name of the Dart package and `<cmd>` is the name of the Dart file in `bin/`,
-minus the `.dart` extension.
-
-Dart lets you omit the `:<cmd>` portion if there's a file with the same name as
-the package.
-
-For other files, dpx lets you omit the `<pkg>` portion since it can be inferred.
+If `:<package-executable>` is specified, dpx will run that executable from the
+installed package. This is equivalent to:
 
 ```bash
-dpx --package=build_runner :graph_inspector
+dart pub global run <package>:<package-executable> [args...]
 ```
 
+If `-e <executable>` is specified, dpx will run `<executable> [args...]`
+directly after installing the package. This allows you to opt-out of the default
+method that uses `dart pub global run`. This may be useful for Dart packages
+that declare an [executable in the pubspec][pubspec executable] that would be
+available in the PATH, or if other executables outside of the package need to be
+used.
+
 ## Exit Status
 
 | Exit Code | Meaning                                              |
@@ -79,9 +63,9 @@ dpx webdev@^3.0.0 [args...]
 
 # Install from custom pub server.
 # Syntax:
-dpx pub@<pub-server>:<pkg>[@<version-constraint] [args...]
+dpx pub@<pub-server>:<package>[@<version-constraint] [args...]
 # Example:
-dpx [email protected]:workiva_nullsafety_migrator@^1.0.0
+dpx [email protected]:dart_null_tools@^1.0.0
 
 # Install from a github repo.
 # Syntax:
@@ -99,11 +83,11 @@ dpx github+ssh:<org>/<repo> [args...]
 # - <path> if the package is not in the root of the repo
 # - <ref> to checkout a specific tag/branch/commit
 # Syntax:
-dpx <git-url>#path:sub/dir,ref:v1.0.2 [args...]
+dpx <git-url>#path=sub/dir,ref=v1.0.2 [args...]
 # Examples:
-dpx github:Workiva/dpx#ref:v0.0.0 --help
-dpx github:Workiva/dpx#path:example/hello
-dpx github:Workiva/dpx#path:example/hello,ref:v0.0.0
+dpx github:Workiva/dpx#ref=v0.1.0 --help
+dpx github:Workiva/dpx#path=example/dpx_hello
+dpx github:Workiva/dpx#path=example/dpx_hello,ref=v0.1.0
 ```
 
 ## Troubleshooting

bin/dpx.dart

@@ -13,16 +13,19 @@ import 'package:io/io.dart';
 
 void main(List<String> args) async {
   final stopwatch = Stopwatch()..start();
+  var logger = Logger.standard();
 
   try {
     final dpxArgs = parseDpxArgs(args);
-    final logger = dpxArgs.verbose ? Logger.verbose() : Logger.standard();
+    if (dpxArgs.verbose) {
+      logger = Logger.verbose();
+    }
 
     PackageSpec spec;
     try {
       spec = PackageSpec.parse(dpxArgs.packageSpec);
-    } on PackageSpecException catch (error) {
-      throw ExitException(ExitCode.usage.code, '$error\n${usage()}');
+    } on PackageSpecException catch (error, stack) {
+      throw ExitException(ExitCode.usage.code, '$error\n${usage()}', stack);
     }
     logger.trace('Parsed package spec "${dpxArgs.packageSpec}" into $spec');
 
@@ -55,21 +58,39 @@ void main(List<String> args) async {
     }
 
     // Finalize the command to run.
-    String? command = dpxArgs.command;
-    if (command == null || command.startsWith(':')) {
+    String exectuable;
+    List<String> executableArgs;
+    String? packageExecutable;
+
+    // If the executable was given explicitly, use it.
+    if (dpxArgs.executable != null) {
+      exectuable = dpxArgs.executable!;
+      executableArgs = dpxArgs.restArgs;
+    } else {
+      // Otherwise, we'll use `dart pub global run` to run a package executable.
+      exectuable = 'dart';
+
+      // Note: this requires that we know the package name.
       if (packageName == null) {
-        throw ExitException(ExitCode.software.code,
-            'Could not infer package name to use as default command.');
+        throw ExitException(
+          ExitCode.software.code,
+          'Could not infer package name, which is needed to run its executables.',
+        );
       }
 
-      if (command == null) {
-        // If command was not explicitly given, default to the package name.
-        command = packageName;
-      } else {
-        // If command starts with `:`, it's shorthand that omits the package name.
-        // Example: dpx --package=build_runner :graph_inspector --> dart pub global run build_runner:graph_inspector
-        command = '$packageName$command';
-      }
+      // If the package spec included a package executable, use that, otherwise
+      // omit that part and let Dart run the package's default executable.
+      packageExecutable = spec.packageExecutable != null
+          ? '$packageName:${spec.packageExecutable}'
+          : packageName;
+
+      executableArgs = [
+        'pub',
+        'global',
+        'run',
+        packageExecutable,
+        ...dpxArgs.restArgs,
+      ];
     }
 
     // Log how long DPX took before handing off to the actual command.
@@ -78,55 +99,42 @@ void main(List<String> args) async {
     stopwatch.stop();
     logger.trace('Took ${dpxTime}s to start command.');
 
-    // First, try to run the command directly, assuming that it's in the PATH.
-    logger.trace('SUBPROCESS: $command ${dpxArgs.commandArgs.join(' ')}');
+    // Run the command.
+    logger.trace('SUBPROCESS: $exectuable ${executableArgs.join(' ')}');
     try {
       final process = await Process.start(
-        command,
-        dpxArgs.commandArgs,
+        exectuable,
+        executableArgs,
         mode: ProcessStartMode.inheritStdio,
       );
       ensureProcessExit(process);
-      exit(await process.exitCode);
+      final dpxExitCode = await process.exitCode;
+      if (packageExecutable != null &&
+          [ExitCode.data.code /* 65 */, ExitCode.noInput.code /* 66 */]
+              .contains(dpxExitCode)) {
+        // `dart pub global run <cmd>` exits with code 65 when the package for
+        // the given <cmd> is not active or code 66 when the file cannot be
+        // found within the package's `bin/`.
+        // These are both equivalent to "command not found".
+        throw ExitException(127, 'dpx: $packageExecutable: command not found');
+      }
+      exit(dpxExitCode);
     } on ProcessException catch (error) {
       if (error.message.contains('No such file')) {
-        // If the command was not found, it may only be available within the
-        // package's `bin/`. Fallback to `dart pub global run`.
-        logger.trace('Command not found, trying `dart pub global run`');
-        final fallbackArgs = [
-          'pub',
-          'global',
-          'run',
-          command,
-          ...dpxArgs.commandArgs,
-        ];
-        logger.trace('SUBPROCESS: dart ${fallbackArgs.join(' ')}');
-        final process = await Process.start(
-          'dart',
-          fallbackArgs,
-          mode: ProcessStartMode.inheritStdio,
-        );
-        ensureProcessExit(process);
-        final dpgrCode = await process.exitCode;
-        if ([ExitCode.data.code /* 65 */, ExitCode.noInput.code /* 66 */]
-            .contains(dpgrCode)) {
-          // `dart pub global run <cmd>` exits with code 65 when the package for
-          // the given <cmd> is not active or code 66 when the file cannot be
-          // found within the package's `bin/`.
-          // These are both equivalent to "command not found".
-          throw ExitException(127, 'dpx: $command: command not found');
-        }
-        exit(await process.exitCode);
+        throw ExitException(127, 'dpx: $exectuable: command not found');
       } else {
         // Otherwise, the command was found but could not be executed.
-        throw ExitException(126, 'dpx: $command: ${error.message}');
+        throw ExitException(126, 'dpx: $packageExecutable: ${error.message}');
       }
     }
-  } on ExitException catch (error) {
-    print(error.message);
+  } on ExitException catch (error, stack) {
+    logger.stderr(error.message);
+    if (logger.isVerbose) {
+      logger.stderr((error.stackTrace ?? stack).toString());
+    }
     exit(error.exitCode);
   } catch (error, stack) {
-    print('Unexpected uncaught exception:\n$error\n$stack');
+    logger.stderr('Unexpected uncaught exception:\n$error\n$stack');
     exit(ExitCode.software.code);
   }
 }

lib/src/args.dart

@@ -1,47 +1,51 @@
+import 'dart:io';
+
 import 'package:args/args.dart';
 import 'package:dpx/src/exit_exception.dart';
 import 'package:dpx/src/version.dart';
 import 'package:io/io.dart';
 
-final argParser = ArgParser(allowTrailingOptions: false)
+final argParser = ArgParser()
   ..addFlag('help', abbr: 'h', negatable: false)
+  ..addFlag('interactive', abbr: 'i', negatable: false, hide: true)
   ..addFlag('verbose', abbr: 'v', negatable: false)
   ..addFlag('version', negatable: false)
   ..addFlag('yes',
       abbr: 'y',
       negatable: false,
       help: 'Install missing packages without prompting.')
   ..addOption(
-    'package',
-    abbr: 'p',
+    'executable',
+    abbr: 'e',
     help:
-        'The package to install. Supports named packages, custom pub servers, version constraints, and git repos with path and ref options.',
-    valueHelp: 'package-spec',
+        'The executable to run. Overrides default behavior of using `dart pub global run ...`',
+    valueHelp: 'executable',
   );
 
 String usage() => '''Usage:
   dpx <package-spec> [args...]
-  dpx --package=<package-spec> <cmd> [args...]
+  dpx <package-spec>:<package-executable> [args...]
+  dpx <package-spec> -e <executable> [args...]
 
 ${argParser.usage}
 
-<package-spec> supports custom pub servers and git sources. See readme for more info: https://github.com/Workiva/dpx#package-sources''';
+<package-spec> supports named packages, custom pub servers, version constraints, and git sources with path and ref options. See readme for more info: https://github.com/Workiva/dpx#package-sources''';
 
 class DpxArgs {
   // Flags
   final bool autoInstall;
   final bool verbose;
 
   // Options/args
-  final String? command;
-  final List<String> commandArgs;
+  final String? executable;
   final String packageSpec;
+  final List<String> restArgs;
 
   DpxArgs({
     required this.autoInstall,
-    required this.command,
-    required this.commandArgs,
+    required this.executable,
     required this.packageSpec,
+    required this.restArgs,
     required this.verbose,
   });
 }
@@ -61,39 +65,28 @@ DpxArgs parseDpxArgs(List<String> args) {
     throw ExitException(ExitCode.success.code, packageVersion);
   }
 
-  // Either a package spec or a command name must be specified as the first
-  // positional arg.
+  if (parsedArgs['interactive'] == true) {
+    stdout.write('Enter the package spec and any additional args:\n> ');
+    final interactiveArgs = stdin.readLineSync()?.split(' ') ?? [];
+    if ({'--interactive', '-i'}.intersection({...interactiveArgs}).isNotEmpty) {
+      throw ExitException(ExitCode.usage.code,
+          'Cannot use --interactive flag in interactive mode.');
+    }
+    return parseDpxArgs(interactiveArgs);
+  }
+
+  // A package spec must be specified as the first positional arg.
   if (parsedArgs.rest.isEmpty) {
     throw ExitException(
         ExitCode.usage.code, '''Must provide at least one positional arg.
 ${usage()}''');
   }
 
-  String? command;
-  List<String> commandArgs;
-  String packageSpec;
-  if (!parsedArgs.wasParsed('package')) {
-    // When `--package` or `-p` is no specified, then we treat the first
-    // positional arg as both the package spec and the command to run. We do
-    // this by assuming that the command to run is the same as the package's
-    // name, which can be obtained from the package spec itself or from the
-    // logs when globally activating the package.
-    packageSpec = parsedArgs.rest.first;
-    commandArgs = parsedArgs.rest.skip(1).toList();
-  } else {
-    // When `--package` or `-p` is specified, then we avoid inferring the
-    // command to run from the package spec and instead require that the first
-    // positional arg be the command.
-    packageSpec = parsedArgs['package'];
-    command = parsedArgs.rest.first;
-    commandArgs = parsedArgs.rest.skip(1).toList();
-  }
-
   return DpxArgs(
     autoInstall: parsedArgs['yes'] == true,
-    command: command,
-    commandArgs: commandArgs,
-    packageSpec: packageSpec,
+    executable: parsedArgs['executable'], // may be null
+    packageSpec: parsedArgs.rest.first, // must be the first positional arg
+    restArgs: parsedArgs.rest.skip(1).toList(),
     verbose: parsedArgs['verbose'] == true,
   );
 }