I originally considered this, but went with quotes in case anyone wanted to use spaces in their label names. However I just realized that's a moot point as people can already escape spaces.
I don't mind changing the code to just handle escaped commas. Are there any other opinions? - dale On Mon, Jun 21, 2010 at 1:56 PM, Jongki Suwandi <[email protected]> wrote: > An alternative would be to allow escaping commas with backslashes instead > of quotes. That way you don't have to worry about how to quote/unquote from > the shell command line. E.g. > > atest host list --label label0,comma\\,label # > unquoted, so double backslahshes > atest host list --label " label0,comma\,label" # double > quote the entire label argument > atest host list --label 'label0,comma\,label' # single > quotes used > > To parse that would be somethine like > > lst = re.split(r'(?<!\\),', arg) > lst = [s.replace(r'\,', ',') for s in lst] > > -Jongki > > > > On Mon, Jun 21, 2010 at 12:53 PM, Dale Curtis <[email protected]>wrote: > >> Yes, you'll need to escape the quotes unfortunately. E.g. >> >> atest host list --label label0,\"comma,label\" >> >> - dale >> >> >> On Mon, Jun 21, 2010 at 12:50 PM, Jongki Suwandi <[email protected]>wrote: >> >>> If you typed >>> >>> atest host list --label label0,"comma,label" >>> >>> on the shell command line, wouldn't the quotes get consumed by the shell >>> first? >>> >>> -Jongki >>> >>> On Mon, Jun 21, 2010 at 12:39 PM, James Ren <[email protected]> wrote: >>> >>>> (Resending for dalecurtis; Patchwork didn't pick up the patch, and I >>>> suspect it's because it doesn't recognize the file extension) >>>> >>>> All, >>>> >>>> The "host list", "host create", and "job create" command line options >>>> for the atest CLI module do not support labels with commas. In our >>>> deployment we have several labels which contain commas, leading to some bad >>>> parsing when trying to use the CLI tools. I've added quoting support to the >>>> topic_common.item_parse_info.get_values.__get_items(...) function to fix >>>> this. >>>> >>>> For example, previously 'atest host list --label label0,"comma,label"' >>>> would not be parsed properly into two labels. Now the labels are parsed as >>>> 'label0' and 'comma,label'. >>>> >>>> Any option using topic_common.item_parse_info gets this behavior for >>>> free. Specifically for our needs, I've changed the "atest host list" and >>>> "atest job create" methods from using <string>.split(',') to >>>> topic_common.item_parse_info for labels. >>>> >>>> Implementation wise, I modified the >>>> topic_common.item_parse_info.get_values.__get_items(...) regex so that it >>>> wouldn't split up items in quotes and changed the stripping so that leading >>>> and trailing quotes were removed from the split items. The new regex is ([, >>>> ]|\\\".*?\\\".*?) if space splitting is enabled and ([,]|\\\".*?\\\".*?) >>>> otherwise. >>>> >>>> Testing was done through 18 new unit tests, running the gamut of good >>>> quotes and mismatched quotes at the parser level as well as individual unit >>>> tests for the "host list", "host create", and "job create" methods. >>>> >>>> All unit tests pass, but if there are any deployments out there that >>>> were using the CLI tools and have quotes in their labels (or any option >>>> relying on topic_common.item_parse_info), they will no longer parse >>>> properly. Another more minor quibble is consistency from a user point of >>>> view. I've only added support for quotes to the >>>> topic_common.item_parse_info >>>> class, but there are a couple other areas which are still using >>>> <string>.split(',') to parse their options. Specifically status names, host >>>> names, test names, and dependencies in the host.py and job.py files. I've >>>> left these alone for now, but if anyone objects I can roll them in. >>>> >>>> I've attached the patch file for the changes, but if you prefer a more >>>> GUI driven experience, you can see the change list here: >>>> >>>> http://codereview.chromium.org/2740001/show >>>> >>>> Risk: Low-Medium (CLI infrastructure and tests modified) >>>> Visibility: CLI users >>>> >>>> Regards, >>>> >>>> Dale Curtis >>>> Software Engineer in Test @ Google >>>> >>>> Signed-off-by: Dale Curtis <[email protected]> >>>> >>>> _______________________________________________ >>>> Autotest mailing list >>>> [email protected] >>>> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest >>>> >>>> >>> >>> _______________________________________________ >>> Autotest mailing list >>> [email protected] >>> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest >>> >>> >> >
_______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
