Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
#multiple matched $field['multiple'] in D6, which has become $field['cardinality']
Comment | File | Size | Author |
---|---|---|---|
#12 | rename-multiple-to-cardinality-497504-7.patch | 2.18 KB | bjaspan |
#10 | rename-multiple-to-cardinality-497504-10.patch | 17.24 KB | dakala |
#7 | rename-multiple-to-cardinality-497504-7.patch | 2.18 KB | dakala |
#5 | field.attach.inc_.patch | 780 bytes | dakala |
#5 | field.form_.inc_.patch | 1.23 KB | dakala |
Comments
Comment #1
yched CreditAttribution: yched commentedSide note: should be super easy for a Field API novice.
Comment #2
tstoecklerWill have a go at this tomorrow!
Comment #3
tstoecklerWell, tomorrow is gone by now. I'm still on this, theoretically, but my network driver has crapped on my linux system, so I'm back to Windows until that is fixed. Shouldn' take me more than a few days nonetheless. Still, if someone beats me to it, I wouldn't be mad.
Comment #4
dakalaI'd like to give this a go, please:-)
Comment #5
dakalaPlease be gentle, it's my first attempt at creating a patch for core. Only a few occurrences remain but hey, who cares? A patch is a patch:-)
Comment #6
bjaspan CreditAttribution: bjaspan commenteddakala, thanks for taking this on!
Please read http://drupal.org/patch for instructions about how to create core patches for Drupal. Proper patches make reviewing easier, allow the test bot to work, and make it possible to commit the patch when it is ready.
Comment #7
dakalaThanks bjaspan. Here's what I've done - updated my working copy of files, checked my changes and ran cvs diff from the root directory. Here's the output.
Comment #8
yched CreditAttribution: yched commentedPatch looks great, thanks dakala ! I would have expected there would be more ocurrences to replace, but I'm not where I can check myself. Please make sure you did a search / replace on the whole of core.
Comment #9
dakalaI'll do a search on the whole of core and not only the field modules as the issue affects all forms indeed. Then, I'll re-roll the patch.
Comment #10
dakalaThere's loads more occurrences of #multiple and they've all been replaced. The new patch is attached. Thanks.
Comment #11
yched CreditAttribution: yched commentedEr, my awful bad :-), most core occurrences of #multiple in core should remain #multiple.
AAMOF, only the field.attach.inc and field.form.inc hunks are valid changes - er, those are the ones in your patch #7...)
I'm not completely sure about the options.module changes. We do need to keep
'#multiple' =>
at the left of => assignments, but the$element['#muliple']
we read is probably the property renamed to #cardinality in field.form.inc. Would need some manual testing...Comment #12
bjaspan CreditAttribution: bjaspan commented@dakala, yeah, yched gave you bad advice. Go easy on him, he's on vacation. :-)
You need to distinguish between #multiple for Form API (which should stay) and #multiple for Field API (which should change to #cardinality). So your patch in #7 may be right. I'll re-attach it here and set CNR to see what hte testbot thinks.
Comment #13
bjaspan CreditAttribution: bjaspan commentedforgot to change status
Comment #14
dakala@bjaspan, @yched: Thanks guys. Actually, there were a few obvious places I'd left out and I suspected there was something going on between FAPI and Field API that I didn't understand. Thanks for the clarification.
Comment #15
Dries CreditAttribution: Dries commentedMakes sense. Committed.
Comment #16
yched CreditAttribution: yched commentedCool, thanks dakala !
Not where I can test, as bjaspan explained in #12 :-p, but I'd say option.module needs some changes too. The #multiple boolean set on select dropdowns, or the fact we chose radios or checkboxes, should depend on #cardinality.
Thus now raising to critical, because I suspect option.module is currently broken
Comment #17
dakalaOk. I'll look into getting that sorted right away.
Comment #18
dakala@yched: Maybe this should wait till you're able to check?
The 2 functions (options_buttons_elements_process() and options_select_elements_process() in options.module) dealing with #multiple/cardinality seem okay to me. In order to set the #multiple boolean, the ternary expressions check for $element['#multiple'] before $field['cardinality'].
Comment #19
bjaspan CreditAttribution: bjaspan commented@dakala: Your help on this issue has been great and very much appreciated... Thanks!
However, the remaining issue of fixing #multiple vs. #cardinality in exactly the right places is frankly not a Field API novice job. In fact I think yched is probably the only person that knows the right answer. Even if you submitted a patch, I wouldn't really know how to verify it is correct.
There are tests for option.widgets now that make it seem they are working, so this can probably wait until yched gets back from vacation, or at least clues me in here about how to understand #multiple vs. #cardinality in these forms. :-)
Comment #20
yched CreditAttribution: yched commentedGetting back late here.
My bad, I was on crack in #16, everything is at it should. Thanks again, dakala.