817729 - dmcli: values min_args and max_args a little confusing

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Mark Côté [:mcote]]

DMCli.commands uses integers to specify min_args and max_args most of the time, but sometimes it uses None instead of 0.  I think it would be clearer to always use 0; as it stands, it seems to imply that None does something different from 0.  Also if we always use integers, we can simplify the if statement on lines 140-141 to remove the boolean check and just compare arg lengths.

Comment 1

[Mark Côté [:mcote]]

Correction: None *does* have a special meaning for max_args: no limit to the number of arguments.  It still doesn't seem to make sense for min_args, though, so I propose changing all None values to 0 for min_args and putting a comment about the special meaning of None for max_args.

Comment 2

[Mark Côté [:mcote]]

Actually it looks like argument-length enforcement was broken if max_args was 0, since the if statement didn't actually look for None specifically, just any falsey value.

Comment 3

[Mark Côté [:mcote]]

Attachment: Fix and clarify argument-length check

Comment 4

[William Lachance (:wlach)]

Comment on attachment 687948 [details] [diff] [review]
Fix and clarify argument-length check

lgtm

Comment 5

[Mark Côté [:mcote]]

https://github.com/mozilla/mozbase/commit/8d7680eb193c2881c2569b76be0da036e1d33a9b