Allow commands to be marked as deprecated

[jmagman]

It would be helpful if commands could be marked as deprecated so users know they may soon be removed. This could be indicated automatically in usage text and output a warning when the command is run.

[masterashu]

Hello. I would like to work on this.

[masterashu]

@jmagman I have a minor doubt. When you mentioned

commands could be marked as deprecated

did you meant options and flags or commands or both as well.

[jmagman]

I meant commands. Flags is a good idea, but maybe needs a separate issue.

[natebosch]

It seems more straightforward to me for this to be handled manually. The description field is as easy to update as adding an argument to the command's arg parser, and the warning on usage is best handled in run() since it has the best knowledge of how the user is interacting with the command. print won't always be the best choice.

[natebosch]

It is possible to implement a generic deprecation mechanism for commands outside of this package, but it has some unfortunate downsides.

Implementation

abstract class DeprecatedCommand<T> extends Command<T> {
  @override
  String get description => '${_delegate.description} (deprecated)';

  @override
  ArgResults get globalResults => _delegate.globalResults;

  @override
  bool get hidden => _delegate.hidden;

  @override
  String get invocation => _delegate.invocation;

  @override
  String get name => _delegate.name;

  @override
  Command<T> get parent => _delegate.parent;

  @override
  void printUsage() {
    _delegate.printUsage();
  }

  @override
  FutureOr<T> run() {
    _onUse(_delegate);
    return _delegate.run();
  }

  @override
  CommandRunner<T> get runner => _delegate.runner;

  @override
  Map<String, Command<T>> get subcommands => _delegate.subcommands;

  @override
  String get summary => _delegate.summary;

  @override
  bool get takesArguments => _delegate.takesArguments;

  @override
  String get usage => _delegate.usage;

  @override
  void usageException(String message) {
    _delegate.usageException(message);
  }

  @override
  String get usageFooter => _delegate.usageFooter;
}

void _printDeprecatedWarning(Command command) {
  print('Warning, the command ${command.name} is deprecated');
}

There are a few things I don't like about this:

• You must extends Command, not implements Command, because CommandRunner needs the private setter _runner=.
• There is an ugly hack in there to get around the fact constructing two commands with the same argParser causes an error when it wants to add the 'help' command twice.

If we were to implement this in the same package we could work around those issue. I'm still not convinced though that this is worth the complexity compared to:

• Manually add (deprecated) to the description in the deprecated command.
• Manually add print('Warning this command is deprecated'); in the run().

cc @munificent Do you have any thoughts?

[munificent]

If we were to implement this in the same package we could work around those issue. I'm still not convinced though that this is worth the complexity compared to:

* Manually add `(deprecated)` to the description in the deprecated command.

* Manually add `print('Warning this command is deprecated');` in the `run()`.

+1. Overall, the Command stuff has always felt fairly brittle and hard to use and extend to me. We harvested it from pub but I don't feel we did a good enough job generalizing it for other use cases. Maybe if we can do an overall revamp of the API, that would make it easier for user-level code to do things like extend it to mark a command as deprecated.