Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
We so need a 'save draft' feature :|
Patch details:
- fapi conversion.
- Added status messages, comments and language fixes in places.
- Added field sets to certain forms.
- Removed #return_value for checkboxes and radios as the former triggered a fatal error. The default is apparenly 1, which was the value assigned anyhow.
- Translate string form: added a negative weight to the original string.
- drupal_goto -> return
Notes:
-The search string form should probably use sessions to store search criteria. It currently scans $_REQUEST.
-Delete strings possibly need a confirmation form.
Please review,
Thanks
-K
Comment | File | Size | Author |
---|---|---|---|
#10 | 52911_locale.fapi.patch | 41.06 KB | Zen |
#3 | locale.diff | 124.21 KB | Zen |
#1 | locale2_0.patch | 41.09 KB | Zen |
locale2.patch | 41.11 KB | Zen | |
Comments
Comment #1
Zen CreditAttribution: Zen commentedFixed a couple of erroneous indentations.
-K
Comment #2
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedplease don't intermingle bug fixes with reformatting code or fiying typos, this makes it hard to see what the real issue is.
Comment #3
Zen CreditAttribution: Zen commentedPoint taken. Please use attached diff to compare superficial changes.
Thanks
-K
Comment #4
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedSorry, no.
I want a real diff without the formatting and the typo fixes.
Comment #5
Zen CreditAttribution: Zen commentedPoint taken and I agree. But, please make an exception here, as rolling this again is going to be a chore. If you still don't prefer to review this, please consider leaving this up for review and perhaps somebody else will.
I'll make sure that I split the formatting into a separate patch in the future.. I just can't work with unformatted code and can't resist fixing this stuff, however much I try to restrain myself.
Thanks :)
-K
Comment #6
Dries CreditAttribution: Dries commentedI spent 10 minutes reviewing the code, and it looks good to me.
I'd like to see this committed too.
Comment #7
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI'd like to see it committed too. Has anybody tested it?
Comment #8
Dries CreditAttribution: Dries commentedI haven't tested this one; just spent some time looking at the changes.
If someone can spent 10 minutes testing it, it should be ready to go in.
Comment #9
riccardoR CreditAttribution: riccardoR commentedI applayed locale2_0.patch to current CVS and I'm doing some testing. (@Zen: great work!)
I got an empty page (no errors displayed or logged) whenever trying to edit a string translation (?q=admin/locale/string/edit/nnn).
As the original patch is quite long I print my fix here, so it is easy to review.
Thanks and have the best,
Riccardo
Comment #10
Zen CreditAttribution: Zen commentedThanks for the review Riccardo :) I've fixed that.
I also noticed in the resulting function that there was some hocus pocus with substituting textareas with textfields based on the string length (40). This will cause data loss/corruption for strings below 40 characters (which is very possible) that have line breaks in them. Moreover, the recent patch to form.inc strips
\r, \n
from all textfields (not textareas). I've switched this to resize the number of rows based on the word _count_ [12], with a maximum of 10 rows. str_word_count is compatible with PHP >= 4.3.0.Please review again - much appreciated :)
Cheers,
-K
Comment #11
Zen CreditAttribution: Zen commentedPatch still applies cleanly. Affected areas - all admin forms.
Testing my own patch:
That's about it.
Setting to RTC.
Cheers
-K
Comment #12
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedapplied.
Comment #13
Zen CreditAttribution: Zen commented