+1 Version Control API, but perhaps the module should be vc_api.module for clarity, and to be more specific? -Mike On Jun 8, 2007, at 5:35 PM, Jakob Petsovits wrote:

On Saturday, 9. June 2007, John Wilkins wrote:

...

How about Revision Control API? rc.module

Would be thinkable, but personally I feel "rc" even less related to version control than "vc" is.

...

Hopefully no one will confuse rc with the cola. or Radio Control. ;-) Although, a Radio-Controlled Drupal sounds like fun.

Hm... release candidate?

__________________ Michael Prasuhn mike@mikeyp.net http://mikeyp.net 714.356.0168

On Saturday, 9. June 2007, Rob Barreca wrote:

I like Version Control API, and think a module named versioncontrol.module is fine. Sure it's a bit more code, but IMO it's better to err on the side of being too explicit, than having people scratch their heads seeing vc.module.

Yes, I think that one is sensible. I'm currently drawn between versioncontrol.module (longer code, but better name) and vcs.module (short code, not so perfect name). The one issue I'm still having with versioncontrol.module is its extensibility for still longer names: there may be a vcs_project.module for the integration with project nodes, a vcslog.module for the commit logs, and a vcsview.module for repository browsing. (Other options left open.) Having the longer name for the original module would probably make those extension modules' names a bit unwieldy, and especially blow up the corresponding menu paths. I'm still undecided. Maybe it's still better to have "Version Control System API" and get all the subsequent advantages in return for the crappy base name. Derek?

On 9-Jun-07, at 1:36 PM, Doug Green wrote:

Angie,

Why is version_control preferred over versioncontrol?

For legibility. It's consistent with our variable naming convention, and separating the individual words help non-English speakers who can babelfish individual words to figure out what a particular thing is doing.

I thought we only used the underscores when the module was extending another module. And I thought we had problems with hook functions related to underscores, or is the problem only related to hook's with underscores?

I know of no such problems. The only thing I can think that would cause a problem is if you named a function something like my_module_insert and Drupal mistook it for a node insert hook when it was just a helper function or something. But mymodule_insert would have exactly the same problem.

-----Original Message----- From: development-bounces@drupal.org [mailto:development- bounces@drupal.org] On Behalf Of Angela Byron Sent: Saturday, June 09, 2007 12:37 PM To: development@drupal.org Subject: Re: [development] Naming the CVS abstraction module

...

What do you think would be the best name for this module?

Just want to chime in as the Coding Standards Queen, that a name such as version_control.module would be preferred over versioncontrol.module. :)

Jakob, once you get the list of possible names down to 2 or 3, you might want to post a poll to the SoC group and cross-post here to get a definitive consensus. :)

-Angie

New thread, so we don't swamp poor Jakob's. :) Let's get consensus here and then update the coding standards. On 9-Jun-07, at 8:29 PM, Larry Garfield wrote:

On Saturday 09 June 2007, Angela Byron wrote:

...

On 9-Jun-07, at 1:36 PM, Doug Green wrote:

...

Angie,

Why is version_control preferred over versioncontrol?

For legibility. It's consistent with our variable naming convention, and separating the individual words help non-English speakers who can babelfish individual words to figure out what a particular thing is doing.

...

I thought we only used the underscores when the module was extending another module. And I thought we had problems with hook functions related to underscores, or is the problem only related to hook's with underscores?

I know of no such problems. The only thing I can think that would cause a problem is if you named a function something like my_module_insert and Drupal mistook it for a node insert hook when it was just a helper function or something. But mymodule_insert would have exactly the same problem.

It also makes it impossible to definitively extract the providing module from an arbitrary function name, because you don't know if you should break on the first or second underscore to get the module name. That's a problem that I ran into with the menu-split patch in some iterations. Sharing a namespace between modules and hook names is nasty and prevents a lot of useful introspection.

