On Mon, Nov 17, 2014 at 08:50:54AM -0500, Anne Mulhern wrote:
> + m = GROUP_STR_PARSE.match(group_str)
> + if not m or not GROUPNAME_VALID.match(m.group(1)):
> + return ("", None)
> +
> + gid = None
> + if m.group(3):
> + gid = int(m.group(3))
> +
> + return (m.group(1), gid)
Might be slightly prettier to do something like:
m = GROUP_STR_PARSE.match(group_str)
if not m:
return ("", None)
(group_name, group_id) = m.group(1,3)
if not GROUPNAME_VALID.match(group_name):
return ("", None)
return (group_name, group_id and int(group_id))
I like it the way it is, gid should be None when there isn't one, not
False.
> +GROUP_STR_PARSE =
re.compile(r'^\s*([a-zA-Z0-9._-]+)\s*(\((\d+)\))*')
The * on the end here means that you match things like:
group(GID1)(GID2), as:
>>> z = GROUP_STR_PARSE.match("group(123)(456)"
>>> z.group(0,1,2,3)
('group(123)(456)', 'group', '(456)', '456')
? is probably better than *.
Good point, fixed and I added a test for it as well (ignores any extra
() groups).
> + name, gid = parse_group_str(group)
> + if not name or not group:
> + continue
It's kind of pointless to check "not group" here, since "not
group" implies
"not name". There would be a point in checking "not group" before
calling
parse_group_str().
Whoops. Not sure why I had that there. Fixed.
Thanks.
--
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)