[webkit-reviews] review denied: [Bug 46407] Allow rebaselines for webkit-patch rebaseline to be chosen : [Attachment 68594] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Mihai Parparita
<mihaip at chromium.org>'s request for review:
Bug 46407: Allow rebaselines for webkit-patch rebaseline to be chosen
https://bugs.webkit.org/show_bug.cgi?id=46407

Attachment 68594: Patch
https://bugs.webkit.org/attachment.cgi?id=68594&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68594&action=review

Good idea.  I'd prefer if we could avoid adding more command line arguments. 
They add complexity, especially for commands that aren't used that often.

> WebKitTools/Scripts/webkitpy/common/system/user.py:77
> -	   result = int(cls.prompt("Enter a number: ")) - 1
> -	   return list_items[result]
> +	   if can_choose_multiple:
> +	       response = cls.prompt("Enter one or more numbers
(comma-separated): ")
> +	       indices = [int(r) - 1 for r in re.split("\s*,\s*", response)]
> +	       return [list_items[i] for i in indices]
> +	   else:
> +	       result = int(cls.prompt("Enter a number: ")) - 1
> +	       return list_items[result]

Do we want better error handling if the user doesn't enter an int?

> WebKitTools/Scripts/webkitpy/tool/commands/rebaseline.py:115
> +	   if options.choose_rebaselines:
> +	       tests_to_update = self._tool.user.prompt_with_list("Which
test(s) to rebaseline:", tests_to_update, can_choose_multiple=True)

Rather than making it a command-line option, can we just have
prompt_with_list+can_choose_multiple make it easy to choose all?  Maybe the
empty string is how you choose all?