low hanging fruit for Drupal 6: variable defaults
Hi, To be able to translate variables to multiple languages in Drupal nicely, we would need a small conceptual change, which would benefit all of the Drupal developers, so I am posting the call here in hopes we have someone or a small group to pick this task up. **We need a central place to define variable defaults** Simple! Now whenever you need a variable, you do variable_get('my_fine_var', 'my_default_value'); The problem with this is that you need to repeat this multiple times, and of course there is a chance you need to modify it later on, so you need to find all places a variable is used. Not good. Remember the changes from bluemarine to garland, we have been fixing theme_default errors for days, finding out places where the variable was used... So we need a central place to define variable defaults. For Drupal 6 this is enough now: hook_settings() { return array( 'my_fine_var' => 'my_default_value', ); } Have fun with naming it hook_settings(), as far as I see, it is an appropriate name, and not taken at the moment :) So Drupal can do a module_invoke_all() on hook_settings() and collect defaults to all variables from the modules defining them. It gets easier and shorter to use variables: variable_get('my_fine_var'); We can either cache the default vars as returned by the callback in the variable_get() function, or in the database among other variables, you get the idea. It is important to have all variables defined in hook_settings(), even if the default value is an empty string or array. Don't care yet about how this fits into translatable variables, because we can only follow up on that if this gets done. So someone please grab this issue and run with it ;) Gabor
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Gabor Hojtsy schrieb:
Hi,
To be able to translate variables to multiple languages in Drupal nicely, we would need a small conceptual change, which would benefit all of the Drupal developers, so I am posting the call here in hopes we have someone or a small group to pick this task up.
**We need a central place to define variable defaults**
Simple! Now whenever you need a variable, you do
variable_get('my_fine_var', 'my_default_value');
The problem with this is that you need to repeat this multiple times, and of course there is a chance you need to modify it later on, so you need to find all places a variable is used. Not good. Remember the changes from bluemarine to garland, we have been fixing theme_default errors for days, finding out places where the variable was used...
So we need a central place to define variable defaults. For Drupal 6 this is enough now:
hook_settings() { return array( 'my_fine_var' => 'my_default_value', ); }
Have fun with naming it hook_settings(), as far as I see, it is an appropriate name, and not taken at the moment :) So Drupal can do a module_invoke_all() on hook_settings() and collect defaults to all variables from the modules defining them. It gets easier and shorter to use variables:
variable_get('my_fine_var');
We should however keep an optional second argument. I don't always use the same second argument, and that's intentional. Cheers, Gerhard -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFGOaSvfg6TFvELooQRAkVJAKCxgx8aoZCi+RlFQ6f+Jmde6J3QDQCfSn3v dD6nu3VJlAmkd7zYcbwS2Tk= =MxzI -----END PGP SIGNATURE-----
Gerhard Killesreiter wrote:
Have fun with naming it hook_settings(), as far as I see, it is an appropriate name, and not taken at the moment :) So Drupal can do a module_invoke_all() on hook_settings() and collect defaults to all variables from the modules defining them. It gets easier and shorter to use variables:
variable_get('my_fine_var');
We should however keep an optional second argument. I don't always use the same second argument, and that's intentional.
Erm, what you do if the user saves his settings into the db, so you have one fixed value to work with? The use case of alternate defaults for variables is not clear to me. Bbut I have nothing against having that supported if you show a good use case, although we would need to hammer people to update their code to use hook_settings() as variable_get() will work as is without modification in Drupal 6. Gabor
One use case is: on the settings page you set the default to a certain value, for example an empty string. But elsewhere in your code, you set the default to NULL, because then you can just do: $var = variable_get('some_var', NULL); foreach ($var as $key => $value) { ..} without having to check if $var is the empty string. Because using NULL will immediately end the foreach loop and an empty string would generate an error. Simple use case, but again a bit shorter! I'll do this one :) Wim On May 3, 2007, at 11:19 , Gabor Hojtsy wrote:
Gerhard Killesreiter wrote:
Have fun with naming it hook_settings(), as far as I see, it is an appropriate name, and not taken at the moment :) So Drupal can do a module_invoke_all() on hook_settings() and collect defaults to all variables from the modules defining them. It gets easier and shorter to use variables:
variable_get('my_fine_var'); We should however keep an optional second argument. I don't always use the same second argument, and that's intentional.
Erm, what you do if the user saves his settings into the db, so you have one fixed value to work with? The use case of alternate defaults for variables is not clear to me. Bbut I have nothing against having that supported if you show a good use case, although we would need to hammer people to update their code to use hook_settings() as variable_get() will work as is without modification in Drupal 6.
Gabor
Wim Leers wrote:
One use case is: on the settings page you set the default to a certain value, for example an empty string. But elsewhere in your code, you set the default to NULL, because then you can just do:
$var = variable_get('some_var', NULL); foreach ($var as $key => $value) { ..}
without having to check if $var is the empty string. Because using NULL will immediately end the foreach loop and an empty string would generate an error. Simple use case, but again a bit shorter!
I'll do this one :)
So you break your code once the user saves the settings page. Do you present this as a viable use case? Gabor
I personally see this as a viable use case, yes. It saves me another if-test. But my example wasn't quite accurate. Suppose you have a multiple select form item. You set the default to -1 for the settings page, and /then/ to NULL in your code. But probably you already figured that yourself. I don't see how exactly this breaks my code? It doesn't break anything, it's pure convenience. Wim On May 3, 2007, at 11:57 , Gabor Hojtsy wrote:
Wim Leers wrote:
One use case is: on the settings page you set the default to a certain value, for example an empty string. But elsewhere in your code, you set the default to NULL, because then you can just do: $var = variable_get('some_var', NULL); foreach ($var as $key => $value) { ..} without having to check if $var is the empty string. Because using NULL will immediately end the foreach loop and an empty string would generate an error. Simple use case, but again a bit shorter! I'll do this one :)
So you break your code once the user saves the settings page. Do you present this as a viable use case?
Gabor
Wim, you said this is cool, because you don't need to check for an empty string, but you would set the default to an empty string for the settings form. So when the user saves the setting form, you get an empty string back from variable_get(), which would show you the error you skipped to check for because you use NULL as a default there. Once the user saves the setting, you can have any number of different default, you will always get the value the user saved. Well, if you mangle with the variable after being submitted from the form, and before being saved (which you did not mention before), it means that you write more code where the form processing takes place (instead of reusing the built in Drupal settings form saver). That moves your code from one place to the other. And you will also need to code around the saving of the localized variables later once we have that included in Drupal. I am fine with having this supported, in case this seems to be making your life easier, although I don't agree that this is easier then not mangling with variable values. Gabor Wim Leers wrote:
I personally see this as a viable use case, yes. It saves me another if-test. But my example wasn't quite accurate. Suppose you have a multiple select form item. You set the default to -1 for the settings page, and /then/ to NULL in your code. But probably you already figured that yourself.
I don't see how exactly this breaks my code? It doesn't break anything, it's pure convenience.
Wim
On May 3, 2007, at 11:57 , Gabor Hojtsy wrote:
Wim Leers wrote:
One use case is: on the settings page you set the default to a certain value, for example an empty string. But elsewhere in your code, you set the default to NULL, because then you can just do: $var = variable_get('some_var', NULL); foreach ($var as $key => $value) { ..} without having to check if $var is the empty string. Because using NULL will immediately end the foreach loop and an empty string would generate an error. Simple use case, but again a bit shorter! I'll do this one :)
So you break your code once the user saves the settings page. Do you present this as a viable use case?
Gabor
Gabor, obviously you're right. I hadn't thought of that immediately, the first example use case was just wrong. But my second example /is/ valid. I'll explain it properly now: 1) On the settings page, you have some select form item, with the multiple attribute set to true. You set the default to 0, -1, whatever. Whatever the user chooses, the result is stored as an /array/. 2) In your actual code, you set the default to NULL. This is necessary, because the foreach you want to use requires an array and not a scalar. So 0, -1, or whatever you used on the settings page, cannot be used here. NULL equals a non-existing array and thus the foreach will stop execution right away. Now, this example makes a lot more sense, I hope :P Note that you don't need any submit or validate handlers in this example, thus zero lines of additional code. Wim On May 3, 2007, at 12:13 , Gabor Hojtsy wrote:
Wim, you said this is cool, because you don't need to check for an empty string, but you would set the default to an empty string for the settings form. So when the user saves the setting form, you get an empty string back from variable_get(), which would show you the error you skipped to check for because you use NULL as a default there. Once the user saves the setting, you can have any number of different default, you will always get the value the user saved.
Well, if you mangle with the variable after being submitted from the form, and before being saved (which you did not mention before), it means that you write more code where the form processing takes place (instead of reusing the built in Drupal settings form saver). That moves your code from one place to the other. And you will also need to code around the saving of the localized variables later once we have that included in Drupal.
I am fine with having this supported, in case this seems to be making your life easier, although I don't agree that this is easier then not mangling with variable values.
Gabor
Wim Leers wrote:
I personally see this as a viable use case, yes. It saves me another if-test. But my example wasn't quite accurate. Suppose you have a multiple select form item. You set the default to -1 for the settings page, and /then/ to NULL in your code. But probably you already figured that yourself. I don't see how exactly this breaks my code? It doesn't break anything, it's pure convenience. Wim On May 3, 2007, at 11:57 , Gabor Hojtsy wrote:
Wim Leers wrote:
One use case is: on the settings page you set the default to a certain value, for example an empty string. But elsewhere in your code, you set the default to NULL, because then you can just do: $var = variable_get('some_var', NULL); foreach ($var as $key => $value) { ..} without having to check if $var is the empty string. Because using NULL will immediately end the foreach loop and an empty string would generate an error. Simple use case, but again a bit shorter! I'll do this one :)
So you break your code once the user saves the settings page. Do you present this as a viable use case?
Gabor
So you break your code once the user saves the settings page. Do you present this as a viable use case?
Besides, if the user doesn't change the #default_value, the variable is not even written to the database. Konstantin Käfer – http://kkaefer.com/
We should however keep an optional second argument. I don't always use the same second argument, and that's intentional.
we now have six years of experience with variable_get(). We rarely rarely use this technique. I don't think we have to design for such rare cases. Just because we can think of a use case does not mean we have to directly support it. My .02 i welcome the centralization of defaults. -moshe
Quoting Moshe Weitzman <weitzman@tejasa.com>:
We should however keep an optional second argument. I don't always use the same second argument, and that's intentional.
we now have six years of experience with variable_get(). We rarely rarely use this technique. I don't think we have to design for such rare cases. Just because we can think of a use case does not mean we have to directly support it. My .02
i welcome the centralization of defaults.
I didn't think of this when responding to Gabor but here is another what if: We make the second argument optional and if not supplied we use the constant DEFAULT__MY_VARIABLE and if both are missing the variable name is used. This gives Gabor a key to use to translate the text and it gives others a smoother transition after six years of using variable_get. Earnie
Quoting Gabor Hojtsy <gabor@hojtsy.hu>:
Hi,
To be able to translate variables to multiple languages in Drupal nicely, we would need a small conceptual change, which would benefit all of the Drupal developers, so I am posting the call here in hopes we have someone or a small group to pick this task up.
**We need a central place to define variable defaults**
At the top of the module: <?php define('MY_DEFAULT_VALUE', t('my_default_value')); ?>
Simple! Now whenever you need a variable, you do
variable_get('my_fine_var', 'my_default_value');
Then in the code: <?php variable_get ('my_find_var', MY_DEFAULT_VALUE); ?>
The problem with this is that you need to repeat this multiple times, and of course there is a chance you need to modify it later on, so you need to find all places a variable is used. Not good. Remember the changes from bluemarine to garland, we have been fixing theme_default errors for days, finding out places where the variable was used...
I then only have one place to edit.
So we need a central place to define variable defaults. For Drupal 6 this is enough now:
The central place is at the top of the module with a define statement.
hook_settings() { return array( 'my_fine_var' => 'my_default_value', ); }
Have fun with naming it hook_settings(), as far as I see, it is an appropriate name, and not taken at the moment :) So Drupal can do a module_invoke_all() on hook_settings() and collect defaults to all variables from the modules defining them. It gets easier and shorter to use variables:
variable_get('my_fine_var');
variable_get('my_fine_var', MY_DEFAULT_VALUE) is simple enough. I don't understand why we would need to add this complication and overhead. Earnie
Any comments regarding default variable namespaces? Right now I have a lot of long variable names just to avoid clashes with my own stuff. variable_get('my_module_name_my_variable_name_var', MY_DEFAULT_VALUE); I was wondering if we should have variables within their own module namespaces. Sort of like local module variables (stored in the db of course) that would be guarrenteed unique??? Also, a side benefit of namespaces might be something like this: $vars = variable_get_array('my_module_name'); which could load all the variables for a module with one db call. On 5/3/07, Earnie Boyd <earnie@users.sourceforge.net> wrote:
Quoting Gabor Hojtsy <gabor@hojtsy.hu>:
Hi,
To be able to translate variables to multiple languages in Drupal nicely, we would need a small conceptual change, which would benefit all of the Drupal developers, so I am posting the call here in hopes we have someone or a small group to pick this task up.
**We need a central place to define variable defaults**
At the top of the module:
<?php define('MY_DEFAULT_VALUE', t('my_default_value')); ?>
Simple! Now whenever you need a variable, you do
variable_get('my_fine_var', 'my_default_value');
Then in the code:
<?php variable_get ('my_find_var', MY_DEFAULT_VALUE); ?>
The problem with this is that you need to repeat this multiple times, and of course there is a chance you need to modify it later on, so you need to find all places a variable is used. Not good. Remember the changes from bluemarine to garland, we have been fixing theme_default errors for days, finding out places where the variable was used...
I then only have one place to edit.
So we need a central place to define variable defaults. For Drupal 6 this is enough now:
The central place is at the top of the module with a define statement.
hook_settings() { return array( 'my_fine_var' => 'my_default_value', ); }
Have fun with naming it hook_settings(), as far as I see, it is an appropriate name, and not taken at the moment :) So Drupal can do a module_invoke_all() on hook_settings() and collect defaults to all variables from the modules defining them. It gets easier and shorter to use variables:
variable_get('my_fine_var');
variable_get('my_fine_var', MY_DEFAULT_VALUE) is simple enough. I don't understand why we would need to add this complication and overhead.
Earnie
-- -> JV
Earnie Boyd wrote:
**We need a central place to define variable defaults**
At the top of the module:
<?php define('MY_DEFAULT_VALUE', t('my_default_value')); ?>
Then in the code:
<?php variable_get ('my_find_var', MY_DEFAULT_VALUE); ?>
I then only have one place to edit. The central place is at the top of the module with a define statement.
hook_settings() { return array( 'my_fine_var' => 'my_default_value', ); }
variable_get('my_fine_var', MY_DEFAULT_VALUE) is simple enough. I don't understand why we would need to add this complication and overhead.
Ernie, I am fine with this solution too for Drupal itself. BUT locale module will require a list of variables and their default values defined somewhere, so we will need to do that ourselfs in this case. It seemed to be odd to do this as part of the hook_locale() introduced today, but if hook_settings() seems to be overcomplication, then we need to go that way. I thought that if - we can save extra typing and value passing on variable_get calls - we have a hook to reuse when we need a list of variables to present for localization adds pluses over just using constants. Sure, only using constants for default variable values could be make variable usage better, but I think a hook do more just by being a central place and not a set of unrelated constants. Gabor
- we can save extra typing and value passing on variable_get calls - we have a hook to reuse when we need a list of variables to present for localization
adds pluses over just using constants.
Sure, only using constants for default variable values could be make variable usage better, but I think a hook do more just by being a central place and not a set of unrelated constants.
i'd like to see this centralized in a hook too. we'll be one step closer to the hierarchy that adrian proposed long ago: http://lists.drupal.org/archives/development/2006-06/msg00309.html
Nice. I hadn't read adrian's thread before. If only I'd searched on 'relm' :) So if we change the variable table to (system.install): db_query("CREATE TABLE {variable} ( namespace varchar(128) NOT NULL default '' name varchar(128) NOT NULL default '', value longtext NOT NULL, language varchar(12) NOT NULL default '', PRIMARY KEY (namespace, name, language) ) /*!40100 DEFAULT CHARACTER SET UTF8 */ "); We'd be halfway there. a patch to variable_get, set, del, and init... in bootstrap.inc and we'd maintain full backward compatibility with the current system, but allow namespace uniqueness and a helper function for mass loading... On 5/3/07, Moshe Weitzman <weitzman@tejasa.com> wrote:
- we can save extra typing and value passing on variable_get calls - we have a hook to reuse when we need a list of variables to present for localization
adds pluses over just using constants.
Sure, only using constants for default variable values could be make variable usage better, but I think a hook do more just by being a central place and not a set of unrelated constants.
i'd like to see this centralized in a hook too. we'll be one step closer to the hierarchy that adrian proposed long ago: http://lists.drupal.org/archives/development/2006-06/msg00309.html
-- -> JV
Quoting Gabor Hojtsy <gabor@hojtsy.hu>:
Earnie Boyd wrote:
**We need a central place to define variable defaults**
At the top of the module:
<?php define('MY_DEFAULT_VALUE', t('my_default_value')); ?>
Then in the code:
<?php variable_get ('my_find_var', MY_DEFAULT_VALUE); ?>
I then only have one place to edit. The central place is at the top of the module with a define statement.
hook_settings() { return array( 'my_fine_var' => 'my_default_value', ); }
variable_get('my_fine_var', MY_DEFAULT_VALUE) is simple enough. I don't understand why we would need to add this complication and overhead.
Ernie, I am fine with this solution too for Drupal itself.
BUT locale module will require a list of variables and their default values defined somewhere, so we will need to do that ourselfs in this case. It seemed to be odd to do this as part of the hook_locale() introduced today, but if hook_settings() seems to be overcomplication, then we need to go that way. I thought that if
- we can save extra typing and value passing on variable_get calls - we have a hook to reuse when we need a list of variables to present for localization
adds pluses over just using constants.
Sure, only using constants for default variable values could be make variable usage better, but I think a hook do more just by being a central place and not a set of unrelated constants.
What if you used get_defined_constants() to return an associative array of constant names? You now have a key to translate the default with. We could go further and add some namespace rules to the naming of constants like DRUPAL__ for Drupal core constants and MY_MODULE__ for module specific namespace. How does this sound? Earnie
Earnie Boyd wrote:
Ernie, I am fine with this solution too for Drupal itself.
BUT locale module will require a list of variables and their default values defined somewhere, so we will need to do that ourselfs in this case. It seemed to be odd to do this as part of the hook_locale() introduced today, but if hook_settings() seems to be overcomplication, then we need to go that way. I thought that if
- we can save extra typing and value passing on variable_get calls - we have a hook to reuse when we need a list of variables to present for localization
adds pluses over just using constants.
Sure, only using constants for default variable values could be make variable usage better, but I think a hook do more just by being a central place and not a set of unrelated constants.
What if you used get_defined_constants() to return an associative array of constant names? You now have a key to translate the default with. We could go further and add some namespace rules to the naming of constants like DRUPAL__ for Drupal core constants and MY_MODULE__ for module specific namespace. How does this sound?
Erm, how would we know that MY_MODULE__ constants are all variable defaults? There are numerous constants in Drupal already. Look into bootstrap.inc for example: we have CACHE_, WATCHDOG_, LANGUAGE_. Or common.inc has SAVED_, file.inc has FILE_. You get the idea. Unless you especially propose the double underscore to signify "constants defined to be variable defaults", we can confuse a lot of constants with being connected to cache.module, saved.module, file.module (if these exist in the system). I don't think a naming standard helps here. As I have said, I am fine with constants, go use them! It still helps possibly multiple defaults, which is a possible place to go wrong in Drupal, and repeat unnecessary code. Gabor
There are times one might try to access a variable defined by another module, such as variable_get('site_name', 'Drupal'); This would make it easier to avoid having to remember later if that's supposed to be variable_get('site_name', 'drupal'), variable_get('site_name', t('Drupal')), variable_get('site_name', SYSTEM_SITE_NAME_DEFAULT), or whatever. I believe just calling variable_get('site_name') would certainly makes things more standard, and easier to maintain. And having a namespace available would be the cherry on top. Aaron Winborn Earnie Boyd wrote:
variable_get('my_fine_var', MY_DEFAULT_VALUE) is simple enough. I don't understand why we would need to add this complication and overhead.
Quoting Aaron Winborn <aaron@culturefix.org>:
There are times one might try to access a variable defined by another module, such as variable_get('site_name', 'Drupal'); This would make it easier to avoid having to remember later if that's supposed to be variable_get('site_name', 'drupal'), variable_get('site_name', t('Drupal')), variable_get('site_name', SYSTEM_SITE_NAME_DEFAULT), or whatever. I believe just calling variable_get('site_name') would certainly makes things more standard, and easier to maintain.
And how do you know that 'site_name' is a variable to get? What happens when I variable_get('not_a_saved_variable')? If the variable is public then it needs to be documented so you would know how to get it and you would know how to supply it's default value. With what is being proposed variable_get would need to try to get the default value of the variable every time. Now it is given so that it doesn't have to try to guess. Why do I need variable_get to try to waste resources by trying to guess the default when I can give it along with the variable? This wasted resource is system wide and affects every module that stores a variable.
And having a namespace available would be the cherry on top.
Something like 'drupal::drupal_variable' and 'my_module::my_variable' might be nice with the default being my_module. Earnie
On 5/3/07, Earnie Boyd <earnie@users.sourceforge.net> wrote:
...
Something like 'drupal::drupal_variable' and 'my_module::my_variable' might be nice with the default being my_module.
Now this solution I like, since you can save and retrieve variables with minimal function arguments. Robin -- Robin Monks @ www.civicspacelabs.org @ www.gmking.org Fax: (419) 791-8076 "Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems." ~IRC
Earnie Boyd wrote:
There are times one might try to access a variable defined by another module, such as variable_get('site_name', 'Drupal'); This would make it easier to avoid having to remember later if that's supposed to be variable_get('site_name', 'drupal'), variable_get('site_name', t('Drupal')), variable_get('site_name', SYSTEM_SITE_NAME_DEFAULT), or whatever. I believe just calling variable_get('site_name') would certainly makes things more standard, and easier to maintain.
And how do you know that 'site_name' is a variable to get? What happens when I variable_get('not_a_saved_variable')? If the variable is public then it needs to be documented so you would know how to get it and you would know how to supply it's default value.
With what is being proposed variable_get would need to try to get the default value of the variable every time. Now it is given so that it doesn't have to try to guess. Why do I need variable_get to try to waste resources by trying to guess the default when I can give it along with the variable? This wasted resource is system wide and affects every module that stores a variable.
Earnie, what is being proposed, is to have a hook to define variable defaults. It was not proposed to have this hook get called *on every variable_get() call*, which you seem to imply with "try to get the default value of the variable every time". Now variable_get() works this way: -> try to get the variable cache from cache_get() -> no cache, select * from variables and save to the cache table -> now we have an associative list of saved variable values -> look up variable name requested -> found, return the value -> not found, return the passed $default With a hook returning variable default values, we can do a whole lot of different things: (a) save the defaults to the variables table, so you get them cached later (b) retrieve the defaults when refreshing the variable cache, and save the defaults to the cache with the variables (c) call the hook every time variable_get is called It seems you imply (c), which is admittedly the least performant solution. I would imply (a) or (b), I really don't care either. An example flow with option (b): -> try to get the variable cache from cache_get() -> no cache, select * from variables *AND* module_invoke_all the variables default hook and save to the cache table -> now we have an associative list of saved variable values -> look up variable name requested -> found, return the value -> not found, return empty string / NULL / FALSE whatever is decided The performance implication of this move is that the variable cache will have more values cached then before. The difference varies depending on how many settings screens did you hit your submit buttons on before. If you have most of your variables saved into the variables table anyway (you submitted most of your settings forms at least once), the difference is negligible. The gain is however that you don't need to specify the default value every time (the *developer* don't need to guess the default value on all variable_get() calls), and as a result, this default value will not get passed every time you call variable_get. It will only be in it's local variable value cache, which only ever changes if you submit a settings form. Gabor
On 03 May 2007, at 14:39, Earnie Boyd wrote:
<?php define('MY_DEFAULT_VALUE', t('my_default_value')); ?>
How about this: define('site_name', 'example'); function variable_get($name) { $value = db_result(db_query(...)); return $value ? $value : constant($name); } print variable_get('site_name'); Might get us the best of both worlds? :) -- Dries Buytaert :: http://www.buytaert.net/
Dries Buytaert wrote:
On 03 May 2007, at 14:39, Earnie Boyd wrote:
<?php define('MY_DEFAULT_VALUE', t('my_default_value')); ?>
How about this:
define('site_name', 'example');
function variable_get($name) { $value = db_result(db_query(...)); return $value ? $value : constant($name); }
print variable_get('site_name');
Might get us the best of both worlds? :)
This is possibly good for the defaults repetition problem, yes. As I have said, we will come out with a way to provide locale related meta information about certain Drupal objects, like content types, where we need to know what is possible to get localized. We will need a list of site settings to localize too, so we will need to have a list of site settings defined by a module defined in that module soon. We can have that in hook_locale(), and we can collect the default values by looking at constants defined by the same names. It seemed to be logical to suggest a more universal hook, but whatever simplification comes out of this thread, we will need stuff like: function system_locale($op = 'groups') { switch($op) { case 'groups': return array('variables' => t('Site settings')); case 'objects': return array('variable' => array('site_name', 'site_logo', ...); } } Note for those not following Drupal 6.x-dev: hook_locale() will be used by the new localization solutions we are working on. Different parts of Drupal will define text groups, like 'Content types', 'Site settings' and so on. You can translate objects in these groups, a content type object having a title, a description, a title_label a body_label and so on. We need a little bit of meta information about these objects, so we know what to translate from them. (hook_locale() is already in core, but only implements the groups $op yet! A detailed explanation of how this works, and deep reasoning comes up tomorrow in the form of a patch, so stay tuned :). So we need a list of what variables are for localization, and we need their default values, so localizers can go and change values even if certain settings forms were not submitted by the original administrator before. It would be quite awkward to tell an administrator to first submit all forms where localized stuff is set, even if nothing needs to be modified there, so we have information about the actual values to localize. So anyway, I am just trying to point out that we will need to have a list of at least the localized variables, and we will need to figure out their defaults. If the current implementation stays in Drupal 6.x, we would need to repeat the defaults in a hook_locale() implementation. If PHP constants are to be used, we would need to look them up ourselfs (but admittedly that would not be hard). A list of variables will still be required. So people trying to get rid of such an idea will not be able to achieve their goal, as far as the current state of Drupal localization stands in our development branch. Gabor
On Thursday 03 May 2007 17:01, Dries Buytaert wrote:
How about this:
define('site_name', 'example');
function variable_get($name) { $value = db_result(db_query(...)); return $value ? $value : constant($name); }
print variable_get('site_name');
What happens if you have a variable whose default is TRUE but for which the set value is FALSE at the moment? I think in that case return $value ? $value : constant($name); is going to give you the constant instead of the perfectly-valid assigned value, isn't it? Scott -- ------------------------------------------------------------------------------- Syscrusher (Scott Courtney) Drupal page: http://drupal.org/user/9184 syscrusher at 4th dot com Home page: http://4th.com/
What happens if you have a variable whose default is TRUE but for which the set value is FALSE at the moment? I think in that case
return $value ? $value : constant($name);
is going to give you the constant instead of the perfectly-valid assigned value, isn't it?
I think Dries was more trying to illustrate a point. It is trivial to change this so it doesn't suffer from this problem: function variable_get($name) { $value = db_query(...); return db_num_rows($value) ? db_result($value) : constant($name); } We've introduced such automatic linking based on naming conventions elsewhere in core too (e.g. menu argument handling, form api callbacks, theme functions, etc), so this idea gets a bit +1 from me. I always liked the idea of centralizing your defaults like this, but I hated that it didn't save you from repeating the defined constant everywhere. Steven Wittens
On 04 May 2007, at 00:52, Steven Wittens wrote:
is going to give you the constant instead of the perfectly-valid assigned value, isn't it?
I think Dries was more trying to illustrate a point. It is trivial to change this so it doesn't suffer from this problem:
function variable_get($name) { $value = db_query(...); return db_num_rows($value) ? db_result($value) : constant($name); }
We've introduced such automatic linking based on naming conventions elsewhere in core too (e.g. menu argument handling, form api callbacks, theme functions, etc), so this idea gets a bit +1 from me. I always liked the idea of centralizing your defaults like this, but I hated that it didn't save you from repeating the defined constant everywhere.
Unfortunately, as pointed out by Gabor, this solution doesn't fly. The problem is that we have dynamic variables: variable_get($node-
type ."_something", ...). A possible solution could be to no longer support dynamic variables. In our running example, we'd then have to add a 'something' field to the node_type database table. However, for a number of use-cases this might be impractical. Then again, it might also prevent that people abuse the variables table as an easy storage system.
Anyway, let's step back for a bit. It seems like we're trying to solve two problems in this thread: 1. For convenience, we want a central place to define a variable's default value. 2. Out of necessity, we want a mechanism to let the locale system translate variables that have dynamic content shown on the UIs (i.e. the footer message). 1 and 2 are somewhat related -- if we had a mechanism to introspect what variables are available, we might be able to extract their default value from a central place, and the locale system could figure out what variables need to be translated. So, each variable should have two properties: (i) a default value and (2) a "dynamic translation"-flag (TRUE if translatable, FALSE if not) and we should be able to poll both. Gabor's original question was: what would be the best implementation? -- Dries Buytaert :: http://www.buytaert.net/
A possible solution could be to no longer support dynamic variables. In our running example, we'd then have to add a 'something' field to the node_type database table. However, for a number of use-cases this might be impractical. Then again, it might also prevent that people abuse the variables table as an easy storage system.
Where do we use dynamic variables in core except for the node type settings storage (which (hopefully) gets tackled in http://drupal.org/ node/111715).
1. For convenience, we want a central place to define a variable's default value.
2. Out of necessity, we want a mechanism to let the locale system translate variables that have dynamic content shown on the UIs (i.e. the footer message).
1 is more or less a side effect of 2 and has not been the initial motivation. Konstantin Käfer – http://kkaefer.com/
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dries Buytaert schrieb:
On 04 May 2007, at 00:52, Steven Wittens wrote:
is going to give you the constant instead of the perfectly-valid assigned value, isn't it?
I think Dries was more trying to illustrate a point. It is trivial to change this so it doesn't suffer from this problem:
function variable_get($name) { $value = db_query(...); return db_num_rows($value) ? db_result($value) : constant($name); }
We've introduced such automatic linking based on naming conventions elsewhere in core too (e.g. menu argument handling, form api callbacks, theme functions, etc), so this idea gets a bit +1 from me. I always liked the idea of centralizing your defaults like this, but I hated that it didn't save you from repeating the defined constant everywhere.
Unfortunately, as pointed out by Gabor, this solution doesn't fly. The problem is that we have dynamic variables: variable_get($node->type ."_something", ...). A possible solution could be to no longer support dynamic variables. In our running example, we'd then have to add a 'something' field to the node_type database table. However, for a number of use-cases this might be impractical. Then again, it might also prevent that people abuse the variables table as an easy storage system.
Yeah, that's a major nuissance. Can't we add a "no cache" bit to the variables table? Variables marked such, would not get added to the cached version (and bloat it) but would be retrieved from the table each time when needed. This would be perfectly acceptable for e.g. the password reminder mail templates. I imagine it would be ok to set the value of this bit based on the length of the stored string. We already do something similar for the locale cache where we do not store strings longer than 75 chars in the cached version. The result of this would be: 1) better performance for most pages (the ones that don't require one of the long strings) 2) somewhat worse performance for a few pages. 3) no guilt trips for abusing the variable_get/set functions once more. :p Cheers, Gerhard -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFGOzSufg6TFvELooQRAk1AAKDGM4iJeAv3SSQN8i0anR4ZzqxEaACguCsk qKjxuef3YFPjfMJihVS4ZGw= =NyLR -----END PGP SIGNATURE-----
we tried this and Dries was not amenable at the time. Maybe a few more opinions and modifications would help. See http://drupal.org/node/79008
Yeah, that's a major nuissance. Can't we add a "no cache" bit to the variables table? Variables marked such, would not get added to the cached version (and bloat it) but would be retrieved from the table each time when needed. This would be perfectly acceptable for e.g. the password reminder mail templates.
I imagine it would be ok to set the value of this bit based on the length of the stored string. We already do something similar for the locale cache where we do not store strings longer than 75 chars in the cached version.
The result of this would be:
1) better performance for most pages (the ones that don't require one of the long strings)
2) somewhat worse performance for a few pages.
3) no guilt trips for abusing the variable_get/set functions once more. :p
On 04 May 2007, at 16:17, Moshe Weitzman wrote:
we tried this and Dries was not amenable at the time. Maybe a few more opinions and modifications would help. See http://drupal.org/ node/79008
I would be amenable if there were some benchmark results. If the motivation is performance, that show me that it matters. How much time is spent loading the variable data from the cache? -- Dries Buytaert :: http://www.buytaert.net/
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dries Buytaert schrieb:
On 04 May 2007, at 16:17, Moshe Weitzman wrote:
we tried this and Dries was not amenable at the time. Maybe a few more opinions and modifications would help. See http://drupal.org/node/79008
I would be amenable if there were some benchmark results. If the motivation is performance, that show me that it matters. How much time is spent loading the variable data from the cache?
I think the main advantage would be having a smaller result set to pull from mysql for each non-cached page request. Drupal.org's variable table has currently 333 items and the variable cache is about 30k. Of the 333 items, 62 (or about 20%) store more than 30 characters, 40 more than 70. The sum of all characters in the variable table is 20330 (apparently serializing adds 50% storage requirement in this case). The sum of the 40 strings which are longer than 70 characters is 16530 or more then 75% of the total. Even if serializing adds more overhead to short strings, we should be able to cut the 30kb to 15kb if we elect to simply not put long strings into the cached version of the table. Now the question is: How significant is this? To evaluate this we'd need to look at the total data transfer from the database to the webserver and a number of other variables. I don't expect that out usual "ab -n100 -c5" test would show a significant difference, but on a website with a lot of visitors it might help. Cheers, Gerhard -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFGPMRffg6TFvELooQRAsaZAJ4gpyLXHQrLrXFHWdn66GZbCFG2owCfQFMT LSaMSzW2/bhogvbUfRNEQYA= =VEU6 -----END PGP SIGNATURE-----
On Friday 04 May 2007, Dries Buytaert wrote:
Anyway, let's step back for a bit. It seems like we're trying to solve two problems in this thread:
1. For convenience, we want a central place to define a variable's default value.
2. Out of necessity, we want a mechanism to let the locale system translate variables that have dynamic content shown on the UIs (i.e. the footer message).
1 and 2 are somewhat related -- if we had a mechanism to introspect what variables are available, we might be able to extract their default value from a central place, and the locale system could figure out what variables need to be translated.
So, each variable should have two properties: (i) a default value and (2) a "dynamic translation"-flag (TRUE if translatable, FALSE if not) and we should be able to poll both. Gabor's original question was: what would be the best implementation?
Because the variables could change at runtime, I think a hook_variables() (or similar) would be the best approach. Core may not use more than just node type, but I know some contribs do, like form IDs. hook_variables() { $items = array(); $items['mymod_num_things'] = array( '#default_value' => 2, // Same syntax as FAPI uses '#realm' => 'mymod', // optional '#translatable' => FALSE, // probably default false '#cacheable' => TRUE, // default to TRUE if under X chars when serialized '#serialize' => FALSE, // default to true for object/array, else false ); foreach (node_get_types('name') as $type) { $items['mymod_things_for_' . $type] = array( '#default_value' => array(), ); } return $items; } I kinda like the idea of using FAPI syntax here, as then system_settings_form() can fill in defaults for us if they're not specified. (I suppose it could anyway, but meh. <g>) I leave that to Gabor to decide. I'm undecided on whether non-cached big variables should still be considered variables or should have some other namespace/table. I guess that depends on how expensive it would be to make variable_get() handle both cases. -- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson
+1 to this. maybe slightly more complex to set up, but declaring variables forces some thought into structure, helps keep them centralized in code, easily extensible. of course, this opens up the can-of-worms possibility of hook_variables_alter... :P Larry Garfield wrote:
hook_variables() { $items = array(); $items['mymod_num_things'] = array( '#default_value' => 2, // Same syntax as FAPI uses '#realm' => 'mymod', // optional '#translatable' => FALSE, // probably default false '#cacheable' => TRUE, // default to TRUE if under X chars when serialized '#serialize' => FALSE, // default to true for object/array, else false ); foreach (node_get_types('name') as $type) { $items['mymod_things_for_' . $type] = array( '#default_value' => array(), ); }
return $items; }
Anyway, let's step back for a bit. It seems like we're trying to solve two problems in this thread:
1. For convenience, we want a central place to define a variable's default value.
2. Out of necessity, we want a mechanism to let the locale system translate variables that have dynamic content shown on the UIs (i.e. the footer message).
While I agree with what the problems are, I disagree with trying to solve item 2 with variables. variable_get and set are an elegantly simple way of storing configuration data about how your application should behave. If we need a way to translate data for the UI, I think it should be stored in a different place, using different structures. UI's are usually tied to page loads formid and such and as such have different caching needs. Loading a lot of complex meaning onto the basic concept of a variable seems like unnecessary complexity to me. You want to be able to give users the ability to alter much of this translation data as possible, but you don't want them mucking around with variables. I recently created a formdefaults module to hand off the altering of what I felt were translatable UI components to the Site admins. I thought about storing this data in variables, but ultimately decided against it, because I didn't want to intermingle UI data with variables. By the way, I'd love to see if there was a slick way to tie my formdefaults module into a translation system so that it was language aware. Any takers on helping? 1. Seems elegantly solvable using DEFINE and using the normal module namespace conventions. It's also the way most programming languages deal with this simple problem. define('EXAMPLEMODULE_MYVALUE','www.example.com');
David Metzler wrote:
Anyway, let's step back for a bit. It seems like we're trying to solve two problems in this thread:
1. For convenience, we want a central place to define a variable's default value.
2. Out of necessity, we want a mechanism to let the locale system translate variables that have dynamic content shown on the UIs (i.e. the footer message).
While I agree with what the problems are, I disagree with trying to solve item 2 with variables. variable_get and set are an elegantly
David, you seem like confusing "translating what we have in variables already" with "translating things in Drupal with variables". We are definitely not solving translation problems with storing translations of Drupal stuff in variables.
simple way of storing configuration data about how your application should behave. If we need a way to translate data for the UI, I think it should be stored in a different place, using different structures. UI's are usually tied to page loads formid and such and as such have different caching needs. Loading a lot of complex meaning onto the basic concept of a variable seems like unnecessary complexity to me. You want to be able to give users the ability to alter much of this translation data as possible, but you don't want them mucking around with variables.
We are about to let the user be able to translate things like "site footer", "site slogan", "user welcome mail" and so on, all stored in variables.
I recently created a formdefaults module to hand off the altering of what I felt were translatable UI components to the Site admins. I thought about storing this data in variables, but ultimately decided against it, because I didn't want to intermingle UI data with variables.
Yes, it is a bad idea to abuse the variables system to store unrelated data.
By the way, I'd love to see if there was a slick way to tie my formdefaults module into a translation system so that it was language aware. Any takers on helping?
We are about to propose a basic object based translation system, watch the issue queue.
1. Seems elegantly solvable using DEFINE and using the normal module namespace conventions. It's also the way most programming languages deal with this simple problem.
define('EXAMPLEMODULE_MYVALUE','www.example.com');
Please read up on previous discussion about why this is problematic. Gabor
We are about to let the user be able to translate things like "site footer", "site slogan", "user welcome mail" and so on, all stored in variables.
Thanks, that helps. I misunderstood what Dries meant by footer, but isn't this possible simply by passing the result of variable_get through t()? Or am I missing something? I'm pretty unfamiliar with translate API's. I still think the separation is good, but that's my own opinion. I still maintain a nervousness around the results of variable_get perhaps being translated without my explicitly asking for it in code. The choice of the "default" behavior will be important. Or the work involved in setting up arrays and hooks for every setting I need to define and tell drupal wether its translatable. What about providing a variable_get_t call that would automatically pass this through the new translation API... or am I oversimplifying here. Not trying to be difficult, but just trying to understand better the impact on module developers.
We are about to propose a basic object based translation system, watch the issue queue.
I'll certainly do so.... but doesn't this mean that we're pursuing a generic solution to translations that could be applied here? Is there an easy way to watch the issue queue for just this
1. Seems elegantly solvable using DEFINE and using the normal module namespace conventions. It's also the way most programming languages deal with this simple problem. define('EXAMPLEMODULE_MYVALUE','www.example.com');
I've read every post on this topic (hours of reading, but luckily was sick and had spare time to kill) and perhaps again have misunderstood. The scope of this discussion has crept a great deal. If in the end, the patch goes in with the item array syntax, perhaps we could write some helper functions that would make something close to the "define" syntax available for module developers. I'll volunteer to write the functions when we get there :).
David Metzler wrote:
We are about to let the user be able to translate things like "site footer", "site slogan", "user welcome mail" and so on, all stored in variables.
Thanks, that helps. I misunderstood what Dries meant by footer, but isn't this possible simply by passing the result of variable_get through t()? Or am I missing something? I'm pretty unfamiliar with translate API's.
t() is not fitting for translating user defined values. There are multitude of problems with using t(). Here is some starter: http://groups.drupal.org/node/1827
I still think the separation is good, but that's my own opinion. I still maintain a nervousness around the results of variable_get perhaps being translated without my explicitly asking for it in code. The choice of the "default" behavior will be important. Or the work involved in setting up arrays and hooks for every setting I need to define and tell drupal wether its translatable.
What about providing a variable_get_t call that would automatically pass this through the new translation API... or am I oversimplifying here.
Not trying to be difficult, but just trying to understand better the impact on module developers.
Most of what Drupal returns now is translated. Drupal has a concept of a language used to display a page. The default will probably be that variable_get() and friends return values in the page language used. We intend to support asking for values in other languages on demand.
We are about to propose a basic object based translation system, watch the issue queue.
I'll certainly do so.... but doesn't this mean that we're pursuing a generic solution to translations that could be applied here? Is there an easy way to watch the issue queue for just this
Watch this node for multilanguge updates from time to time (quite frequently :) http://groups.drupal.org/node/3714
1. Seems elegantly solvable using DEFINE and using the normal module namespace conventions. It's also the way most programming languages deal with this simple problem. define('EXAMPLEMODULE_MYVALUE','www.example.com');
I've read every post on this topic (hours of reading, but luckily was sick and had spare time to kill) and perhaps again have misunderstood. The scope of this discussion has crept a great deal. If in the end, the patch goes in with the item array syntax, perhaps we could write some helper functions that would make something close to the "define" syntax available for module developers. I'll volunteer to write the functions when we get there :).
Multiple people pointed out that constants are not viable if you have variables named dynamically. Maybe you missed this point in the thread. Gabor
Most of what Drupal returns now is translated. Drupal has a concept of a language used to display a page. The default will probably be that variable_get() and friends return values in the page language used. We intend to support asking for values in other languages on demand.
This is precisely what I was concerned about. Although is is certainly common to use variables for storing things like site slogans, site names, etc. Much of the data stored in variables is not textual in nature and is not in fact designed to be alterable, regardless of the language used. What does it mean to translate an array, or an integer for example? There are countless examples of code that tests for text values, radio buttons, array selections, etc. I would hate to see those chunks of code start misbehaving because the values they were testing against in If's, foreaches, cases, etc. got translated, cause the coder didn't know better and didn't test for a translated case. I realize that we could put the burden on the developer to make sure to intialize all their data structures appropriately as NON_TRANSLATABLE, but it seems like a lot of overhead when writing code, (Hey all I wanted was a persistant configuration variable). I'm concerned that this will make contrib modules less stable in the end. Wouldn't it be better to store translatable text strings somewhere else entirely? And then make site slogans, etc store data in these locations instead? Make the data domain allowed to be stored in these cases be textual only (not serialized structured data). I don't want to clog the airwaves, so I'll humbly step out of this debate if I'm alone in this concern. Dave P.S. I get the problem with define now. Thanks for the synopsis.
David Metzler wrote:
Most of what Drupal returns now is translated. Drupal has a concept of a language used to display a page. The default will probably be that variable_get() and friends return values in the page language used. We intend to support asking for values in other languages on demand.
This is precisely what I was concerned about. Although is is certainly common to use variables for storing things like site slogans, site names, etc. Much of the data stored in variables is not textual in nature and is not in fact designed to be alterable, regardless of the language used. What does it mean to translate an array, or an integer for example?
There are countless examples of code that tests for text values, radio buttons, array selections, etc. I would hate to see those chunks of code start misbehaving because the values they were testing against in If's, foreaches, cases, etc. got translated, cause the coder didn't know better and didn't test for a translated case.
I realize that we could put the burden on the developer to make sure to intialize all their data structures appropriately as NON_TRANSLATABLE, but it seems like a lot of overhead when writing code, (Hey all I wanted was a persistant configuration variable). I'm concerned that this will make contrib modules less stable in the end.
Wouldn't it be better to store translatable text strings somewhere else entirely? And then make site slogans, etc store data in these locations instead? Make the data domain allowed to be stored in these cases be textual only (not serialized structured data).
I don't want to clog the airwaves, so I'll humbly step out of this debate if I'm alone in this concern.
Dave, don't be concerned. The point of my suggestion to have a list of variables defined is that we can have a subset of them for translation. Look at the issue about user defined text translation and you will see that we are about to translate only what is there for translation, not all values blindly. http://drupal.org/node/141461 (How the approach in the patch fits with variables is not a far stretch probably, but depends on the outcome of how variable defaults will be defined). Gabor
On Fri, May 04, 2007 at 09:26:44AM -0500, Larry Garfield wrote:
On Friday 04 May 2007, Dries Buytaert wrote:
Anyway, let's step back for a bit. It seems like we're trying to solve two problems in this thread:
1. For convenience, we want a central place to define a variable's default value.
2. Out of necessity, we want a mechanism to let the locale system translate variables that have dynamic content shown on the UIs (i.e. the footer message).
1 and 2 are somewhat related -- if we had a mechanism to introspect what variables are available, we might be able to extract their default value from a central place, and the locale system could figure out what variables need to be translated.
So, each variable should have two properties: (i) a default value and (2) a "dynamic translation"-flag (TRUE if translatable, FALSE if not) and we should be able to poll both. Gabor's original question was: what would be the best implementation?
Because the variables could change at runtime, I think a hook_variables() (or similar) would be the best approach. Core may not use more than just node type, but I know some contribs do, like form IDs.
hook_variables() { $items = array(); $items['mymod_num_things'] = array( '#default_value' => 2, // Same syntax as FAPI uses '#realm' => 'mymod', // optional '#translatable' => FALSE, // probably default false '#cacheable' => TRUE, // default to TRUE if under X chars when serialized '#serialize' => FALSE, // default to true for object/array, else false ); foreach (node_get_types('name') as $type) { $items['mymod_things_for_' . $type] = array( '#default_value' => array(), ); }
return $items; }
I kinda like the idea of using FAPI syntax here, as then system_settings_form() can fill in defaults for us if they're not specified. (I suppose it could anyway, but meh. <g>) I leave that to Gabor to decide.
I'm undecided on whether non-cached big variables should still be considered variables or should have some other namespace/table. I guess that depends on how expensive it would be to make variable_get() handle both cases.
I'd actually declare it as hook_variables($type, $arg2) The module will not always know about the available node types as we can add a bunch after enabling the module. So I'd say that whenever we have a new content type (No idea how to detect that) we call this hook to update our defaults. The problem is that hook_variables() returns the default related to the node types we have as well as the defaults not related to the node types. That's why $type can either by 'node' for node types related defaults, 'comment' for comments related settings, 'term' for term related, 'vocabulary' for vocabulary related and 'default' for anything else ? Or would this be an over kill ? -- GPG-Key: 0xA3FD0DF7 - 9F73 032E EAC9 F7AD 951F 280E CB66 8E29 A3FD 0DF7 Debian User and Developer. Homepage: www.foolab.org
On Fri, 4 May 2007 19:50:34 +0300, Mohammed Sameer <msameer@foolab.org> wrote:
hook_variables() { $items = array(); $items['mymod_num_things'] = array( '#default_value' => 2, // Same syntax as FAPI uses '#realm' => 'mymod', // optional '#translatable' => FALSE, // probably default false '#cacheable' => TRUE, // default to TRUE if under X chars when serialized '#serialize' => FALSE, // default to true for object/array, else false ); foreach (node_get_types('name') as $type) { $items['mymod_things_for_' . $type] = array( '#default_value' => array(), ); }
return $items; }
I kinda like the idea of using FAPI syntax here, as then system_settings_form() can fill in defaults for us if they're not specified. (I suppose it could anyway, but meh. <g>) I leave that to Gabor to decide.
I'm undecided on whether non-cached big variables should still be considered variables or should have some other namespace/table. I guess that depends on how expensive it would be to make variable_get() handle both cases.
I'd actually declare it as hook_variables($type, $arg2)
The module will not always know about the available node types as we can add a bunch after enabling the module. So I'd say that whenever we have a new content type (No idea how to detect that) we call this hook to update our defaults. The problem is that hook_variables() returns the default related to the node types we have as well as the defaults not related to the node types. That's why $type can either by 'node' for node types related defaults, 'comment' for comments related settings, 'term' for term related, 'vocabulary' for vocabulary related and 'default' for anything else ? Or would this be an over kill ?
I think that's over-engineering. While node types are the most common "variable variable", there's lots of others. captcha.module uses form ids. Some modules could even use node ids (although that would probably be rather silly). I don't think locking it down to a specific type of entity would be a good idea, as it would either create a crapload of different entity types or block us from using certain types, or both. --Larry Garfield
Mohammed Sameer wrote:
hook_variables() { $items = array(); $items['mymod_num_things'] = array( '#default_value' => 2, // Same syntax as FAPI uses '#realm' => 'mymod', // optional '#translatable' => FALSE, // probably default false '#cacheable' => TRUE, // default to TRUE if under X chars when serialized '#serialize' => FALSE, // default to true for object/array, else false ); foreach (node_get_types('name') as $type) { $items['mymod_things_for_' . $type] = array( '#default_value' => array(), ); }
return $items; }
I'd actually declare it as hook_variables($type, $arg2)
The module will not always know about the available node types as we can add a bunch after enabling the module. So I'd say that whenever we have a new content type (No idea how to detect that) we call this hook to update our defaults. The problem is that hook_variables() returns the default related to the node types we have as well as the defaults not related to the node types. That's why $type can either by 'node' for node types related defaults, 'comment' for comments related settings, 'term' for term related, 'vocabulary' for vocabulary related and 'default' for anything else ? Or would this be an over kill ?
Erm, this is completely unrealistic. We don't have concepts such as "node related variables", "term related variables" and so on. We have variables, which are any kind of key => value pairs, what is what we know (at least what we need here). Gabor
On Fri, May 04, 2007 at 07:47:00PM +0200, Gabor Hojtsy wrote:
Mohammed Sameer wrote:
hook_variables() { $items = array(); $items['mymod_num_things'] = array( '#default_value' => 2, // Same syntax as FAPI uses '#realm' => 'mymod', // optional '#translatable' => FALSE, // probably default false '#cacheable' => TRUE, // default to TRUE if under X chars when serialized '#serialize' => FALSE, // default to true for object/array, else false ); foreach (node_get_types('name') as $type) { $items['mymod_things_for_' . $type] = array( '#default_value' => array(), ); }
return $items; }
I'd actually declare it as hook_variables($type, $arg2)
The module will not always know about the available node types as we can add a bunch after enabling the module. So I'd say that whenever we have a new content type (No idea how to detect that) we call this hook to update our defaults. The problem is that hook_variables() returns the default related to the node types we have as well as the defaults not related to the node types. That's why $type can either by 'node' for node types related defaults, 'comment' for comments related settings, 'term' for term related, 'vocabulary' for vocabulary related and 'default' for anything else ? Or would this be an over kill ?
Erm, this is completely unrealistic. We don't have concepts such as "node related variables", "term related variables" and so on. We have variables, which are any kind of key => value pairs, what is what we know (at least what we need here).
I thought it's an over kill. We don't have the concepts but they are there. We have some variables depending on the content type. The only problem is that some variables won't have defaults if don't call the hook whenever we add a new content type ? -- GPG-Key: 0xA3FD0DF7 - 9F73 032E EAC9 F7AD 951F 280E CB66 8E29 A3FD 0DF7 Debian User and Developer. Homepage: www.foolab.org
Quoting Larry Garfield <larry@garfieldtech.com>:
On Friday 04 May 2007, Dries Buytaert wrote:
--8<--
2. Out of necessity, we want a mechanism to let the locale system translate variables that have dynamic content shown on the UIs (i.e. the footer message).
Which default may have been changed by the administrator and that changed, stored in the database, value needs to be translated.
1 and 2 are somewhat related -- if we had a mechanism to introspect what variables are available, we might be able to extract their default value from a central place, and the locale system could figure out what variables need to be translated.
So, each variable should have two properties: (i) a default value and (2) a "dynamic translation"-flag (TRUE if translatable, FALSE if not) and we should be able to poll both. Gabor's original question was: what would be the best implementation?
So we have a default locale and a user chosen locale. The chosen locale has an ID value that is incorporated into the variable table. Am I beginning to understand what Gabor is after? Sorry for the noise to solution ratio. And thanks for the explanations Gabor. I don't want to make assumptions, would the administrator need to supply the translations? The default locale ID for the variable (and other) table would be 0 and would be used if the translation for chosen locale ID doesn't exist. Earnie
On Thursday 03 May 2007 18:52, Steven Wittens wrote:
I think Dries was more trying to illustrate a point. It is trivial to change this so it doesn't suffer from this problem:
Sorry for being needlessly pedantic, then. I just didn't want a bug to creep in because someone (a PHP novice, perhaps) grabbed the example code from the list archives for real work. Scott -- ------------------------------------------------------------------------------- Syscrusher (Scott Courtney) Drupal page: http://drupal.org/user/9184 syscrusher at 4th dot com Home page: http://4th.com/
On 5/3/07, Gabor Hojtsy <gabor@hojtsy.hu> wrote:
**We need a central place to define variable defaults**
Why can't that central place be the variable table? We could remove the second argument to variable_get and refactor some code to do the variable_set at install time which eliminates the need for the defaults in the variable_get() calls themselves. add1sun recently contributed a patch to pathauto that did this. The reason it's needed in pathauto was the use of variable_get's that involved variable variable names (e.g. variable_get('pathauto_'. $something .'..., '') which had empty defaults and the code did nothing if the variable came back empty. To make sure pathauto worked without submitting the settings page, it now sets variables in hook_install. Couldn't all modules do that? Then we eliminate the second argument to variable_get, create the centralized place to define variable defaults as you wanted, but need no new hooks and no new module_invoke_all and no new caching of "yet another variable system".
Don't care yet about how this fits into translatable variables, because we can only follow up on that if this gets done. So someone please grab this issue and run with it ;)
The tasks would be fairly simple: 1. Modify variable get to take one argument 2. Find all variable_gets and their corresponding default, move the default to an _install 3. Document it on the update/modules page 4. Rejoice 5. Console killes I'd be happy to run with that if people like the approach. Regards, Greg
Greg Knaddison - GVS wrote:
On 5/3/07, Gabor Hojtsy <gabor@hojtsy.hu> wrote:
**We need a central place to define variable defaults**
Why can't that central place be the variable table?
We could remove the second argument to variable_get and refactor some code to do the variable_set at install time which eliminates the need for the defaults in the variable_get() calls themselves.
add1sun recently contributed a patch to pathauto that did this. The reason it's needed in pathauto was the use of variable_get's that involved variable variable names (e.g. variable_get('pathauto_'. $something .'..., '') which had empty defaults and the code did nothing if the variable came back empty. To make sure pathauto worked without submitting the settings page, it now sets variables in hook_install.
Couldn't all modules do that? Then we eliminate the second argument to variable_get, create the centralized place to define variable defaults as you wanted, but need no new hooks and no new module_invoke_all and no new caching of "yet another variable system".
Don't care yet about how this fits into translatable variables, because we can only follow up on that if this gets done. So someone please grab this issue and run with it ;)
The tasks would be fairly simple:
1. Modify variable get to take one argument 2. Find all variable_gets and their corresponding default, move the default to an _install 3. Document it on the update/modules page 4. Rejoice 5. Console killes
I'd be happy to run with that if people like the approach.
Indeed, makes a lot of sense to me. I'd be glad if you could run with this and submit a patch. But wait for more positive reactions first :) Gabor
On Thu, May 03, 2007 at 05:11:41PM +0200, Gabor Hojtsy wrote:
Greg Knaddison - GVS wrote:
On 5/3/07, Gabor Hojtsy <gabor@hojtsy.hu> wrote:
**We need a central place to define variable defaults**
Why can't that central place be the variable table?
We could remove the second argument to variable_get and refactor some code to do the variable_set at install time which eliminates the need for the defaults in the variable_get() calls themselves.
add1sun recently contributed a patch to pathauto that did this. The reason it's needed in pathauto was the use of variable_get's that involved variable variable names (e.g. variable_get('pathauto_'. $something .'..., '') which had empty defaults and the code did nothing if the variable came back empty. To make sure pathauto worked without submitting the settings page, it now sets variables in hook_install.
Couldn't all modules do that? Then we eliminate the second argument to variable_get, create the centralized place to define variable defaults as you wanted, but need no new hooks and no new module_invoke_all and no new caching of "yet another variable system".
Don't care yet about how this fits into translatable variables, because we can only follow up on that if this gets done. So someone please grab this issue and run with it ;)
The tasks would be fairly simple:
1. Modify variable get to take one argument 2. Find all variable_gets and their corresponding default, move the default to an _install 3. Document it on the update/modules page 4. Rejoice 5. Console killes
I'd be happy to run with that if people like the approach.
Indeed, makes a lot of sense to me. I'd be glad if you could run with this and submit a patch. But wait for more positive reactions first :)
Gabor
If we have it as a hook then we don't really need to clean them up in hook_uninstall() because drupal can invoke the variables defaults hook, get the list of variables and simply delete them from the "registry". We now only have one problem. hook uninstall runs when the module is unloaded but then, we can implement the extra hook in the .install file ? Hope I'm not talking nonsense ;-) -- GPG-Key: 0xA3FD0DF7 - 9F73 032E EAC9 F7AD 951F 280E CB66 8E29 A3FD 0DF7 Debian User and Developer. Homepage: www.foolab.org
On Thu, 3 May 2007 21:02:40 +0300, Mohammed Sameer <msameer@foolab.org> wrote:
The tasks would be fairly simple:
1. Modify variable get to take one argument 2. Find all variable_gets and their corresponding default, move the default to an _install 3. Document it on the update/modules page 4. Rejoice 5. Console killes
I'd be happy to run with that if people like the approach.
Indeed, makes a lot of sense to me. I'd be glad if you could run with this and submit a patch. But wait for more positive reactions first :)
Gabor
If we have it as a hook then we don't really need to clean them up in hook_uninstall() because drupal can invoke the variables defaults hook, get the list of variables and simply delete them from the "registry". We now only have one problem. hook uninstall runs when the module is unloaded but then, we can implement the extra hook in the .install file ?
Hope I'm not talking nonsense ;-)
If implemented that way, I believe that would qualify as one of those "Rare Hooks" I talked about earlier. :-) --Larry Garfield
On 5/3/07, Mohammed Sameer <msameer@foolab.org> wrote:
If we have it as a hook then we don't really need to clean them up in hook_uninstall() because drupal can invoke the variables defaults hook, get the list of variables and simply delete them from the "registry". We now only have one problem. hook uninstall runs when the module is unloaded but then, we can implement the extra hook in the .install file ?
Hope I'm not talking nonsense ;-)
I think it makes at least some sense. My experience from looking at hook_uninstall solutions is that lots of variables get left around. The proposed hook_settings() would create some basic variables. It would probably not know about _all_ of the variables that the module may create. Many variable names are based on variables: e.g. modulename_contenttype_something. My solution for that has been to delete all rows from variable table that start with modulename_ I think if you try to delete specific variables I think you'll end up leaving lots of unused keys around. The realm idea would help a lot with the cleanup, but is not "low hanging fruit" IMO. Greg
Variable relms/namespaces++ New names for variable functions-- Variable registry with defaults in a DB-- Variable defaults in hooks+- (not sure) This is MHO. Robin On 5/3/07, Gabor Hojtsy <gabor@hojtsy.hu> wrote:
Hi,
To be able to translate variables to multiple languages in Drupal nicely, we would need a small conceptual change, which would benefit all of the Drupal developers, so I am posting the call here in hopes we have someone or a small group to pick this task up.
**We need a central place to define variable defaults**
Simple! Now whenever you need a variable, you do
variable_get('my_fine_var', 'my_default_value');
The problem with this is that you need to repeat this multiple times, and of course there is a chance you need to modify it later on, so you need to find all places a variable is used. Not good. Remember the changes from bluemarine to garland, we have been fixing theme_default errors for days, finding out places where the variable was used...
So we need a central place to define variable defaults. For Drupal 6 this is enough now:
hook_settings() { return array( 'my_fine_var' => 'my_default_value', ); }
Have fun with naming it hook_settings(), as far as I see, it is an appropriate name, and not taken at the moment :) So Drupal can do a module_invoke_all() on hook_settings() and collect defaults to all variables from the modules defining them. It gets easier and shorter to use variables:
variable_get('my_fine_var');
We can either cache the default vars as returned by the callback in the variable_get() function, or in the database among other variables, you get the idea. It is important to have all variables defined in hook_settings(), even if the default value is an empty string or array.
Don't care yet about how this fits into translatable variables, because we can only follow up on that if this gets done. So someone please grab this issue and run with it ;)
Gabor
-- Robin Monks @ www.civicspacelabs.org @ www.gmking.org Fax: (419) 791-8076 "Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems." ~IRC
On Thursday 03 May 2007 14:29, Robin Monks wrote:
New names for variable functions--
Here's a suggestion: string variable(string $name, string $namespace='', string $value=NULL) Semantics: variable($name) returns the value of the specified variable in the current module's namespace (or the Drupal system namespace, if you prefer), or returns the default if not defined, or returns NULL if no default variable($name, $namespace) returns the value of the specified variable in the specified namespace, or returns the default if not defined, or returns NULL if no default variable($name, $namespace, $value) stores $value for the specified variable in the specified namespace, if $value is not null. Returns $value as-stored, both to preserve semantic consistency AND to reflect any data changes done by the variable-handling code (for instance, if it did an implied trim()). For now, probably a straight pass-through. And we add one function that typically would be used only by deinstalls: boolean variable_unset(string $name, string $namespace) removes the specified variable from storage permanently, returning TRUE if successful or if the variable already didn't exist, FALSE if there was some kind of database error or if the $namespace didn't exist. In other words, missing $name in valid $namespace is okay, just a no-op, but invalid $namespace is an error condition. The behavior of variable() is consistent with similar functions I've worked with in other environments, such as REXX's GLOBALV function on mainframes. It also would allow a fairly easy wrapper for legacy compatibility. Scott -- ------------------------------------------------------------------------------- Syscrusher (Scott Courtney) Drupal page: http://drupal.org/user/9184 syscrusher at 4th dot com Home page: http://4th.com/
Right, but, now you need to specify yourself as the namespace every time you set a variable. That's adding a lot of bulk. Robin On 5/3/07, Syscrusher <syscrusher@4th.com> wrote:
On Thursday 03 May 2007 14:29, Robin Monks wrote:
New names for variable functions--
Here's a suggestion:
string variable(string $name, string $namespace='', string $value=NULL)
Semantics:
variable($name) returns the value of the specified variable in the current module's namespace (or the Drupal system namespace, if you prefer), or returns the default if not defined, or returns NULL if no default
variable($name, $namespace) returns the value of the specified variable in the specified namespace, or returns the default if not defined, or returns NULL if no default
variable($name, $namespace, $value) stores $value for the specified variable in the specified namespace, if $value is not null. Returns $value as-stored, both to preserve semantic consistency AND to reflect any data changes done by the variable-handling code (for instance, if it did an implied trim()). For now, probably a straight pass-through.
And we add one function that typically would be used only by deinstalls:
boolean variable_unset(string $name, string $namespace) removes the specified variable from storage permanently, returning TRUE if successful or if the variable already didn't exist, FALSE if there was some kind of database error or if the $namespace didn't exist. In other words, missing $name in valid $namespace is okay, just a no-op, but invalid $namespace is an error condition.
The behavior of variable() is consistent with similar functions I've worked with in other environments, such as REXX's GLOBALV function on mainframes. It also would allow a fairly easy wrapper for legacy compatibility.
Scott
--
------------------------------------------------------------------------------- Syscrusher (Scott Courtney) Drupal page: http://drupal.org/user/9184 syscrusher at 4th dot com Home page: http://4th.com/
-- Robin Monks @ www.civicspacelabs.org @ www.gmking.org Fax: (419) 791-8076 "Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems." ~IRC
On Thursday 03 May 2007 14:56, Robin Monks wrote:
Right, but, now you need to specify yourself as the namespace every time you set a variable. That's adding a lot of bulk.
True, but how many variable set calls are there outside of the settings pages? In my modules, variable_get() is all over the place in the code, but variable_set() is common only in the settings. Do yours differ from that mode? Scott -- ------------------------------------------------------------------------------- Syscrusher (Scott Courtney) Drupal page: http://drupal.org/user/9184 syscrusher at 4th dot com Home page: http://4th.com/
A follow up to this is an issue by dww: http://drupal.org/node/145164 Although he reaches the conclusion that we need a hook_variable() from another angle, the goal is the same after all. So people sympathizing / against the effort, please use that issue! Gabor Gabor Hojtsy wrote:
Hi,
To be able to translate variables to multiple languages in Drupal nicely, we would need a small conceptual change, which would benefit all of the Drupal developers, so I am posting the call here in hopes we have someone or a small group to pick this task up.
**We need a central place to define variable defaults**
Simple! Now whenever you need a variable, you do
variable_get('my_fine_var', 'my_default_value');
The problem with this is that you need to repeat this multiple times, and of course there is a chance you need to modify it later on, so you need to find all places a variable is used. Not good. Remember the changes from bluemarine to garland, we have been fixing theme_default errors for days, finding out places where the variable was used...
So we need a central place to define variable defaults. For Drupal 6 this is enough now:
hook_settings() { return array( 'my_fine_var' => 'my_default_value', ); }
Have fun with naming it hook_settings(), as far as I see, it is an appropriate name, and not taken at the moment :) So Drupal can do a module_invoke_all() on hook_settings() and collect defaults to all variables from the modules defining them. It gets easier and shorter to use variables:
variable_get('my_fine_var');
We can either cache the default vars as returned by the callback in the variable_get() function, or in the database among other variables, you get the idea. It is important to have all variables defined in hook_settings(), even if the default value is an empty string or array.
Don't care yet about how this fits into translatable variables, because we can only follow up on that if this gets done. So someone please grab this issue and run with it ;)
Gabor
Gabor Hojtsy wrote:
A follow up to this is an issue by dww: http://drupal.org/node/145164 Although he reaches the conclusion that we need a hook_variable() from another angle, the goal is the same after all. So people sympathizing / against the effort, please use that issue!
And now Jose A. Reyero is at it, and proposing a nice patch, which has good performance and development-friendliness implications, so I would encourage people to go and review the patch: http://drupal.org/node/145164#comment-250759 Performance improvements: - slightly less code to parse on each page view, as variable defaults are centralized - less t() calls to invoke, as not close to every variable_default() call requires calling that, it only needs to be invoked at most one time for a value - less data to load in and unserialize from variable cache, as default values are not saved - less updates / cache refreshed on variables table because if value is unchanged it is not updated in database - less variables stored because uninstalled modules get their variables uninstalled automatically Development convenience improvements: - no need to repeat variable defaults everytime - you get your variables removed on uninstall, no need to write code for that - because you provide a type for your variable in the variables hook, you have your common value types automatically validated (implemented for email addresses and numbers) PLUS this makes it a lot easier to provide user friendly localization or translation features for site settings. Gabor
participants (16)
-
Aaron Winborn -
David Metzler -
Dries Buytaert -
Earnie Boyd -
Gabor Hojtsy -
Gerhard Killesreiter -
Greg Knaddison - GVS -
John Vandervort -
Konstantin Käfer -
Larry Garfield -
Mohammed Sameer -
Moshe Weitzman -
Robin Monks -
Steven Wittens -
Syscrusher -
Wim Leers