menu_tail should automatically act as a load function as well to allow slashes in search

Some comments would be helpful. I have no idea when to use this, nor even what it does.

This would be very useful for auto-completion functions. It's unfortunate that it feels like %menu_tail was added as an incomplete feature. :/

See
#93854: Allow autocompletion requests to include slashes

"arguments" is misspelled:

+            if (empty($item['load argumnets']) && $function == 'menu_tail_load') {

For efficiency's sake, here's the same exact patch except with the typo fixed.

Could we get some PHPDoc for menu_tail_load()?

subscribe

Here's some very simple Doxygen for this and also for menu_tail_to_arg(). As chx wrote, not much has to be written here as these are automatically called functions documented here: http://drupal.org/node/109153#load (although this needs to be slightly updated for 7.x, namely the part saying that menu_tail_load doesn't exist).

This seems a little too hard-coded:

+            if (empty($item['load arguments']) && $function == 'menu_tail_load') {
+              $item['load arguments'] = array('%map', '%index');
+            }

Can we just make it:

+            if (empty($item['load arguments'])) {
+              $item['load arguments'] = array('%map', '%index');
+            }

Wouldn't that make this sort of an API change, since you'd now be passing two additional arguments to every load function?

ignore #12 - I was confusing load arguments with something else. I think we have little choice but to special-case this if we want it to work.

What about requiring hook_menu()s to manually specify those load arguments, if they want to use menu_tail_load()?
A bit harder on the developers, but
- we wouldn't need a hard-coded special case,
- probably only few will use it anyways, and
- those should be able to read the documentation of what they are using.
I think this will be an API change anyways, since a page callback might now get different arguments, if %menu_tail was previously used without intended load functionality.

In any case, +1 for doing this, in some way. Has arguably been a bit of a pain up to now.

core only uses it one place - I think specifying the load arguments might be preferred?

modules/search/search.module:211:      $items["$path/%menu_tail"] = array(

Well, if PHP will be very, very unhappy, then module developers will almost immediately realize their mistake and add those arguments. It's not an "elaborate dance", it's two strings and would be well-documented.

Here's the patch putting it just in hook_menu.

Looks good to me, seems to work perfectly and the bot doesn't complain either.

+++ modules/search/search.test
@@ -282,6 +282,31 @@ class SearchPageText extends DrupalWebTestCase {
+class SearchSlashTestCase extends DrupalWebTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'Search slash',
+      'description' => 'Tests that searches containing slashes work.',
+      'group' => 'Search'

Missing phpDoc for the test class.

But on a second thought, I wonder why this is a separate test case in the first place? Isn't there an existing one already?

+++ modules/search/search.test
@@ -282,6 +282,31 @@ class SearchPageText extends DrupalWebTestCase {
+    // This verifies whether the arg appears in the search form. Not as
+    // accurate as writing a complicated xpath but works.
+    $this->assertRaw("\"$arg");

Can we at least use single quotes and string concatenation for the value, and make the comment actually state what is being asserted here, and why?

Powered by Dreditor.

Looks like it can easily be merged with the prior test case - that's pretty short and generic.

Still works perfectly for me. (Sorry I forgot about this issue …)
Let's just let the test bot check this once again (although, as said, it still applies cleanly).

#24: 298561-menu-tail-load-24.patch queued for re-testing.

#24: 298561-menu-tail-load-24.patch queued for re-testing.

D7 is closed for features.

webchick, right now date search is broken unless this patch goes in. i.e., searching for "1/15/2001" or "9/11" does not work on Drupal 7 (see #915214: Regression: Slashes broken in search for more info on this).

Agreed - this is a bug fix. "task" was more a carry-over before we realized what else was broken.

Ah, ok. I think I misread when I was whipping through these the other day. Sorry!

Committed to HEAD.