John Rice wrote: > Brock Pytlik wrote: >> 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). > Brock this is exactly what happens. The input is validated on each key > stroke so: > > h - invalid uri (301), but part of http:// (302) so ok no error > ditto for: ht, htt, http, http:, http:/, http:// - invalid uri (301), > but part of http:// (302) so ok no error > http://# - invalid uri(301), and not part of http:// (302) so display > the error at once and continue to do so as further invalid chars are > entered. Thanks for explaining that, I understand now. LGTM then.
Brock >> >> 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
