[drupal-devel] [bug] Multiple node types are broken
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. chx
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: jakeg Status: patch (code needs review) +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. jakeg Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: jakeg Status: patch (code needs review) Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. jakeg Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. chx Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. chx Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: robertDouglass Status: patch (code needs review) question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? robertDouglass Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: robertDouglass Status: patch (code needs review) the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } robertDouglass Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this?
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: jakeg Status: patch (code needs review) Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. jakeg Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); }
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: jakeg Status: patch (code needs review) hook_base_name should maybe be hook_node_base_name instead. jakeg Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :) chx Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: jakeg Status: patch (code needs review) in theory chx whenever the word 'page' or 'story' is mentioned you could use node_get_module_name instead. and in the seldom places they do deviate other than simply the word page/story, then you can have a switch statement based on node_get_module_name maybe i've missed a whole chunk of discussion re. simple node types etc in the past though jakeg Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead. ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:07:52 +0000 : chx That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :)
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: asimmonds Status: patch (code needs review) In node_list(): else { $types[$module] = array($value, $module); } I think should be ($node_names is a string here): else { $types[$module] = $node_names; } As I'm getting a few errors as a array is being passed to other functions instead of a string. I've now got a working project module against HEAD, with this in project.module and chx's patch function project_node_name() { return array('project_project' => 'project', 'project_issue' => 'issue'); } function project_base_name() { return array('project_project' => 'project_project', 'project_issue' => 'project_issue'); } asimmonds Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead. ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:07:52 +0000 : chx That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :) ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:13:07 +0000 : jakeg in theory chx whenever the word 'page' or 'story' is mentioned you could use node_get_module_name instead. and in the seldom places they do deviate other than simply the word page/story, then you can have a switch statement based on node_get_module_name maybe i've missed a whole chunk of discussion re. simple node types etc in the past though
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Assimonds, your comment is not against the latest version of my patch. Please update. chx Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead. ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:07:52 +0000 : chx That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :) ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:13:07 +0000 : jakeg in theory chx whenever the word 'page' or 'story' is mentioned you could use node_get_module_name instead. and in the seldom places they do deviate other than simply the word page/story, then you can have a switch statement based on node_get_module_name maybe i've missed a whole chunk of discussion re. simple node types etc in the past though ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:33:18 +0000 : asimmonds In node_list(): else { $types[$module] = array($value, $module); } I think should be ($node_names is a string here): else { $types[$module] = $node_names; } As I'm getting a few errors as a array is being passed to other functions instead of a string. I've now got a working project module against HEAD, with this in project.module and chx's patch function project_node_name() { return array('project_project' => 'project', 'project_issue' => 'issue'); } function project_base_name() { return array('project_project' => 'project_project', 'project_issue' => 'project_issue'); }
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: asimmonds Status: patch (code needs review) My comments were against node_basename_1.patch, with that node/add fails to show any content types. asimmonds Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead. ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:07:52 +0000 : chx That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :) ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:13:07 +0000 : jakeg in theory chx whenever the word 'page' or 'story' is mentioned you could use node_get_module_name instead. and in the seldom places they do deviate other than simply the word page/story, then you can have a switch statement based on node_get_module_name maybe i've missed a whole chunk of discussion re. simple node types etc in the past though ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:33:18 +0000 : asimmonds In node_list(): else { $types[$module] = array($value, $module); } I think should be ($node_names is a string here): else { $types[$module] = $node_names; } As I'm getting a few errors as a array is being passed to other functions instead of a string. I've now got a working project module against HEAD, with this in project.module and chx's patch function project_node_name() { return array('project_project' => 'project', 'project_issue' => 'issue'); } function project_base_name() { return array('project_project' => 'project_project', 'project_issue' => 'project_issue'); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:50:03 +0000 : chx Assimonds, your comment is not against the latest version of my patch. Please update.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_basename_2.patch (20.72 KB) assimonds, sorry , you are absolutely right. I corrected the patch. That array is a remnant of a previous version. chx Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead. ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:07:52 +0000 : chx That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :) ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:13:07 +0000 : jakeg in theory chx whenever the word 'page' or 'story' is mentioned you could use node_get_module_name instead. and in the seldom places they do deviate other than simply the word page/story, then you can have a switch statement based on node_get_module_name maybe i've missed a whole chunk of discussion re. simple node types etc in the past though ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:33:18 +0000 : asimmonds In node_list(): else { $types[$module] = array($value, $module); } I think should be ($node_names is a string here): else { $types[$module] = $node_names; } As I'm getting a few errors as a array is being passed to other functions instead of a string. I've now got a working project module against HEAD, with this in project.module and chx's patch function project_node_name() { return array('project_project' => 'project', 'project_issue' => 'issue'); } function project_base_name() { return array('project_project' => 'project_project', 'project_issue' => 'project_issue'); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:50:03 +0000 : chx Assimonds, your comment is not against the latest version of my patch. Please update. ------------------------------------------------------------------------ Sat, 27 Aug 2005 11:06:49 +0000 : asimmonds My comments were against node_basename_1.patch, with that node/add fails to show any content types.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/hook_node.patch (24.25 KB) I talked long with Dries this time. Let me introduce you hook_node! hook_node let's you define one or more node types. you need to return an array($type => array('name' => $name, 'base' => $base), $type1 => array('name' => $name1, 'base' => $base1), ) where $type is the node type, $name is the human readable name of the type and $base is used instead of 'hook' for hook_load, hook_view etc. If you it's a bit too much writing all the time: [19:06] but... do you want to always write return array('name' => t('something'), 'type' => 'example_module', 'base' => 'example_module'); instead of return t('something') ??? [19:06] chx: yes, i always want to write that So there. chx Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead. ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:07:52 +0000 : chx That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :) ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:13:07 +0000 : jakeg in theory chx whenever the word 'page' or 'story' is mentioned you could use node_get_module_name instead. and in the seldom places they do deviate other than simply the word page/story, then you can have a switch statement based on node_get_module_name maybe i've missed a whole chunk of discussion re. simple node types etc in the past though ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:33:18 +0000 : asimmonds In node_list(): else { $types[$module] = array($value, $module); } I think should be ($node_names is a string here): else { $types[$module] = $node_names; } As I'm getting a few errors as a array is being passed to other functions instead of a string. I've now got a working project module against HEAD, with this in project.module and chx's patch function project_node_name() { return array('project_project' => 'project', 'project_issue' => 'issue'); } function project_base_name() { return array('project_project' => 'project_project', 'project_issue' => 'project_issue'); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:50:03 +0000 : chx Assimonds, your comment is not against the latest version of my patch. Please update. ------------------------------------------------------------------------ Sat, 27 Aug 2005 11:06:49 +0000 : asimmonds My comments were against node_basename_1.patch, with that node/add fails to show any content types. ------------------------------------------------------------------------ Sat, 27 Aug 2005 11:12:36 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_2.patch (20.72 KB) assimonds, sorry , you are absolutely right. I corrected the patch. That array is a remnant of a previous version.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: asimmonds Status: patch (code needs review) A couple of bugs: In theme.inc there is a node_list() @@ -232,7 +232,7 @@ ); if (module_exist('node')) { - foreach (node_list() as $type) { + foreach (node_get_types() as $type) { $defaults['toggle_node_info_' . $type] = 1; } } In node.module, a node_name hook is still called @@ -1403,15 +1409,15 @@ } } $output = node_form($node); - drupal_set_title(t('Submit %name', array('%name' => node_invoke($node, 'node_name')))); + drupal_set_title(t('Submit %name', array('%name' => node_get_name($node)))); } else { If _node_names() is called with a non-existing node-type, it returns the last node-type in the list because the "foreach ($node_names as $type => $value)" is overwriting the $type derived from $node a couple of lines above. asimmonds Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead. ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:07:52 +0000 : chx That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :) ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:13:07 +0000 : jakeg in theory chx whenever the word 'page' or 'story' is mentioned you could use node_get_module_name instead. and in the seldom places they do deviate other than simply the word page/story, then you can have a switch statement based on node_get_module_name maybe i've missed a whole chunk of discussion re. simple node types etc in the past though ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:33:18 +0000 : asimmonds In node_list(): else { $types[$module] = array($value, $module); } I think should be ($node_names is a string here): else { $types[$module] = $node_names; } As I'm getting a few errors as a array is being passed to other functions instead of a string. I've now got a working project module against HEAD, with this in project.module and chx's patch function project_node_name() { return array('project_project' => 'project', 'project_issue' => 'issue'); } function project_base_name() { return array('project_project' => 'project_project', 'project_issue' => 'project_issue'); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:50:03 +0000 : chx Assimonds, your comment is not against the latest version of my patch. Please update. ------------------------------------------------------------------------ Sat, 27 Aug 2005 11:06:49 +0000 : asimmonds My comments were against node_basename_1.patch, with that node/add fails to show any content types. ------------------------------------------------------------------------ Sat, 27 Aug 2005 11:12:36 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_2.patch (20.72 KB) assimonds, sorry , you are absolutely right. I corrected the patch. That array is a remnant of a previous version. ------------------------------------------------------------------------ Sat, 27 Aug 2005 18:01:11 +0000 : chx Attachment: http://drupal.org/files/issues/hook_node.patch (24.25 KB) I talked long with Dries this time. Let me introduce you hook_node! hook_node let's you define one or more node types. you need to return an array($type => array('name' => $name, 'base' => $base), $type1 => array('name' => $name1, 'base' => $base1), ) where $type is the node type, $name is the human readable name of the type and $base is used instead of 'hook' for hook_load, hook_view etc. If you it's a bit too much writing all the time: [19:06] but... do you want to always write return array('name' => t('something'), 'type' => 'example_module', 'base' => 'example_module'); instead of return t('something') ??? [19:06] chx: yes, i always want to write that So there.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch (code needs review) Attachment: http://drupal.org/files/issues/hook_node_0.patch (24.86 KB) chx Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead. ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:07:52 +0000 : chx That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :) ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:13:07 +0000 : jakeg in theory chx whenever the word 'page' or 'story' is mentioned you could use node_get_module_name instead. and in the seldom places they do deviate other than simply the word page/story, then you can have a switch statement based on node_get_module_name maybe i've missed a whole chunk of discussion re. simple node types etc in the past though ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:33:18 +0000 : asimmonds In node_list(): else { $types[$module] = array($value, $module); } I think should be ($node_names is a string here): else { $types[$module] = $node_names; } As I'm getting a few errors as a array is being passed to other functions instead of a string. I've now got a working project module against HEAD, with this in project.module and chx's patch function project_node_name() { return array('project_project' => 'project', 'project_issue' => 'issue'); } function project_base_name() { return array('project_project' => 'project_project', 'project_issue' => 'project_issue'); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:50:03 +0000 : chx Assimonds, your comment is not against the latest version of my patch. Please update. ------------------------------------------------------------------------ Sat, 27 Aug 2005 11:06:49 +0000 : asimmonds My comments were against node_basename_1.patch, with that node/add fails to show any content types. ------------------------------------------------------------------------ Sat, 27 Aug 2005 11:12:36 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_2.patch (20.72 KB) assimonds, sorry , you are absolutely right. I corrected the patch. That array is a remnant of a previous version. ------------------------------------------------------------------------ Sat, 27 Aug 2005 18:01:11 +0000 : chx Attachment: http://drupal.org/files/issues/hook_node.patch (24.25 KB) I talked long with Dries this time. Let me introduce you hook_node! hook_node let's you define one or more node types. you need to return an array($type => array('name' => $name, 'base' => $base), $type1 => array('name' => $name1, 'base' => $base1), ) where $type is the node type, $name is the human readable name of the type and $base is used instead of 'hook' for hook_load, hook_view etc. If you it's a bit too much writing all the time: [19:06] but... do you want to always write return array('name' => t('something'), 'type' => 'example_module', 'base' => 'example_module'); instead of return t('something') ??? [19:06] chx: yes, i always want to write that So there. ------------------------------------------------------------------------ Sat, 27 Aug 2005 21:53:54 +0000 : asimmonds A couple of bugs: In theme.inc there is a node_list() @@ -232,7 +232,7 @@ ); if (module_exist('node')) { - foreach (node_list() as $type) { + foreach (node_get_types() as $type) { $defaults['toggle_node_info_' . $type] = 1; } } In node.module, a node_name hook is still called @@ -1403,15 +1409,15 @@ } } $output = node_form($node); - drupal_set_title(t('Submit %name', array('%name' => node_invoke($node, 'node_name')))); + drupal_set_title(t('Submit %name', array('%name' => node_get_name($node)))); } else { If _node_names() is called with a non-existing node-type, it returns the last node-type in the list because the "foreach ($node_names as $type => $value)" is overwriting the $type derived from $node a couple of lines above.
Issue status update for http://drupal.org/node/29785 Post a follow up: http://drupal.org/project/comments/add/29785 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: Dries -Status: patch (code needs review) +Status: patch (code needs work) This patch no longer applies. Is it still needed? I believe this has been taken care of. Dries Previous comments: ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:31:11 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename.patch (3.92 KB) People come and tell me that I have broken node_get_module_name. I tell them I fixed it. This leads nowhere. Yesterday Kjartan, jakeg and me had finally a conversation during which we have arrived to the conclusion that there is no need for two hooks and hook_node_name could return either a string or an array in the form of: <?php array('type-a' => array('node type a', 'base')), 'type-b' => 'node type b', 'node type c'); ?> Which would define a 'type-a' , with a human readable name 'node type a' and base_load would be called for hook_load and so forth. for 'type-b' , the difference is that modulename_load would be called. For 'node type c', the node type would be modulename. ------------------------------------------------------------------------ Sat, 27 Aug 2005 08:53:15 +0000 : jakeg +1 Except on third example surely their shouldn't be spaces in 'node type c' as surely that won't work? Just to make it clear to people: this patch enables a module, such as project.module, to define more than one node type, using different hooks for each of those node types within the same module. Otherwise, functions would have to be shared between the node types defined by the function, or a different module would be needed for each node type. Neither of which are desirable. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:20:08 +0000 : jakeg Currently gives warnings such as: Warning: Missing argument 1 for page_node_name() in ...something.module on line x Perhaps add argument $null = '' or something similar for backwards compatability as e.g. page.module calls as thus: function page_node_name($node) { return t('page'); } ... so we need the $node argument to be allowed, even if its totally pointless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:26:07 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_0.patch (16.81 KB) I thought a bit more on the issue and created something which I think is a very clean solution. What do we need? We need a capability to get human readable names for node names. Also, we need occasionly a capatibility to override 'module' as the base for 'hook_foo'. Well, then let's define hook_node_name and hook_base_name for these purposes. hook_node_name returns either a string or an array eg. 'somemodule-1' => t('this is a content type');. If you want to use different base name, define them in hook_base_name eg. 'somemodule-1' => 'somemodule_text' and then 'somemodule_text_load' will be called. If you have only one node type per module ie. you returned a string in hook_node_name then hook_base_name is not even called, there is no reason. Now node_list('somemodule-1') would return t('this is a content type');. Also, node_list() as it stands is pretty useable in itself as you'll see in the code, it elinimates a few lines of code here and there. The change of node_get_module_name to node_get_base_name affects only group and og module in contrib and as for current node type modules the output is the same (because noone has hook_base_name atm) a simple replace will do as upgrade. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:29:43 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_1.patch (20.71 KB) Oh, jakeg is right, I changed hook_node_name to be parameterless. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:34:04 +0000 : robertDouglass question: would this functionality allow us to get rid of page and story node types in favor of a similar module that can be called whatever you want to call it? Instead of function hook_node_types() { return array('page'); } we could have function hook_node_types() { return array(variable_get('user_defined_nodes', array('page', 'story')); } This would let us get rid of these two identical (yes, do a diff on them, they're identical now) modules and let people start defining all the module names that should be attributed to the title+body paradigm. chx - get what I'm saying? How would your patch support this? ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:35:13 +0000 : robertDouglass the second example should be this: function hook_node_types() { return variable_get('user_defined_nodes', array('page', 'story')); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:54:05 +0000 : jakeg Robert: I think that would be possible with chx's changes and code: function simplenode_node_name() { return array('page' => t('page'),'story' => t('story')); } function simplenode_base_name() { return array('page' => 'simplenode','story' => 'simplenode'); } ... renaming page.module or story.module simplenode.module instead. Then the one module can deal with two identical nodes but giving them different names. ------------------------------------------------------------------------ Sat, 27 Aug 2005 09:55:50 +0000 : jakeg hook_base_name should maybe be hook_node_base_name instead. ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:07:52 +0000 : chx That's not enough. You need short help, long help, an UI to list defined types and to edit them. I have such a module -- I ripped fields stuff flexinode. It's possible I'll redo it (not big a job) by ripping fields from content.module instead :) ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:13:07 +0000 : jakeg in theory chx whenever the word 'page' or 'story' is mentioned you could use node_get_module_name instead. and in the seldom places they do deviate other than simply the word page/story, then you can have a switch statement based on node_get_module_name maybe i've missed a whole chunk of discussion re. simple node types etc in the past though ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:33:18 +0000 : asimmonds In node_list(): else { $types[$module] = array($value, $module); } I think should be ($node_names is a string here): else { $types[$module] = $node_names; } As I'm getting a few errors as a array is being passed to other functions instead of a string. I've now got a working project module against HEAD, with this in project.module and chx's patch function project_node_name() { return array('project_project' => 'project', 'project_issue' => 'issue'); } function project_base_name() { return array('project_project' => 'project_project', 'project_issue' => 'project_issue'); } ------------------------------------------------------------------------ Sat, 27 Aug 2005 10:50:03 +0000 : chx Assimonds, your comment is not against the latest version of my patch. Please update. ------------------------------------------------------------------------ Sat, 27 Aug 2005 11:06:49 +0000 : asimmonds My comments were against node_basename_1.patch, with that node/add fails to show any content types. ------------------------------------------------------------------------ Sat, 27 Aug 2005 11:12:36 +0000 : chx Attachment: http://drupal.org/files/issues/node_basename_2.patch (20.72 KB) assimonds, sorry , you are absolutely right. I corrected the patch. That array is a remnant of a previous version. ------------------------------------------------------------------------ Sat, 27 Aug 2005 18:01:11 +0000 : chx Attachment: http://drupal.org/files/issues/hook_node.patch (24.25 KB) I talked long with Dries this time. Let me introduce you hook_node! hook_node let's you define one or more node types. you need to return an array($type => array('name' => $name, 'base' => $base), $type1 => array('name' => $name1, 'base' => $base1), ) where $type is the node type, $name is the human readable name of the type and $base is used instead of 'hook' for hook_load, hook_view etc. If you it's a bit too much writing all the time: [19:06] but... do you want to always write return array('name' => t('something'), 'type' => 'example_module', 'base' => 'example_module'); instead of return t('something') ??? [19:06] chx: yes, i always want to write that So there. ------------------------------------------------------------------------ Sat, 27 Aug 2005 21:53:54 +0000 : asimmonds A couple of bugs: In theme.inc there is a node_list() @@ -232,7 +232,7 @@ ); if (module_exist('node')) { - foreach (node_list() as $type) { + foreach (node_get_types() as $type) { $defaults['toggle_node_info_' . $type] = 1; } } In node.module, a node_name hook is still called @@ -1403,15 +1409,15 @@ } } $output = node_form($node); - drupal_set_title(t('Submit %name', array('%name' => node_invoke($node, 'node_name')))); + drupal_set_title(t('Submit %name', array('%name' => node_get_name($node)))); } else { If _node_names() is called with a non-existing node-type, it returns the last node-type in the list because the "foreach ($node_names as $type => $value)" is overwriting the $type derived from $node a couple of lines above. ------------------------------------------------------------------------ Sun, 28 Aug 2005 08:19:10 +0000 : chx Attachment: http://drupal.org/files/issues/hook_node_0.patch (24.86 KB)
participants (5)
-
asimmonds -
chx -
Dries -
jakeg -
robertDouglass