Why can't you just do: <?php $list = module_list(); foreach ($list as $module) { // Check for existence of function or whatever you're doing. if (module_implements($module .'_func') { ... } } ?> The module name and the prefix will always match, except in the case where an underscore prefixes a function name, but again, that problem surfaces for both _mymodule_func and mymodule_func. Relying on the module name to NOT have underscores in it seems brittle and error-prone to me. There are underscores-o-plenty in http://cvs.drupal.org/viewcvs/drupal/contributions/modules/, regardless of what we actually decide the standard should be. :) If that means we need to make our utility functions more intelligent, then we should probably do that. -Angie

On Sunday 10 June 2007, Angela Byron wrote:

...

It also makes it impossible to definitively extract the providing module from an arbitrary function name, because you don't know if you should break on the first or second underscore to get the module name. That's a problem that I ran into with the menu-split patch in some iterations. Sharing a namespace between modules and hook names is nasty and prevents a lot of useful introspection.

Why can't you just do:

<?php $list = module_list(); foreach ($list as $module) { // Check for existence of function or whatever you're doing. if (module_implements($module .'_func') { ... } } ?>

I ran into this issue in the context of "which file do we need to include so that this function becomes available". The file was module-dependent. The function then did not exist yet, so module_implements() and function_exists() won't work. Checking all possible permutations of the module/function can get expensive, too. And then there's namespace collisions. Take for example og and og_vocab. How can core tell whether og_vocab_foo() is the vocab_foo() function of og or the foo() function of og_vocab? I rejected including or trying to include the various files it could be in out of hand as way too inefficient. "Guess and check" is not a solution.

Relying on the module name to NOT have underscores in it seems brittle and error-prone to me. There are underscores-o-plenty in http://cvs.drupal.org/viewcvs/drupal/contributions/modules/, regardless of what we actually decide the standard should be. :) If that means we need to make our utility functions more intelligent, then we should probably do that.

Yes, there are lots of modules that do have underscores and lots that don't. There is no standard right now; certainly not one that's enforced. Were we to start doing so, however, no-underscore is far better as it allows us better introspection capabilities. I personally find it more readable as well, but I'm sure others will disagree. :-) -- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson

On Jun 10, 2007, at 10:26 AM, Derek Wright wrote:

2) Drupal hates CamelCapsToSeparateWords, so we have to use underscores_to_separate_words, unless you really expect everyone to be able to understand nothingtoseparatewords.

p.s. thatsnotafaircomparison sinceyou alreadyknewtheseparators bythetime youreadthelastone. -dww p.p.s. Even this is being generous with some spaces to aid readability, but I hope you get the point. ;)

Quoting Chris Johnson <cxjohnson@gmail.com>:

Usnativeshavenoproblematallreadingthiskindofstuff.

Butsomeuscitizenshavefailingeyesightandhavetoreaditmorethanoncetogetitright. This_is_a_bit_more_ridculous_to_type. ICanReadThisAndTypeItALittleMoreReasonablyButIntroducesTheTwoHumpedCamel. All and all I think it comes down to the module creators choice. The original thread was considering a name for VCS which every techy probably knows what that is for; it is an acronym for Virus Control System. Acronyms are not alway understandable and should be avoided. versioncontrol.module for a file name is good because it is a file name. The README.txt and the product pages would spell it out for those who cannot read that it is version control. Earnie

FWIW, I've never had any trouble typing underscores. I definitely do prefer them to camelCase. I just think it looks cleaner. Not sure this should be considered a hugely critical issues for contrib projects, but I'd like to keep the code style for core as is. Richard Morse wrote:

On Jun 11, 2007, at 6:16 AM, Chris Johnson wrote:

...

CamelCase can get really out of hand, but I hate typing underscores with a passion. It's in a really weak (and worse, sometimes variable) location on the keyboard. I'm a touch typist -- I can scream along as long as I stay out of the symbols. But having to type - or _ between letters really ticks me off, because then I spend 10-20% of my time correcting mistakes.

I don't know what the solution is, but the choices seem to be between bad and worse. :-(

You could learn how to type in the Dvorak layout. Then, the '-' (and, perforce, the '_') key are where the "'" key is now, so you just use the pinky of your right hand...

HTH, Ricky

