Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
few arrays in Drupal defined same array keys twice, for example:
$elements['checkbox']['element'] = array(
'#title' => $this->randomName(), // <<< ~~~~~~~ SAME KEY
'#type' => 'checkbox',
'#required' => TRUE,
'#title' => $this->randomName() // <<< ~~~~~~~ SAME KEY
);
Comment | File | Size | Author |
---|---|---|---|
#29 | duplicate_array_key-1599618-29.patch | 3.75 KB | Albert Volkman |
#26 | duplicate_array_key-1599618-26.patch | 666 bytes | Albert Volkman |
#26 | duplicate_array_key-all-changes-do-not-test.patch | 4.25 KB | Albert Volkman |
#15 | 1599618-duplicate_array_key-14.patch | 4.27 KB | yurtboy |
#14 | 1599618-duplicate_array_key-14.patch | 4.27 KB | yurtboy |
Comments
Comment #1
crifi CreditAttribution: crifi commentedThis isn't the base system.
Comment #2
droplet CreditAttribution: droplet commentedComment #3
droplet CreditAttribution: droplet commentedkill_duplcate_array_key.patch queued for re-testing.
Comment #5
droplet CreditAttribution: droplet commentedComment #6
mr.baileysLooks good to me, except maybe for:
Maybe we should remove the 100 => 3/3 test case instead, since the 200/200 testcase tests specifically for numbers with more digits.
Comment #7
droplet CreditAttribution: droplet commentedComment #9
droplet CreditAttribution: droplet commented#7: 1599618-duplicate_array_key-7.patch queued for re-testing.
Comment #10
mr.baileysLooks good to me, nice clean-up!
Comment #11
Dries CreditAttribution: Dries commentedPatch doesn't seem to apply:
Asking for a re-test.
Comment #12
Dries CreditAttribution: Dries commented#7: 1599618-duplicate_array_key-7.patch queued for re-testing.
Comment #14
yurtboy CreditAttribution: yurtboy commentedApplied patch but it failed but then I modified it to pass locally
After modifications
It applied cleanly at
HEAD is now at 4bd1231 - Patch #1630108 by cosmicdreams: improper reference to Drupal\Database\Query\AlterableInterface.
Comment #15
yurtboy CreditAttribution: yurtboy commentedseems I did not need a take two. I just misunderstood that status message after the comment was posted.
Comment #16
droplet CreditAttribution: droplet commented#15 okay.
Comment #17
chx CreditAttribution: chx commentedThank you for contributing to core but what is going on here?? I see test changes, a new persistent variable introduced, a CSS class changing... omnibus patch :)
Comment #18
droplet CreditAttribution: droplet commented@chx,
look at #0 patch with more diff context :)
eg.
last one overrided previous one
same problem to others.
2 "#title" here :)
Comment #19
chx CreditAttribution: chx commentedAh so. Thanks.
Comment #20
droplet CreditAttribution: droplet commented??
so remove it . anything wrong ?
Comment #21
droplet CreditAttribution: droplet commentedoutput:
bool(true)
Comment #22
chx CreditAttribution: chx commentedErm. Sorry, wrong status.
Comment #23
webchickAll of these seem to make sense (once you read the surrounding code) except:
One is keyed en,en-US, the other en-US,en, so I'm not sure how these are duplicates. Also, the above code notes:
Therefore, I don't think removing this is correct; the similar values seem to be intentional. If this is wrong, feel free to re-open.
Committed and pushed the rest of the patch to 8.x. Nice catches! Will need a port to D7 due to PSR-0.
In the future, though, please make meaningful issue summaries to explain to people what a patch is doing. That ":)" cost me 10-15 minutes of not committing other peoples' patches. :(
Comment #24
droplet CreditAttribution: droplet commentedThere're at different places:
Comment #25
droplet CreditAttribution: droplet commentedtagging Novice,
D8 Patch:
- remove #24 duplicate array key
D7 Patch:
- reroll all changes
Thanks.
Comment #26
Albert Volkman CreditAttribution: Albert Volkman commentedIf we revert 7d41f69046999a2e0b90b24c1a24ecc621a978bd, the second patch will apply. First patch includes only changes requested by @droplet.
Comment #27
droplet CreditAttribution: droplet commentedComment #24 explained why we needed it, patch looks good.
Comment #28
webchickOk, thanks.
Committed and pushed to 8.x. Back to 7.x.
Comment #29
Albert Volkman CreditAttribution: Albert Volkman commentedD7 backport.
Comment #30
droplet CreditAttribution: droplet commented5 same changes. Worked.
Comment #31
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/8bd1079
Comment #33
chrisjlee CreditAttribution: chrisjlee commentedComment #33.0
chrisjlee CreditAttribution: chrisjlee commentedadd summary