Re: [Zope3-dev] Re: [Z3d] 721/ 8 Comment "Handling of empty prefixes in zope.formlib and zope.app.form"
Hi, Jacob Holm wrote: > I have now checked it in on the trunk, and backported to the 3.3 and 3.2 > branches. Yay! > You can tell me if there is anything else I need to do. From the perspective of the collector issue, this is all good. I'll close the issue. (Looks like you even were lucky and the patch applied cleanly to all three branches, cool!) > Thank you for reminding me. In the rush to get everything done before > Christmas I almost forgot. Thank you, for taking the time and getting it in! Christian -- gocept gmbh & co. kg - forsterstraße 29 - 06112 halle/saale - germany www.gocept.com - [EMAIL PROTECTED] - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development signature.asc Description: OpenPGP digital signature ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] Re: [Z3d] 721/ 8 Comment "Handling of empty prefixes in zope.formlib and zope.app.form"
Christian Theune skrev: Hi Jacob, Jacob Holm wrote: I don't have time to check it in now, but I will do it in a few days unless i hear otherwise. I have now checked it in on the trunk, and backported to the 3.3 and 3.2 branches. I didn't see any change on this, if there is anything I can do to help or support your, I'd be happy to do so. You can tell me if there is anything else I need to do. No worries if you're just short on time (we're all on Christmas preparations!), I just wanted to follow up and keep this on the radar. Thank you for reminding me. In the rush to get everything done before Christmas I almost forgot. -- Jacob Holm CTO Improva ApS ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] Re: [Z3d] 721/ 8 Comment "Handling of empty prefixes in zope.formlib and zope.app.form"
Hi Jacob, Jacob Holm wrote: > I don't have time to check it in now, but I will do it in a few days > unless i hear otherwise. I didn't see any change on this, if there is anything I can do to help or support your, I'd be happy to do so. No worries if you're just short on time (we're all on Christmas preparations!), I just wanted to follow up and keep this on the radar. Christian -- gocept gmbh & co. kg - forsterstraße 29 - 06112 halle/saale - germany www.gocept.com - [EMAIL PROTECTED] - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development signature.asc Description: OpenPGP digital signature ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
[Zope3-dev] Re: [Z3d] 721/ 8 Comment "Handling of empty prefixes in zope.formlib and zope.app.form"
Hi, Christian Ah. I didn't look right here too and mistook -3 as the alternative. I've looked at -alt now and I like it more. Sorry about the confusion. Thank you for taking the time to look again. The name 'expandPrefix' doesn't strike me as very obvious, but I couldn't find an alternative. 'expandPrefix' was the least bad name i could think of. If anyone can suggest a better name I am all for changing it. Nothing to say besides that. IMHO enough people already agreed in general, so if I'd be you, I'd go ahead and check it in. I don't have time to check it in now, but I will do it in a few days unless i hear otherwise. Jacob -- Jacob Holm CTO Improva ApS ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
[Zope3-dev] Re: [Z3d] 721/ 8 Comment "Handling of empty prefixes in zope.formlib and zope.app.form"
Hi, Moving discussion from collector to list as discussions fit over here. We can link to the discussion and provide the result in the collector later, I'm not sure whether this needs so much more discussion though. (For people joining the discussion, have a look at the collector entry to catch up if you like.) Collector: Zope 3 ... wrote: > Issue #721 Update (Comment) "Handling of empty prefixes in zope.formlib and > zope.app.form" > Status Pending, community/bug+solution medium > To followup, visit: > http://www.zope.org/Collectors/Zope3-dev/721 > Please look again. I removed the *first* definition of the 'submitted' method. > The second definition is the one that is currently used. It works > because the __get__ method prepends the form prefix to the __name__ of > the result. (And the deleted version does *not* work for the same > reason) Right. I mixed that up, stupid mistake. Everything's alright. ;) >> form.py: >> "Join a number prefixes into a single prefix, or join a number of" >> >> Small typo: "Join a number *of* prefixes ..." > > This tells me that you have been looking at the prefixhandling-3.patch . > What do > you think of the alternative patch I posted? Ah. I didn't look right here too and mistook -3 as the alternative. I've looked at -alt now and I like it more. The name 'expandPrefix' doesn't strike me as very obvious, but I couldn't find an alternative. Nothing to say besides that. IMHO enough people already agreed in general, so if I'd be you, I'd go ahead and check it in. Christian -- gocept gmbh & co. kg - forsterstraße 29 - 06112 halle/saale - germany www.gocept.com - [EMAIL PROTECTED] - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development signature.asc Description: PGP signature signature.asc Description: OpenPGP digital signature ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
Re: [Zope3-dev] Re: [Z3d] 721/ 8 Comment "Handling of empty prefixes in zope.formlib and zope.app.form"
Please ignore the last post, my mailer was too trigger happy. The real post will be there in a few minutes. Christian -- gocept gmbh & co. kg - forsterstraße 29 - 06112 halle/saale - germany www.gocept.com - [EMAIL PROTECTED] - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development signature.asc Description: OpenPGP digital signature ___ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
[Zope3-dev] Re: [Z3d] 721/ 8 Comment "Handling of empty prefixes in zope.formlib and zope.app.form"
Moving discussion from collector to list as discussions fit over here. We can link to the discussion and provide the result in the collector later. (For people joining the discussion, have a look at the collector entry to catch up if you like.) Collector: Zope 3 ... wrote: > Issue #721 Update (Comment) "Handling of empty prefixes in zope.formlib and > zope.app.form" > Status Pending, community/bug+solution medium > To followup, visit: > http://www.zope.org/Collectors/Zope3-dev/721 > Please look again. I removed the *first* definition of the 'submitted' > method. The second definition is the one that is currently used. It works because the __get__ method prepends the form prefix to the __name__ of the result. (And the deleted version does *not* work for the same reason) > >> Some smaller issues that would be nice to change, but I won't insist: >> >> form.txt: >> "For certain use cases it is important to be able to generate forms >> *without* a +prefix. Using an empty string for the prefix omits it >> entirely." >> >> It would be really cool if you give at least one example of thos >> 'certain' use cases there, it will help other people understand >> the motivation better. > > I will see what I can do. > >> form.py: >> "Join a number prefixes into a single prefix, or join a number of" >> >> Small typo: "Join a number *of* prefixes ..." > > This tells me that you have been looking at the prefixhandling-3.patch . > What do you think of the alternative patch I posted? > > = Comment - Entry #7 by ctheune on Dec 11, 2006 12:52 pm > > Nice patch, thanks! > > +1 from me too. Here are some small adjustments you could make. > > There is one small issue that I don't understand (and from my point might > spoil the patch): > > form.py > @@ -570,13 +576,6 @@ > condition = self.condition > return (condition is None) or condition(self.form, self) > > -def submitted(self): > -if not self.available(): > -return False > -form = self.form > -name = "%s.%s" % (form.prefix, self.__name__) > -return name in form.request.form > > Why are you removing this method? It is used throughout the file. > I see that the method was defined twice in the class, but this > is the one that is being used now, and the other one looks like > it wouldn't work correctly, or does it? > > Some smaller issues that would be nice to change, but I won't insist: > > form.txt: > "For certain use cases it is important to be able to generate forms > *without* a +prefix. Using an empty string for the prefix omits it > entirely." > > It would be really cool if you give at least one example of thos > 'certain' use cases there, it will help other people understand > the motivation better. > > form.py: > "Join a number prefixes into a single prefix, or join a number of" > > Small typo: "Join a number *of* prefixes ..." > > = Comment - Entry #6 by jacobholm on Oct 10, 2006 4:00 pm > > > Uploaded: "prefixhandling-alt.patch" > - http://www.zope.org/Collectors/Zope3-dev/721/prefixhandling-alt.patch/view > Alternative patch using better abstractions (has tests). > > This patch differs from the previous only in the changes to > zope.formlib.form. There are two notable differences: > > 1) I extended the 'Widgets' constructor as described in my latest mail to > zope3-dev > (http://mail.zope.org/pipermail/zope3-dev/2006-October/020730.html). This > made the ugly 'prefixlen' function from the other patch irrelevant, and > allowed a check to ensure that all the widgets have the correct form prefix. > Previously, mismatched prefixes would be silently accepted leading to strange > bugs. > > 2) A new 'expandPrefix' function has been added. The idea is to expand a > given prefix into a 'full' prefix that can be directly prepended to another > name or prefix. So I now use 'expandPrefix(prefix)+other' wherever the > original code used 'prefix+'.'+other'. This removes the need for the badly > named 'prefixjoin' function from the other patch. > > The new patch is cleaner IMHO, but both solve the original problem . Which > one should I attempt to check in (if any)? > > = Comment - Entry #5 by poster on Oct 10, 2006 11:15 am > > +1 > > The "submitted" cleanup is different, of course, but also needed. > > Thanks. > > = Comment - Entry #4 by jacobholm on Oct 10, 2006 11:00 am > > > Uploaded: "prefixhandling-3.patch" > - http://www.zope.org/Collectors/Zope3-dev/721/prefixhandling-3.patch/view > New patch including tests. > > = Comment - Entry #3 by BjornT on Oct 9, 2006 5:22 am > > I replied to the list thread with a small review of the patch, since I'm +1 > on providing more control on the names being used as well. (It'd be good
