[drupal-devel] Why is there no designation between hooks and private functions in a module?
Maybe gotten spoiled from working with Macromedia products where everything in the core API is set with MM, but why is there no designation between drupal hook functions and private functions? Checking the docs is not useful especially given that the api changes so often. Frequent changes are also the reason that memorization does not work. Marking core hooks and other would not cost anything and would help in the creation and maintenance of contributed modules. Changing: function mymodule_validate(&$node) { To: function mymodule_dh_validate(&$node) { Or: function mymodule_validate_dh(&$node) { _dh being the core designation would make th process of updating a module easier also. This might be a solution to a naming problem also in that all core code must start with the designation. So anything after that can be used including the underscore character. Changing the hook invoke code has to be done but from the looks of it this is not a big change. Carl McDade
I am not sure if its just me, but I unbderstand nothing of your proposal. Could you rephrase this in a differnet, easier way? Remember that probably non of us use macromedea developement IDEs. Also remember that the APIs, Theme and hooks are not maintained as backwards compatible, when we can achieve improvments by not providing that backwards compatibility. This is one of the main "features" of Drupal, something which has brought us wherre we are now. So chances we will become "fully backwards compatible", code-wise are nearly non-existant. Bèr Op zaterdag 12 maart 2005 16:53, schreef Carl McDade:
Maybe gotten spoiled from working with Macromedia products where everything in the core API is set with MM, but why is there no designation between drupal hook functions and private functions? Checking the docs is not useful especially given that the api changes so often. Frequent changes are also the reason that memorization does not work. Marking core hooks and other would not cost anything and would help in the creation and maintenance of contributed modules.
Changing: function mymodule_validate(&$node) {
To: function mymodule_dh_validate(&$node) {
Or: function mymodule_validate_dh(&$node) {
_dh being the core designation would make th process of updating a module easier also.
This might be a solution to a naming problem also in that all core code must start with the designation. So anything after that can be used including the underscore character.
Changing the hook invoke code has to be done but from the looks of it this is not a big change.
Carl McDade Regards, Bèr -- [ Bèr Kessels | Drupal services www.webschuur.com ]
When you look at most modules there is or should be a comment that a function is a hook. This is not really good enough unless you go through the entire core and memorize all the API functions in the core and perhaps the use of those functions in core/contrib modules. fscache_save is a hook but you don't know this just by looking at it and it may be called in a place in the code where there is no comment. If not familiar with it then you have no idea if is part of the core or not. Potentially one could conflict the code by creating a function of the same name. fscache_dh_save it is now apparent that the function used is a hook and that it is part of a core module. Since non-core functions are not allowed to use the _dh, there would not be a chance of conflict when writing a module. I also know it is available for re-use. I don't think this has to be backwards compatible ,just part of the coding conventions as a rule. core functions : must carry _dh (or whatever)in their name non-core functions: cannot have _dh in their name I am partial to the way Macromedia does this because I don't have to know 1500 API functions. I just have to know that anything starting with mm_ is core. mm_function_save means that is a core function. Carl McDade Bèr Kessels wrote:
I am not sure if its just me, but I unbderstand nothing of your proposal. Could you rephrase this in a differnet, easier way? Remember that probably non of us use macromedea developement IDEs.
Also remember that the APIs, Theme and hooks are not maintained as backwards compatible, when we can achieve improvments by not providing that backwards compatibility. This is one of the main "features" of Drupal, something which has brought us wherre we are now. So chances we will become "fully backwards compatible", code-wise are nearly non-existant.
Bèr
Op zaterdag 12 maart 2005 16:53, schreef Carl McDade:
Maybe gotten spoiled from working with Macromedia products where everything in the core API is set with MM, but why is there no designation between drupal hook functions and private functions? Checking the docs is not useful especially given that the api changes so often. Frequent changes are also the reason that memorization does not work. Marking core hooks and other would not cost anything and would help in the creation and maintenance of contributed modules.
Changing: function mymodule_validate(&$node) {
To: function mymodule_dh_validate(&$node) {
Or: function mymodule_validate_dh(&$node) {
_dh being the core designation would make th process of updating a module easier also.
This might be a solution to a naming problem also in that all core code must start with the designation. So anything after that can be used including the underscore character.
Changing the hook invoke code has to be done but from the looks of it this is not a big change.
Carl McDade Regards, Bèr
Hmm. Makes sense to me. But unless you make a patch for this, and you pimp that patch, this is off course not going to happen. Talk is silver , code is gold. Bèr Op maandag 14 maart 2005 14:32, schreef Carl McDade:
When you look at most modules there is or should be a comment that a function is a hook. This is not really good enough unless you go through the entire core and memorize all the API functions in the core and perhaps the use of those functions in core/contrib modules.
fscache_save
is a hook but you don't know this just by looking at it and it may be called in a place in the code where there is no comment. If not familiar with it then you have no idea if is part of the core or not. Potentially one could conflict the code by creating a function of the same name.
fscache_dh_save
it is now apparent that the function used is a hook and that it is part of a core module. Since non-core functions are not allowed to use the _dh, there would not be a chance of conflict when writing a module. I also know it is available for re-use.
I don't think this has to be backwards compatible ,just part of the coding conventions as a rule.
core functions : must carry _dh (or whatever)in their name
non-core functions: cannot have _dh in their name
I am partial to the way Macromedia does this because I don't have to know 1500 API functions. I just have to know that anything starting with mm_ is core. mm_function_save means that is a core function.
Carl McDade
Bèr Kessels wrote:
I am not sure if its just me, but I unbderstand nothing of your proposal. Could you rephrase this in a differnet, easier way? Remember that probably non of us use macromedea developement IDEs.
Also remember that the APIs, Theme and hooks are not maintained as backwards compatible, when we can achieve improvments by not providing that backwards compatibility. This is one of the main "features" of Drupal, something which has brought us wherre we are now. So chances we will become "fully backwards compatible", code-wise are nearly non-existant.
Bèr
Op zaterdag 12 maart 2005 16:53, schreef Carl McDade:
Maybe gotten spoiled from working with Macromedia products where everything in the core API is set with MM, but why is there no designation between drupal hook functions and private functions? Checking the docs is not useful especially given that the api changes
so
often. Frequent changes are also the reason that memorization does
not
work. Marking core hooks and other would not cost anything and would help in the creation and maintenance of contributed modules.
Changing: function mymodule_validate(&$node) {
To: function mymodule_dh_validate(&$node) {
Or: function mymodule_validate_dh(&$node) {
_dh being the core designation would make th process of updating a module easier also.
This might be a solution to a naming problem also in that all core
code
must start with the designation. So anything after that can be used including the underscore character.
Changing the hook invoke code has to be done but from the looks of it this is not a big change.
Carl McDade
Regards, Bèr Regards, Bèr -- [ Bèr Kessels | Drupal services www.webschuur.com ]
On Monday 14 March 2005 14.52, Bèr Kessels wrote:
Hmm. Makes sense to me. But unless you make a patch for this, and you pimp that patch, this is off course not going to happen. Talk is silver , code is gold. Bèr
Bèr is totally right and not forget that we are talking of changing EVERY function in Drupal. Some of them are called like 'callback' => 'node_page'. So it's gonna be a funny thing to write, and even funnier to test. And do not forget that you are aiming at a moving target, just look at Killes' revision patch. It would be a great thing to reroll your patch every few days just to keep it in sync with HEAD. Maybe writing a replacing script would be better? I do not know and have no wish to pursue this. Regards Karoly Negyesi
Negyesi Karoly wrote:
On Monday 14 March 2005 14.52, Bèr Kessels wrote:
Makes sense to me. But unless you make a patch for this, and you pimp that patch, this is off course not going to happen. Talk is silver , code is gold. Bèr Bèr is totally right and not forget that we are talking of changing EVERY function in Drupal. Some of them are called like 'callback' => 'node_page'. So it's gonna be a funny thing to write, and even funnier to test.
Is it 'let's all pick on Carl day'? A recurring theme perhaps? However, I must say I was pleasantly surprised when mr. Karoly a few hours later posted this to the list:
I know that code is golden, but first, a bit of talk is necessary, especially for the first proposal...
Makes sense to me. From the list-guidelines[1]: "Messages should only be replied to when there is important information to add. Only quote the key points when replying to the list, everyone else will have the previous message and can look it up for more information." I also recommend smiling. regards, Torgeir [1] http://lists.drupal.org/listinfo/drupal-devel
Bèr is totally right and not forget that we are talking of changing EVERY function in Drupal. Some of them are called like 'callback' => 'node_page'. So it's gonna be a funny thing to write, and even funnier to test. Is it 'let's all pick on Carl day'? A recurring theme perhaps?
No. I take everyone seriously, I do not pick on anyone (or at least I try to). I was just warning him or whoever takes this idea, that this an enormous undertaking. Understand me: the idea is _good_ but I do not know whether anyone would do it. Regarding 'fun', let me quote Steven: "hacking core is fun". Another example. pager_query needs its argument in the order they have, but db_query_range has another order, which is very confusing. db_query_range should be changed, and every query that uses it. It's only 21, but I do not feel an urge to go and correct them. Eventually, I will do it. But what will happen with the function calls which are in the thousands?
However, I must say I was pleasantly surprised when mr. Karoly a few
Károly is my given name. Regards Károly Négyesi better known as: chx
However, I must say I was pleasantly surprised when mr. Karoly a few
Károly is my given name.
Then you rather change the order of your names in your mail program, so you will not get caught in such misunderstandings later on. Goba
On Tue, 15 Mar 2005 11:13:20 +0100, Gabor Hojtsy <gabor@hojtsy.hu> wrote:
However, I must say I was pleasantly surprised when mr. Karoly a few Károly is my given name.
Then you rather change the order of your names in your mail program, so you will not get caught in such misunderstandings later on.
In some cultures (Japanese, for instance), your family name goes before your given name. -- Tim Altman
However, I must say I was pleasantly surprised when mr. Karoly a few
Károly is my given name.
Then you rather change the order of your names in your mail program, so you will not get caught in such misunderstandings later on.
In some cultures (Japanese, for instance), your family name goes before your given name.
I know :) We live in the same country with Károly, and I also changed my name order in my mail settings a few years ago to be more internationally recognizable for the very reason that I was called Mr. Gábor. Goba
I know :) We live in the same country with Károly, and I also changed my name order in my mail settings a few years ago to be more internationally recognizable for the very reason that I was called Mr. Gábor.
While I am not reluctant to even change my name temporarily so it is easier for the international community (ie. if someone calls me Charlie, that's OK) I have hesitated to change to change the order in the mailer 'cos this account is also for Hungarian mails. Nevermind. Regards ChX
Op maandag 14 maart 2005 23:23, schreef Torgeir Berg:
Is it 'let's all pick on Carl day'? A recurring theme perhaps?
Iam not sure if you were referring to me here too. In that case the answer is no. I simply ignore: Annoying mails Ignorant mails. And in this case I think this souded like something good, but i simply could get the details from the way Carl wrote his first messge, so I asked him for more details. Then next I warned him, that eventhough its a very good idea, its a huge, undertaking. One that will simply not happen if no-one just does it. On a similar note: comments as nodes, or even users as nodes have been discussed over-and-over-an-over-an-over, and probably even more often. But no-one ever did it. So they are not there. Hell, if all thge hours that went into these discussions (yes I am guilty of quite some comments in these threads too) were stuck into coding, we would have the whole world as nodes by now. This, however does not (never!!) mean that you should Just Start Coding. Thats the opposite and prolly even worse. but OSS project suffer from debates and politics too often, so indeed: Just Code is copper, Talk is silver and Good Code In A Patch Or Module is gold. Is that a good alternative? Regards, Bèr -- [ Bèr Kessels | Drupal services www.webschuur.com ]
I don't think this can be done as a patch it would have to be a system wide and complete change in the hardcoded API. It would also require catching the contribs before they are branched. Just too many are effected by this. Doing in partially through point point versions might work but the best case is a full point version change. Something for 4.7 maybe or if things get slow on bugs a 4.6.5 change over. Carl McDade Bèr Kessels wrote:
Hmm. Makes sense to me. But unless you make a patch for this, and you pimp that patch, this is off course not going to happen. Talk is silver , code is gold.
Bèr
Op maandag 14 maart 2005 14:32, schreef Carl McDade:
When you look at most modules there is or should be a comment that a function is a hook. This is not really good enough unless you go through the entire core and memorize all the API functions in the core and perhaps the use of those functions in core/contrib modules.
fscache_save
is a hook but you don't know this just by looking at it and it may be called in a place in the code where there is no comment. If not familiar with it then you have no idea if is part of the core or not. Potentially one could conflict the code by creating a function of the same name.
fscache_dh_save
it is now apparent that the function used is a hook and that it is part of a core module. Since non-core functions are not allowed to use the _dh, there would not be a chance of conflict when writing a module. I also know it is available for re-use.
I don't think this has to be backwards compatible ,just part of the coding conventions as a rule.
core functions : must carry _dh (or whatever)in their name
non-core functions: cannot have _dh in their name
I am partial to the way Macromedia does this because I don't have to know 1500 API functions. I just have to know that anything starting with mm_ is core. mm_function_save means that is a core function.
Carl McDade
Bèr Kessels wrote:
I am not sure if its just me, but I unbderstand nothing of your proposal. Could you rephrase this in a differnet, easier way? Remember that probably non of us use macromedea developement IDEs.
Also remember that the APIs, Theme and hooks are not maintained as backwards compatible, when we can achieve improvments by not providing that backwards compatibility. This is one of the main "features" of Drupal, something which has brought us wherre we are now. So chances we will become "fully backwards compatible", code-wise are nearly non-existant.
Bèr
Op zaterdag 12 maart 2005 16:53, schreef Carl McDade:
Maybe gotten spoiled from working with Macromedia products where everything in the core API is set with MM, but why is there no designation between drupal hook functions and private functions? Checking the docs is not useful especially given that the api changes
so
often. Frequent changes are also the reason that memorization does
not
work. Marking core hooks and other would not cost anything and would help in the creation and maintenance of contributed modules.
Changing: function mymodule_validate(&$node) {
To: function mymodule_dh_validate(&$node) {
Or: function mymodule_validate_dh(&$node) {
_dh being the core designation would make th process of updating a module easier also.
This might be a solution to a naming problem also in that all core
code
must start with the designation. So anything after that can be used including the underscore character.
Changing the hook invoke code has to be done but from the looks of it this is not a big change.
Carl McDade
Regards, Bèr Regards, Bèr
On Monday 14 March 2005 15.34, Carl McDade wrote:
I don't think this can be done as a patch it would have to be a system wide and complete change in the hardcoded API.
A patch is a file which upgrades one or more files to a newer version. This would be a HUGE patch 'cos it would upgrade all modules and include files. Current policy is that everything is done via patches. This does not mean that you can not implement it by the ways of writing a tricky script, but the way to hand it to the core maintainers is a unified patch. Regards Karoly Negyesi
How would you handle the database calls? They would have to be changed also wouldn't they? Carl McDade Negyesi Karoly wrote:
On Monday 14 March 2005 15.34, Carl McDade wrote:
I don't think this can be done as a patch it would have to be a system wide and complete change in the hardcoded API.
A patch is a file which upgrades one or more files to a newer version. This would be a HUGE patch 'cos it would upgrade all modules and include files.
Current policy is that everything is done via patches. This does not mean that you can not implement it by the ways of writing a tricky script, but the way to hand it to the core maintainers is a unified patch.
Regards
Karoly Negyesi
On Monday 14 March 2005 15.52, Carl McDade wrote:
How would you handle the database calls? They would have to be changed also wouldn't they?
Carl McDade
How could I know what would be changed when this is YOUR idea? You should know what you want to change.
many modules in Drupal tend to name their private functions (eg non-hooks) as function _mymodule_functionname() Maybe enforce such a convention? This does not need that huge patches inside core (but there are still many direct non-hook function calls from module to module) kristjan
Yes, I noticed this and at first thought it was a rule. It would be nice if it was an enforced convention. Even better if it was an advertised (handbook page) coding convention. This would be a good start. I have tried doing a search and replace on just the 4.6 core. Then I ran 20 patches. Diff/Patch is not perfect, out of the 20 patches I got only 11 working without error :( I have no idea why this is, I am using WindowsXP and cygwin to do the work. Some of the code just refuses to merge correctly or the patch is not created correctly. It problably takes some fuzzy controls that I am not familiar with. I first noticed something like this happening while working on the firefox theme. This is why I am sceptical about using diff patch to do something major like this. Carl McDade Kristjan Jansen wrote:
many modules in Drupal tend to name their private functions (eg non-hooks) as
function _mymodule_functionname()
Maybe enforce such a convention?
This does not need that huge patches inside core (but there are still many direct non-hook function calls from module to module)
kristjan
I first noticed something like this happening while working on the firefox theme. This is why I am sceptical about using diff patch to do something major like this.
If you want to do anything of this sort, you really need to get your diff and patch sorted out. They work fine when used correctly. I submitted a huge patch a couple days ago which affects the way plain-text is checked in Drupal. I keep it in a CVS tree so I can do a cvs checkout from Drupal and keep the patch up to date with the latest code. Then I hit "diff" in WinCVS on the entire tree to create a fresh patch. Steven Wittens
many modules in Drupal tend to name their private functions (eg non-hooks) as
function _mymodule_functionname()
Maybe enforce such a convention?
This does not need that huge patches inside core (but there are still many direct non-hook function calls from module to module)
Problem is that modules do have public API functions which are not hooks. Think about taxonomy module for example... So it is not "either hook or not public"... Goba
participants (9)
-
Bèr Kessels -
Carl McDade -
Gabor Hojtsy -
Karoly Negyesi -
Kristjan Jansen -
Negyesi Karoly
-
Steven Wittens -
Tim Altman -
Torgeir Berg