Padraig O'Briain wrote: > > > On 12/04/08 20:09, Brock Pytlik wrote: >> jmr wrote: >>> Padraig - this looks great and really improves the usability of the >>> dialog. >>> >>> In repository.py: >>> >>> 301 + if not misc.valid_auth_url(name): >>> 302 + if "http://".startswith(name): >>> 303 + self.url_error = None >>> Should you not also be allowing https:// as a valid start for a repo >>> url? >>> >> Shouldn't line 302 also be checking that no invalid characters appear >> in the string? > > Is this not what line 301 is doing? That's one of the things it's doing, yes. But on line 302, we know that it's not a valid URL. There are (at least) two possible reasons for that. 1) The user isn't done typing yet, in which case we don't want to tell them there's an error. 2) The user may still be typing, but has entered something that can never be a valid hostname. For example they enter http://##$$%!#$%#%.c I'd like that to be reported as an error (right after they enter the first # ideally).
Does that clarify what I mean? Brock > > I have changed the code to use _misc.valid_proto. See webrev at > http://cr.opensolaris.org/~padraig/ips-5069-v4/ > > Padraig > >> The second part of line 196 in misc.py shows roughly, but probably >> not exactly, what I think the test needs to include. Also, the test >> should probably use _valid_proto in misc.py for the test(s) on line >> 302 since it would be nice not to have to hop around the code if we >> decide to support another protocol in the future. >> >> It's possible that the test on line 301 should be using >> misc.valid_auth_prefix, at least while the user is actively typing, >> but I'm not certain. >> >> Brock >> >>> JR >>> >>> Padraig O'Briain wrote: >>> >>>> The webrev http://cr.opensolaris.org/~padraig/ips-5069-v2/ fixes >>>> bug 5069 Pkg Mgr GUI provides poor error feedback on Repository Add >>>> >>>> We now do the following: >>>> >>>> * Add button is enabled only when both fields have valid strings >>>> * Provide character by character messaging as done now except >>>> as below >>>> * Remove the error graphic, but leave the text red color - the >>>> message is important but the error graphic is a bit much in this >>>> context >>>> * Left-align the message with the text fields; alignment is very >>>> awkward as it crossed the label/field divide. >>>> * Do *not* display the "Name/URL is not specified" messages. The >>>> user can figure out that values are required in each field. >>>> * Do *not* display "URL is not valid" as long as what they are >>>> typing might be valid, i.e., "http://"; is valid. >>>> >>>> >>>> Padraig >>>> ------------------------------------------------------------------------ >>>> >>>> >>>> _______________________________________________ >>>> pkg-discuss mailing list >>>> [email protected] >>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss >>>> >>> >>> _______________________________________________ >>> pkg-discuss mailing list >>> [email protected] >>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss >>> >> _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
