Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin hooks and settings fixes #140

Merged

Conversation

vendethiel
Copy link
Collaborator

No description provided.

}
}

// if the plugin has a class (events, etc), register it.
$plugin_class_name = 'plugin_' . $k;
if (class_exists($plugin_class_name))
$plugin_class_name = 'class_plugin_' . $k;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change the name of the class? I can't remember why I have used that name, but maybe some private plugins are using the other format... I need to check before integrating your code.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After having looked at the code and analyzed other plugins, I still prefer to keep the inclusion of the autoload class in a simpler way, so every plugin may have a file called "class.php" within his root folder which gets included automatically. I will accept the name change of the class, but unless there is another good reason, I won't add another INCLUDE to common.php (it just complicate the code, without adding anything really beneficial, at least at a first look).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class.php doesn't get loaded automatically, does it?

@@ -71,11 +71,11 @@ function create_input($name, $properties)
break;

case 'LIST_CHECKBOX':
case 'LIST_FLAGS':
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you moved this below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because in the case of a LIST_FLAGS, we don't want an array of default values.

@@ -26,7 +26,7 @@ class class_plugins
var $plugins_path = '';
var $plugins_settings_path = '';

var $plugin_includes_array = array('constants', 'common', 'functions', 'class');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer keeping 'class' here. We will discuss privately the reason

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, 'class' was an addition I made that broke things.

global $config;

$files = array();
$base_dir = '../../' . PLUGINS_PATH;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you didn't use IP_ROOT_PATH instead of '../../'?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the template class doesn't know how to handle IP_ROOT_PATH. I tried it.

@MightyGorgon MightyGorgon merged commit 3738d7b into MightyGorgon:master Sep 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants