Ciao

I also think it should be moved out of adminlib.php, since It is probably needed in places where you don't need the rest of adminlib.php. I will probably move it to moodlelib.php if not one objects.

anyway I agree this code needs a bit of cleanup and since it is me who introduced it working on solution now...

Also, can I make an additional request for a get_plugin_dir function that does

quiz -> mod/quiz
qtype_shortanswer -> question/type/shortanswer

and so on. Thanks.

• function get_plugin_types($fullpaths=true) now returns full paths by default
• function get_plugin_types() supports module subtypes - no more hardcoding of quiz reports and friends
• new function get_plugin_directory($plugintype, $name, $fullpaths=true)
• new function get_plugin_list($plugintype, $fullpaths=true)
• normalized component values (aka full plugin names) in capabilities, events and messaging tables
• fixed assignment and quiz plugin names
• fixed savepoint version bug
• get_string() separation of full reports and course reports
• plugin support everywhere
• local/ support completely replaced by plugins in /local/xxxx/*

This code is not tested much,going to test it more today and fix regressions

• http://docs.moodle.org/en/Development:Coding_style#Foreach shows space around => in foreach loops. (foreach ($modules as $module=>$moduledir)
  Unknown macro: { should be foreach ($modules as $module => $moduledir) {) * Instead of changing {code} $mods = get_list_of_plugins('mod'); if (!in_array(strtolower($modulename), $mods)) { {code}
  to
  

  $mods = get_plugin_list('mod');
  
          $mods = array_keys($mods);
  
           if (!in_array(strtolower($modulename), $mods)) {

  
  you could do
  

  
  $mods = get_list_of_plugins('mod');
  if (!array_key_exists(strtolower($modulename), $mods)) {{code}}

• Or, is it worth making a plugin_exists($type, $modulename) function? (No need to get a list of all plugins if you just want to know whether one exists.)

• Similarly, you could write function get_plugins_with_file($type, $filename), so you can say things like get_plugins_with_file('local', 'cron.php'); get_plugins_with_file('gradeimport', 'lib.php'); Would let you delete if(file_exists(...)) code from lots of other places.

• case FEATURE_MOD_SUBPLUGINS: return array('quiz'=>'mod/quiz/report'); is really nice!

• Surely we don't need the if in

  if ($mods = get_plugin_list('mod')) {
  
          foreach ($mods as $mod=>$moddir) {

  I hope get_plugin_list('mod') returns an empty array when there are none.

• @param string $component full plugin name, examples: 'moodle', 'mod_forum'

'mod_forum' is not consistent with how get_string has always worked.

• get_cached_capabilities, the new logic here looks broken. Oh, I see, will you force a complete reload of the table, so the contents of the component field is updated for all rows?

... OK, am up to line 815 of the patch, and need to take a break. More later.

• coding style - oki, going to change my style
• mod - I did not want to do major refactoring at the same time, sometimes I resorted to array_keys - lazy and greater danger of regressions - but in general I agree
• plugin_exists() - yes good idea
• get_plugin_with_file() - yes, good idea
• mod always exist - the same as $mod above
• component string were previously "mod/forum", I personally hate the missing "mod_" prefix in string, tables and everything because it may cause collisions
• get_cached_capabilities was buggy in fact because it did not use component string, you could not define mod/forum:something outside of forum - local hooks

get_plugin_types($fullpaths=true)
get_plugin_list($plugintype, $fullpaths=true, $withfile=null)
get_component_directory($component, $fullpaths=true)

• It seems that the old get_list_of_plugins is still there. Why? (Backwards compatibility, I guess.)

• Wow! locallib.php is completely gone. Is the information from all the useful comments preserved somewhere (in updated form)? I guess it should be on moodle docs.

• $plugs = get_plugin_list($type); I would prefer the variable name $plugins, but I guess the above comment about refactoring applies.

• javascript-mod.php is a bit of a nasty hack. I am hoping to phase it out and let people use $PAGE->requires instead. (Should be better performance than doing is_readable on a file in about a 100 different folders. Therefore, can we leave this doing only mod, block and filter for backwards compatibility.

• Diffstat says:
  48 files changed, 467 insertions, 763 deletions
  300 lines of code saved. Very nice.

In summary, this is brillant. Please get it finishded, tested, and checked in soon

Of the above comments, the only one I feel strongly about is the javascript-mod.php one. Thank you.

• get_list_of_plugins() is there because it is used for lang and otehr custom types, we need it for BC too - those are not true plugins
• yes locallib info needs to be stored somewhere, locallib.txt ?
• yes, longer names are better

You did not comment on the javascript-mod.php bit. I hope that means you agree.

thanks again

In mod/assignment/lib.php line 3140, you use

$assignmenttypes = get_plugin_list('assignment');

This always returns an empty array, no matter what is in mod/assignment/type

Delving a bit deeper, in lib/moodlelib.php lines 6841-6847, we see that there is possible way that get_plugin_list('assignment') could return anything, no matter was exists in mod/assignment.

I wonder why Petr commented that out?

'gformat' => 'mod/glossary/formats',

Peace - Anthony