The information transmitted in this electronic communication is intended only for the person or entity to whom it is addressed and may contain confidential and/or privileged material. Any review, retransmission, dissemination or other use of or taking of any action in reliance upon this information by persons or entities other than the intended recipient is prohibited. If you received this information in error, please contact the Compliance HelpLine at 800-856-1983 and properly dispose of this information.

-- Sean Robertson Web Developer NGP Software, Inc. seanr@ngpsoftware.com (202) 686-9330 http://www.ngpsoftware.com

I ran into this issue in the context of "which file do we need to include so that this function becomes available". The file was module-dependent. The function then did not exist yet, so module_implements() and function_exists() won't work.

many keep saying that, but there is an easy fix. just use module_invoke('og', 'vocab') if thats what is meant. then module_invoke can do the needed includes. module invoke makes explicit the module that owns the function.

Larry Garfield wrote:

On Sun, 10 Jun 2007 22:13:04 -0400, Moshe Weitzman <weitzman@tejasa.com> wrote:

...

...

I ran into this issue in the context of "which file do we need to include so that this function becomes available". The file was module-dependent. The function then did not exist yet, so module_implements() and function_exists() won't work. many keep saying that, but there is an easy fix. just use module_invoke('og', 'vocab') if thats what is meant. then module_invoke can do the needed includes. module invoke makes explicit the module that owns the function.

That again assumes that the code running that line knows which part of the function is the module and which isn't. If you are given the string:

"foo_bar_baz_ugh_agh"

Is that the foo module, function bar_baz_ugh_agh? Or the foo_bar_baz module function ug_agh? Or foo_bar_baz_ugh module function agh? That cannot be determined without a guess-and-check method. That's precisely why I didn't go that route for the menu-split patch.

