Posted by spazio on March 5, 2009 at 10:48am
Jump to:
| Project: | Recipe |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
After I updated Drupal and recipe module, I tried to put in ingredients. Capital letters of ingredients are shown as small letters after saving the recipe. This bug did not exist before. Thanks for continuous development.
Comments
#1
This is normalization of case, which helps avoid duplication of ingredients in the database.. ex: ham and Ham.
Can you provide an example where capitalization is very important? Thanks.
#2
Thanks for the follow-up. Capitalization is important in other languages for example. In many languages some words are always capitalized. In German for example all nouns are capitalized.
#3
OK thanks.
I think the solution will need to take place as one of either:
A) a translation option
B) As a Recipe Admin option (forces capitalization for all visitors, but would do so regardless of their language).
I prefer A to be because it would be "automatic" and not needing a preference, and will happen only for those languages which need it (so English installs could show this properly just for the users browsing with German set as their browser preferred language).
Unfortunately I know little about translation API currently, and "B" is easier for me. I could use some help w/regard to translations - there's several older patches which need updating and merging for example.
I'm not sure how other modules handle this; if no one steps up with a patch or answer I will return to this issue later.
Here's a UNTESTED possibility, and again this forces it on all ingredients regardless of your visitor's preferred language.
The only change is: $ingredients[] = ucfirst($ingredient);
/**
* Loads the ingredients for a recipe
*/
function recipe_load_ingredients($node) {
...
$ingredients = array();
while ($ingredient = db_fetch_object($rs)) {
$ingredients[] = ucfirst($ingredient);
}
return $ingredients;
}
#4
#5
Problem was that our ingredient compare function was actually changing to the ingredient names to lower case which is not required.
We can do a lookup with a caseless value to prevent duplicate ingredients without out modifying the case of new ingredients.
I just committed a fix to dev which leaves the case alone. If you are database has capitalized ingredient names, they should stay that
way when saving. I still have to check the various import functions though.
#6
I am 100% for changes to the code which make i18n and l10n easier and more effective,
I am a little concerned about removing the case normalization-on-save however.
If the first person to add in the "ingredient" does so as "InGrEdIeNt", that will be pretty annoying.
I can see some English speakers abusing capitalization (maybe not as explicitly as the above example).
I wouldn't want to have to add in code to flatten ingredients just because we took it out of the save routine..
Also, Postgres queries are case sensitive, FYI.
Introducing case sensitivity could complicate work on Views or create more steps for the Views designer.
I tend to have a bit of skepticism (maybe too much, I know). Mostly about unknown impacts due to previous SQA experiences.
My opinion is that the data should stay normalized on save.
If we want to capitalize nouns for the German language, it should be done at the display layer - possibly overriding the t() function or using theme hooks or even our own helper function which checks for the browser's advertised locale (or whatever the official Drupal method is).
#7
Problem is there is no normal. Some things need to be capitalized. Making everything lower case precludes trademarked or copywritten? ingredients. The current solution let's the author decide, but let's the admin have the final word. Translating won't help me when I need a capitalized English ingredient. Heck maybe I like all cap ingredients.
PS: no problem for postgres. We still have a case free lookup to prevent case based duplication.
#8
Trademarked names should also be acknowledged with a TM, but let's not go there. :-)
Actually the current solution is the module decides, because it's all standardized to lowercase on saves.
But OK I can see this is not language specific..
Would you consider adding in an admin option then, so admin can decide on policy?
Thinking that would be something like "[radio] Allow mixed-case ingredients" || "[radio] All ingredients saved as lower-case".
The former incorporates your change, and the latter will auto-normalize all ingredient strings to lc on save/add.
The admin option will serve those sites wanting uniformity or to continue things as they have always worked.
#9
1-1/2 Cup red bull
1 oz tabasco®
Just doesn't look right. Firefox even tells me that tabasco is misspelled:)
But OK, we can move back to a setting.
admin -> Make all ingredients lowercase.
I think the default should be "off" because you can always strlower the database in a single query,
whereas fixing it one by one would be... a pain.
#10
Just a side note, the strtolower feature was added by thox in recipe.module 1.48 (9/20/2005) without a mention:)
#11
new admin setting added.
#12
Re: the admin labeling of the preference, vs. what it actually does...
Is "Make all ingredients lowercase" retroactive against current ingredients in the db?
Or does it just make new ingredients save as lower case.
I am not suggesting that it should go back and cleanup... merely pointing out that the current wording may be misunderstood as doing just that..
#13
Ingredient name normalization: *
[ ] Leave as entered
[ ] Convert to lowercase
When recipes are entered, should ingredient names be converted or left alone.
Went with that.
#14
awesome!
#15
Automatically closed -- issue fixed for 2 weeks with no activity.