Add excluded directories to branches
jcnventura - March 18, 2009 - 19:15
| Project: | API |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | jcnventura |
| Status: | closed |
Description
Re-using some of ax's work from #207365: Windows directory separator compatibility, I have added an option to specify directories to excluded from branches.
As with the main directories specification, these must also be absolute.
Similar 'development' modules (such as coder) also provide an option for this, which is useful when using external libraries to prevent the inclusion of those libraries when documenting the integration modules.
João Ventura
| Attachment | Size |
|---|---|
| api_excluded_dirs.patch | 5.11 KB |
#1
1. If I create a new API branch without any excluded directories, I cannot save the branch since I get an error that says "is not a directory."
2. The exclude_dirs schema should probably be a TEXT field since it has a limit of 255 characters. If I have paths like "/home/dave/Projects/www/drupal6dev/sites/all/modules/my_cool_module" I can only add three excluded directories. I'm going to file an issue for the *included* directories as well.
3. Why can't we run these trim()'s in the saving of the branch instead of having to run it every time in api_scan_directories():
<?php+function api_scan_directories($directories, $exclude_dirs) {
$directory_array = explode("\n", $directories);
foreach ($directory_array as $key => $directory) {
- $directory_array[$key] = trim($directory);
+ $directory_array[$key] = rtrim(trim($directory), '/');
+ }
+ $excluded_array = explode("\n", $exclude_dirs);
+ foreach ($excluded_array as $key => $directory) {
+ $excluded_array[$key] = rtrim(trim($directory), '/');
}
?>
#2
@Dave Reid:
1. Yes, that also happened to me, but I forgot to fix it. I will do it ASAP and submit a fresh patch. It's trivial: a simple (!empty) around the foreach loop in the validate function.
2. Changing to a textfield object will reduce it's size and make it harder to read the contents.. There's got to be a better way to handle large excludes.
3. One one hand, because the trims here do not really add that much complexity and performance problems to the code. On the other because doing it when saving the branch requires altering the user input and that can have unintended side-effects.
João
#3
Hi,
This patch fixes points 1 and 3 in Dave Reid's comments in #1.
Point 2 can be trivially fixed by increasing the varchar size of the SQL field in the install file.
João
#4
@jcnventura: You misunderstood my point #2. :)
Instead of:
+ 'exclude_dirs' => array('type' => 'varchar', 'length' => '255', 'not null' => TRUE, 'default' => ''),It should be:
+ 'exclude_dirs' => array('type' => 'text', 'not null' => TRUE),#5
Yes, I did.. However, I will change that from varchar to text when the directories field also changes.
I'd prefer not to mix the problems at this point. I have been manually merging this patch into my API module since September 2008, and I would rather stop doing it..
#6
Patch no longer applies.
#7
Refreshed the patch to apply to the latest 6.x-1.x-dev.. Also changed the type from varchar to text in accordance with the latest changes to the module.
#8
Committed with changes to expand abbreviations and avoid duplicitous code.
#9
Automatically closed -- issue fixed for 2 weeks with no activity.