You are assuming that I am assuming :). I hadn't spelled it all out yet ... Right now, hook_menu() accepts only a string as the value of 'page callback' element. lets say we allow a simple array with 2 values. the values will be the first two arguments of module_invoke(). in this case menu_execute_active_item() would then perform a module_invoke() instead of a direct call_user_func_array(). then the including of the right file gets done in module_invoke(), not directly specified in the menu item definition. here is an example from current HEAD and how I propose. Note that 'file' element is gone, and we know definitively that it belongs to system.module based on the page callback value. CURRENT $items['admin/build'] = array( 'title' => 'Site building', 'page callback' => 'system_admin_menu_block_page', 'file' => 'system.admin.inc', PROPOSED $items['admin/build'] = array( 'title' => 'Site building', 'page callback' => array('system, 'admin_menu_block_page'), ( Note that we may still have to support a string as a page callback (i think) for non module defined callbacks.

you do so based on the first part of the function. so in my example below, 'admin' is the part and thus system.admin.inc is known. sorry, i omitted that. it was buried in the issue where larry added the include parameter. that file would only be included if it existed, of course. Peter Wolanin wrote:

Moshe - how does this make sense? You no longer know which file to include for the callback.

-Peter

On 6/11/07, Moshe Weitzman <weitzman@tejasa.com> wrote:

...

CURRENT $items['admin/build'] = array( 'title' => 'Site building', 'page callback' => 'system_admin_menu_block_page', 'file' => 'system.admin.inc',

PROPOSED $items['admin/build'] = array( 'title' => 'Site building', 'page callback' => array('system, 'admin_menu_block_page'), (

Note that we may still have to support a string as a page callback (i think) for non module defined callbacks.

That's the other reason I rejected that method for menu-split. :-) Too much magic, especially with so much legacy code. --Larry Garfield On Mon, 11 Jun 2007 16:10:58 -0400, "Peter Wolanin" <pwolanin@gmail.com> wrote:

This seems like a little too much "magic" in terms of the file and function naming, and perhaps too rigid as well. What if I want to include both admin and export function in one .inc file?

-Peter

On 6/11/07, Moshe Weitzman <weitzman@tejasa.com> wrote:

...

you do so based on the first part of the function. so in my example below, 'admin' is the part and thus system.admin.inc is known. sorry, i omitted that. it was buried in the issue where larry added the include parameter. that file would only be included if it existed, of course.

Peter Wolanin wrote:

...

Moshe - how does this make sense? You no longer know which file to include for the callback.

-Peter

On 6/11/07, Moshe Weitzman <weitzman@tejasa.com> wrote:

...

CURRENT $items['admin/build'] = array( 'title' => 'Site building', 'page callback' => 'system_admin_menu_block_page', 'file' => 'system.admin.inc',

PROPOSED $items['admin/build'] = array( 'title' => 'Site building', 'page callback' => array('system, 'admin_menu_block_page'), (

Note that we may still have to support a string as a page callback (i think) for non module defined callbacks.

If a module needs to use a function in another module that is a page callback, then the mechanism is already there to include it cleanly. If a module needs to use a function in another module that is not a page callback, it should still be in the main .module file and therefore always available. If a module author puts a function that is not a page callback into an include file, thus hiding it from other modules, that's a bug with that module and should be filed as such. :-) Page callbacks are low-hanging fruit. They're easy to split off with little code and get a lot of performance benefits from. Splitting off API functions and hooks is more difficult because they DO get called at random times, unlike page callbacks. That's not to say it's impossible, but it's more difficult as you have to make sure it's always-callable and you don't balloon the number of includes on a page. (Disk IO == expensive.) The trade-off point there is far less clear. I actually originally wanted to allow multiple includes per menu entry in order to support library files (from another module or from /includes), but chx insisted we hold off on that for simplicity. That's something we can revisit in D7 after we see how the page splitting works out. On Monday 11 June 2007, Moshe Weitzman wrote:

ah, thats the other thing that annoyed me. this derogatory term "magic". larry commented in the issue that module developers need full control over function naming and so on. this is laughable. drupal already has strict conventions for function naming - the hook system. my proposal is more of the same, except is is a convenience not a requirement. these are *conventions*, not magic. conventions are powerful, and are popular in many apps like drupal and rails.

"we have so much legacy code". all that code is not affected until you start splitting up your module into .inc files. this is a moot point.

we can keep the include element if people want to, so they can include random files. that will solve the desire to put an export_ function in the admin file.

this isn't just about the menu system. i think we've hurt interoperability of modules with larry's patch. i think modules are going to have to start including each others files in order to get the features they want. thats dirty. it shouldn't happen, but it will. my proposal gets us past that. all a module developer has to do is use module_invoke(<other module>, <function>) and the right includes will happen. this isn't foolproof, but i do think it is an improvement.

Peter Wolanin wrote:

...

This seems like a little too much "magic" in terms of the file and function naming, and perhaps too rigid as well. What if I want to include both admin and export function in one .inc file?

-Peter

On 6/11/07, Moshe Weitzman <weitzman@tejasa.com> wrote:

...

you do so based on the first part of the function. so in my example below, 'admin' is the part and thus system.admin.inc is known. sorry, i omitted that. it was buried in the issue where larry added the include parameter. that file would only be included if it existed, of course.

Peter Wolanin wrote:

...

Moshe - how does this make sense? You no longer know which file to include for the callback.

-Peter

On 6/11/07, Moshe Weitzman <weitzman@tejasa.com> wrote:

...

CURRENT $items['admin/build'] = array( 'title' => 'Site building', 'page callback' => 'system_admin_menu_block_page', 'file' => 'system.admin.inc',

PROPOSED $items['admin/build'] = array( 'title' => 'Site building', 'page callback' => array('system, 'admin_menu_block_page'), (

Note that we may still have to support a string as a page callback (i think) for non module defined callbacks.

-- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson

Until we get to PHP 5.2 and then we can use the __autoload function. Of course, that would mean that modules would have to be classes, too. ;-)

True. Or true enough. We could add a magic drupal module in class wrapper. The pros will come from namespace separation and transparent includes. The cons, probably, speed. That bit needs to be evaluated very carefully. Otherwise, it will be trivial to do some kind of a module in a class adapter. Is it worth it? Don't ask me :)