You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by "Karsten Beyer (JIRA)" <ji...@apache.org> on 2008/06/06 14:16:45 UTC
[jira] Created: (SHINDIG-351) Featureloader legibility of register
recursion / Bugs in register recursion
Featureloader legibility of register recursion / Bugs in register recursion
---------------------------------------------------------------------------
Key: SHINDIG-351
URL: https://issues.apache.org/jira/browse/SHINDIG-351
Project: Shindig
Issue Type: Bug
Components: Gadget Rendering Server (PHP)
Environment: Lighttpd, Mac OS X (should aplly to all)
Reporter: Karsten Beyer
Priority: Minor
The variable names in the JSFeatureLoader.php ->loadFeatures and JSFeatureLoader->register methods are very confusing. The same item has different names in different methods.
Also there were two logical bugs in the code.
First the loadFiles method gets the $deps (<- confusing name, as this holds in fact all features, and no dependencies at all) as a reference. Nevertheless this is the call:
$deps = $this->loadFiles($path, $deps);
and in loadFiles: return $features;
This is unnecessary as loadFiles is anyway working on a reference of this object.
The second logical bug is with the $registered array in the register method. First of all, the value is set, but the key is checked:
if (isset($registered[$feature->name])) {
...
$registered[] = $feature->name;
which means that this is not considered at all and all features are registered multiple times (which is later no big problem, as they are overwritten, but unnecessary nevertheless). The second issue is that the $registered array is not passed as reference, which means that it stays empty during the recursion and does not work at all.
I have produced a patch that fixes all these issues with the loadFeatures functionality (including some changes in the locally used variable names to make it easier to understand what is going on), which i will attach in a few minutes..
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-351) Featureloader legibility of register
recursion / Bugs in register recursion
Posted by "Karsten Beyer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-351?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Karsten Beyer updated SHINDIG-351:
----------------------------------
Attachment: fix-issue-SHINDIG-351_2.patch
updated patch
> Featureloader legibility of register recursion / Bugs in register recursion
> ---------------------------------------------------------------------------
>
> Key: SHINDIG-351
> URL: https://issues.apache.org/jira/browse/SHINDIG-351
> Project: Shindig
> Issue Type: Bug
> Components: Gadget Rendering Server (PHP)
> Environment: Lighttpd, Mac OS X (should aplly to all)
> Reporter: Karsten Beyer
> Priority: Minor
> Attachments: fix-issue-SHINDIG-351_2.patch
>
>
> The variable names in the JSFeatureLoader.php ->loadFeatures and JSFeatureLoader->register methods are very confusing. The same item has different names in different methods.
> Also there were two logical bugs in the code.
> First the loadFiles method gets the $deps (<- confusing name, as this holds in fact all features, and no dependencies at all) as a reference. Nevertheless this is the call:
> $deps = $this->loadFiles($path, $deps);
> and in loadFiles: return $features;
> This is unnecessary as loadFiles is anyway working on a reference of this object.
> The second logical bug is with the $registered array in the register method. First of all, the value is set, but the key is checked:
> if (isset($registered[$feature->name])) {
> ...
> $registered[] = $feature->name;
> which means that this is not considered at all and all features are registered multiple times (which is later no big problem, as they are overwritten, but unnecessary nevertheless). The second issue is that the $registered array is not passed as reference, which means that it stays empty during the recursion and does not work at all.
> I have produced a patch that fixes all these issues with the loadFeatures functionality (including some changes in the locally used variable names to make it easier to understand what is going on), which i will attach in a few minutes..
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-351) Featureloader legibility of register
recursion / Bugs in register recursion
Posted by "Karsten Beyer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-351?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Karsten Beyer updated SHINDIG-351:
----------------------------------
Attachment: fix-issue-SHINDIG-351.patch
Patch improves understanding of the loadFeatures functionality by changing some variable names and fixes the logical bugs described in the issue.
> Featureloader legibility of register recursion / Bugs in register recursion
> ---------------------------------------------------------------------------
>
> Key: SHINDIG-351
> URL: https://issues.apache.org/jira/browse/SHINDIG-351
> Project: Shindig
> Issue Type: Bug
> Components: Gadget Rendering Server (PHP)
> Environment: Lighttpd, Mac OS X (should aplly to all)
> Reporter: Karsten Beyer
> Priority: Minor
> Attachments: fix-issue-SHINDIG-351.patch
>
>
> The variable names in the JSFeatureLoader.php ->loadFeatures and JSFeatureLoader->register methods are very confusing. The same item has different names in different methods.
> Also there were two logical bugs in the code.
> First the loadFiles method gets the $deps (<- confusing name, as this holds in fact all features, and no dependencies at all) as a reference. Nevertheless this is the call:
> $deps = $this->loadFiles($path, $deps);
> and in loadFiles: return $features;
> This is unnecessary as loadFiles is anyway working on a reference of this object.
> The second logical bug is with the $registered array in the register method. First of all, the value is set, but the key is checked:
> if (isset($registered[$feature->name])) {
> ...
> $registered[] = $feature->name;
> which means that this is not considered at all and all features are registered multiple times (which is later no big problem, as they are overwritten, but unnecessary nevertheless). The second issue is that the $registered array is not passed as reference, which means that it stays empty during the recursion and does not work at all.
> I have produced a patch that fixes all these issues with the loadFeatures functionality (including some changes in the locally used variable names to make it easier to understand what is going on), which i will attach in a few minutes..
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-351) Featureloader legibility of
register recursion / Bugs in register recursion
Posted by "Karsten Beyer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-351?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12603016#action_12603016 ]
Karsten Beyer commented on SHINDIG-351:
---------------------------------------
there seems to be some bug left in the patch. ignore for now. working on some updated version...
> Featureloader legibility of register recursion / Bugs in register recursion
> ---------------------------------------------------------------------------
>
> Key: SHINDIG-351
> URL: https://issues.apache.org/jira/browse/SHINDIG-351
> Project: Shindig
> Issue Type: Bug
> Components: Gadget Rendering Server (PHP)
> Environment: Lighttpd, Mac OS X (should aplly to all)
> Reporter: Karsten Beyer
> Priority: Minor
> Attachments: fix-issue-SHINDIG-351.patch
>
>
> The variable names in the JSFeatureLoader.php ->loadFeatures and JSFeatureLoader->register methods are very confusing. The same item has different names in different methods.
> Also there were two logical bugs in the code.
> First the loadFiles method gets the $deps (<- confusing name, as this holds in fact all features, and no dependencies at all) as a reference. Nevertheless this is the call:
> $deps = $this->loadFiles($path, $deps);
> and in loadFiles: return $features;
> This is unnecessary as loadFiles is anyway working on a reference of this object.
> The second logical bug is with the $registered array in the register method. First of all, the value is set, but the key is checked:
> if (isset($registered[$feature->name])) {
> ...
> $registered[] = $feature->name;
> which means that this is not considered at all and all features are registered multiple times (which is later no big problem, as they are overwritten, but unnecessary nevertheless). The second issue is that the $registered array is not passed as reference, which means that it stays empty during the recursion and does not work at all.
> I have produced a patch that fixes all these issues with the loadFeatures functionality (including some changes in the locally used variable names to make it easier to understand what is going on), which i will attach in a few minutes..
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (SHINDIG-351) Featureloader legibility of register
recursion / Bugs in register recursion
Posted by "Chris Chabot (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-351?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Chris Chabot resolved SHINDIG-351.
----------------------------------
Resolution: Fixed
Assignee: Chris Chabot
The feature loader is known to be a bit messy, the basis of PHP shindig was based on java's structure, and that was known to be a bit messy due to the haste in getting something out there. Nice cleanup, thanks
Applied and commited
> Featureloader legibility of register recursion / Bugs in register recursion
> ---------------------------------------------------------------------------
>
> Key: SHINDIG-351
> URL: https://issues.apache.org/jira/browse/SHINDIG-351
> Project: Shindig
> Issue Type: Bug
> Components: Gadget Rendering Server (PHP)
> Environment: Lighttpd, Mac OS X (should aplly to all)
> Reporter: Karsten Beyer
> Assignee: Chris Chabot
> Priority: Minor
> Attachments: fix-issue-SHINDIG-351_2.patch
>
>
> The variable names in the JSFeatureLoader.php ->loadFeatures and JSFeatureLoader->register methods are very confusing. The same item has different names in different methods.
> Also there were two logical bugs in the code.
> First the loadFiles method gets the $deps (<- confusing name, as this holds in fact all features, and no dependencies at all) as a reference. Nevertheless this is the call:
> $deps = $this->loadFiles($path, $deps);
> and in loadFiles: return $features;
> This is unnecessary as loadFiles is anyway working on a reference of this object.
> The second logical bug is with the $registered array in the register method. First of all, the value is set, but the key is checked:
> if (isset($registered[$feature->name])) {
> ...
> $registered[] = $feature->name;
> which means that this is not considered at all and all features are registered multiple times (which is later no big problem, as they are overwritten, but unnecessary nevertheless). The second issue is that the $registered array is not passed as reference, which means that it stays empty during the recursion and does not work at all.
> I have produced a patch that fixes all these issues with the loadFeatures functionality (including some changes in the locally used variable names to make it easier to understand what is going on), which i will attach in a few minutes..
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-351) Featureloader legibility of register
recursion / Bugs in register recursion
Posted by "Karsten Beyer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-351?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Karsten Beyer updated SHINDIG-351:
----------------------------------
Attachment: (was: fix-issue-SHINDIG-351.patch)
> Featureloader legibility of register recursion / Bugs in register recursion
> ---------------------------------------------------------------------------
>
> Key: SHINDIG-351
> URL: https://issues.apache.org/jira/browse/SHINDIG-351
> Project: Shindig
> Issue Type: Bug
> Components: Gadget Rendering Server (PHP)
> Environment: Lighttpd, Mac OS X (should aplly to all)
> Reporter: Karsten Beyer
> Priority: Minor
>
> The variable names in the JSFeatureLoader.php ->loadFeatures and JSFeatureLoader->register methods are very confusing. The same item has different names in different methods.
> Also there were two logical bugs in the code.
> First the loadFiles method gets the $deps (<- confusing name, as this holds in fact all features, and no dependencies at all) as a reference. Nevertheless this is the call:
> $deps = $this->loadFiles($path, $deps);
> and in loadFiles: return $features;
> This is unnecessary as loadFiles is anyway working on a reference of this object.
> The second logical bug is with the $registered array in the register method. First of all, the value is set, but the key is checked:
> if (isset($registered[$feature->name])) {
> ...
> $registered[] = $feature->name;
> which means that this is not considered at all and all features are registered multiple times (which is later no big problem, as they are overwritten, but unnecessary nevertheless). The second issue is that the $registered array is not passed as reference, which means that it stays empty during the recursion and does not work at all.
> I have produced a patch that fixes all these issues with the loadFeatures functionality (including some changes in the locally used variable names to make it easier to understand what is going on), which i will attach in a few minutes..
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.