dm's command line handling could be improved

RESOLVED FIXED

Status

Testing
Mozbase
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: wlach, Assigned: wlach)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Problem 1: The shell command (one of the most useful commands) doesn't handle shell output with "-" in it (because it confuses it with command line options). This prevents many useful commands from being run from the client.
Problem 2: Options for subcommands is not supported. For example, it would be nice if the launchapp command had optional subcommands for intent name and url, so you could specify one without the other. Or we might want to add a "--root" parameter to shell so commands are run as root. The list goes on...

It happens that the argparse module was designed with these use cases in mind. Let's use it in the dm client. argparse was only introduced in python 2.7, but I don't think we need to support older versions of python in the dm client.
Created attachment 737623 [details] [diff] [review]
Improve commandline parsing in dm using argparse
Attachment #737623 - Flags: review?(mcote)

Comment 2

5 years ago
Comment on attachment 737623 [details] [diff] [review]
Improve commandline parsing in dm using argparse

Review of attachment 737623 [details] [diff] [review]:
-----------------------------------------------------------------

Very cool.  Aside from the ugly help output, as discussed, this looks good.

::: mozdevice/mozdevice/dmcli.py
@@ +128,5 @@
> +        self.dm = self.getDevice(dmtype=args.dmtype, hwid=args.hwid,
> +                                 host=args.host, port=args.port,
> +                                 verbose=args.verbose)
> +
> +        ret = args.func(args)

As discussed, it would be really cool if we could get a dict of only the subcommand arguments and pass that via ** to the function.  You would have to store dmtype somehow, though, for sutver(), but that's the only function that needs to access a "global" arg.

@@ +135,5 @@
>  
>          sys.exit(ret)
>  
>      def add_options(self, parser):
> +        parser.add_argument("-v", "--verbose", action="store_true",

Fix indentation of following lines (as with other parser.add_argument() calls below).
Attachment #737623 - Flags: review?(mcote) → review+
Pushed with suggested fixes: https://github.com/mozilla/mozbase/commit/6f3500b59ae400ab5e59edd47b011dfe841e7376
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.