Document proper use of #autocomplete_path for slashes

I'm not sure a helper would help that much, because you'd still need to call it correctly in PHP 5.2 it would have to look something like this:

This would still be obscure, more so than just documenting it I think.

I'm moving this to D7, because in D8 a better solution would be to just fix menu_tail_load so it can just be used as we'd all like it to be. I'll raise another issue for that. Feel free to disagree with me on this point!

We still should put it in 8.x first. :)

Awesome, thanks @scottalan. That looks like the correct API function. (Once we have finalized the wording, we should probably add it in the FAPI handbook chart as well. See also #100680: Automatically generate Forms API & other big array documentation.)

One correction I have is that these paragraphs of text should go above the parameter documentation rather than below it. It should immediately follow the one-line summary after a blank line. Reference: http://drupal.org/node/1354#functions

Other feedback:

1. +++ b/core/includes/form.incundefined
@@ -3709,6 +3709,36 @@ function theme_vertical_tabs($variables) {
+ * * Here is an example of how to use an autocompletion callback:

   Double asterisk here (oops!). Though, I think we can leave this sentence out entirely.

2. +++ b/core/includes/form.incundefined
@@ -3709,6 +3709,36 @@ function theme_vertical_tabs($variables) {
+ * Autocomplete callback functions tend to have a bug wherein forward-slash
+ * characters cause an error.
+ *
+ * If the request has a '/' in the text, then the menu system will split it
+ * into multiple arguments and recover the intended $keywords.

   I think it would be better to merge these two paragraphs. I'd actually replace the first sentence to be more specific (and to not refer to a bug since we're making this a documented behavior.) Maybe:
   "Note that autocomplete callbacks should include special handling if the user input may contain forward slashes. If the user-submitted string has a '/' in the text that is sent in the autocomplete request, the menu system will split the text and pass it to the callback as multiple arguments."

3. +++ b/core/includes/form.incundefined
@@ -3709,6 +3709,36 @@ function theme_vertical_tabs($variables) {
+ *
+ * @code
+ * '#autocomplete_path' => 'mymodule_autocomplete/' . $some_key . '/' . $some_id,
+ * @endcode
+ *

   I think I'd remove the blank lines before and after the code snippet so that this section forms one paragraph.

4. +++ b/core/includes/form.incundefined
@@ -3709,6 +3709,36 @@ function theme_vertical_tabs($variables) {
+ * and the user types in "keywords" so the full path called is:
+ *
+ * 'mymodule_autocomplete/$some_key/$some_id/keywords'.

   We can remove the blank line between these two. I'd also remove the period at the end of the second line, I think.

   Maybe we can add another paragraph after it that says something like: "Then you should include code like the following to handle slashes in the input:"

5. +++ b/core/includes/form.incundefined
@@ -3709,6 +3709,36 @@ function theme_vertical_tabs($variables) {
+ *   array_shift($args);
+ *   array_shift($args);

   I'd add an inline comment here indicating that this removes $arg1 and $arg2 from the beginning of the array.

6. +++ b/core/includes/form.incundefined
@@ -3709,6 +3709,36 @@ function theme_vertical_tabs($variables) {
+ *   // Your code here.

   I'd add a comment above this line with some more specific information. Maybe something like "The user's original input is now in $keywords, including any slashes."

Great job on this!

changing to needs review...

Is this step necessary for documentation, as to maintain protocol?

Thanks @tstoeckler and @scottalan!

It looks like we still need the second part of point 4, though (paragraph between the "if" part and the "then" part). I think that would make it a little more clear. I'd also like to suggest a slight improvement to the wording so it doesn't sound like a run-on:

Let's make this three sentences:

I think that sounds a little better. With those two changes (as above, plus adding the second sentence from point 4 above), then I'd consider it RTBC too.

@scottalan, Yep, you can set the issue to "Needs Review" any time an updated patch is submitted. This triggers testbot to automatically make sure the patch applies (which is relevant even for documentation patches), and also signals to reviewers that there are new changes to review.

Aside @tstoeckler:

Since @xjm has already had a look at this and all of his her remarks have been fixed

:)

Trying once more :)

Docs look good, would be good to fix this properly in 8.x but documenting the requirements here makes sense to avoid drift.

Committed/pushed to 8.x, moving to 7.x for backport.

+ *   // We store the user's original input in $keywords, including any slashes.

Perhaps we should add a note that a leading or trailing slash in the user's original input will not be kept.

i.e. if the user enters /keywords/ then $keywords will contain keywords.

#17 would need to go into 8.x as well, so a followup would be fine.

The backport is RTBC.

Thanks -- nice work on the documentation here! Committed to 7.x.

Oops.

Thanks, that's pretty close!

But I think #17 (and your example) are saying that slashes are removed from the front and back of the string -- and your wording says "preprended" only (maybe say "leading and trailing"?).

Also, a nitpick: "i.e. If the..." should be "I.e., if the..." (i.e. starts this sentence)... well actually, it should be "E.g." or better yet just "For example, ". i.e. means "that is", and this is an example.

I also think the example would be better if the sample text was something other than "keywords" and if it contained a slash. Maybe use "/a/few/words/" as the input text?

That looks good to me, thanks! Assuming the test bot agrees, I'll get it committed to 8.x and 7.x (not sure if it will need a port/reroll for 7 or not until I try).

Automatically closed -- issue fixed for 2 weeks with no activity.