Re-implement improvements from 5.x-2.5

Good point. So let's start with that :)

Hrm. Why copying? ;)

Attached patch re-implements 90% of all new features in 5.x-2.5.

Missing: Views support -- however, I didn't look into Views 2 yet. It's possible that it already exposes admin links like CCK does. Leaving this for a follow-up patch and committing this one first.

Well, do you mean to alter the update this way?

Also moved all menu callbacks into admin_menu.inc.

Thanks for reviewing. Yes, that access check is indeed duplicate and removed in attached patch.

Thanks for the additional clean-up! :)

Re #11: Yes. I still can't believe why it has ever been considered better to have those package categories opened by default. The admin/build/modules page is unusable this way, at least on all of our sites (with commonly > 50 modules installed). So it's absolutely required to have those fieldsets closed by default. All in all, one of the primary goals of admin_menu is to improve usability/user experience in all cases where core lacks.

Since this was your first commit to admin_menu, there is (like always) room for improvement:

1. Testing: As shown at drupalmodules.com, admin_menu is not 100% reliable, thus lacks in testing. The last patch still contained a bug in update 6000 (introduced by myself; see attached patch) and introduced a new one (see below). Let's review and test each patch at least once.
2. Variable renaming: I'm not very happy with renaming the variable $current_state to $saved_state, because a) admin_menu_toggle_modules() is now inconsistent with admin_menu_admin_menu(), and b) porting improvements between D5 and D6 gets even harder, if both branches also include unnecessary changes like this. But, well, since $saved_state is more appropriate than $current_state, we have to merge this change into D5 now.
3. Code clean-up: Same as previous, such changes need either to be avoided, or merged into DRUPAL-5--2 as well. Of course, merging is definitely preferred. For this reason, I've switched the order of hook_menu() and hook_init() in attached patch for D6.
4. HEAD: Good job! However, you missed to create a corresponding changelog entry. In rare cases where there's no issue to reference, I just omit the #1234 part.
5. Commit message: As mentioned earlier, all commit messages should always be 1:1 identical to changelog entries and adhere to Drupal's best practice on commit messages (which is actually a Standard for admin_menu).

After disabling devel modules, the following error message appears, permanently:

warning: call_user_func_array() [function.call-user-func-array]: First argument is expected to be a valid callback, 'devel_menu_title_theme_developer' was given in .\includes\menu.inc on line 501.

I've already committed attached patch for 5.x-2.x.

btw: HEAD/7.x still depends on #268050: Unable to create dev snapshot from HEAD for D7 unfortunately...

erm... no, patch in #15 fixes a missing [] in update 6000. The permanently displayed error message after disabling devel modules (probably introduced by your patch in #11) still needs to be fixed.

The clean-up patch for DRUPAL-5--2 is the only one I've committed so far.

Yes, of course, as already outlined in #15: $saved_state is inconsistent currently, missing punctuation in messages in admin_menu_toggle_modules(), missing variable_dels in hook_uninstall(), and re-ordering of hook_menu() and hook_init() to make merging differences between 5.x and 6.x easier.

We should probably move that rebuilding code into a helper function, since #173570: Add link to disable non-required core modules for upgrading will require it, too.

Heh... well, yes, I think that is the best way. Thanks for this pointer; it makes #270691: Return full path for drupal_get_path()/drupal_get_filename() obsolete. ;)

I also didn't know that .install files compatible to a previous version of core are loaded during an upgrade. Anyway, it seems that would unnecessarily hold-off this patch. So I've replaced the two instances of include_once() in attached patch, and after a final test/confirmation to ensure everything works, it can be committed.

Great!

I've updated the release notes to outline all changes since 5.x-2.4, using the cvs-release-notes script (which is another benefit of following the changelog/commit message syntax).