Updated: Comment #0

Problem/Motivation

While reviewing #2027183: hook_menu() title callback is ignored on routes
I thought this was a mistake.
Turns out it's in 4 other places.
Since it's fixed in one place in that issue, this issue will fix it in the other places so it's not copied around more.

Proposed resolution

Remaining tasks

tbd

User interface changes

No.

API changes

No.

CommentFileSizeAuthor
#15 2027351-menu-example-docs-15.patch3.36 KBmicnap
#12 2027351-menu-example-docs-12.patch3.44 KBlongwave
#10 2027351-menu-example-docs-10.patch3.44 KBlongwave
#9 2027351-menu-example-docs-9.patch3.43 KBlongwave
#3 2027351-2-array-example.patch1.67 KBhelenkim

Comments

yesct’s picture

yesct commented
Title: tidy up grammer in doc copied around: (ex: array('node', '5')) » tidy up grammar in doc copied around: (ex: array('node', '5'))
yesct’s picture

yesct commented 
$ ag "An array of path arguments \(ex: array\('node', '5'\)\)"
core/includes/menu.inc
511: *   An array of path arguments (ex: array('node', '5'))
581: *   An array of path arguments (ex: array('node', '5'))
730: *   An array of path arguments (ex: array('node', '5'))
805: *   An array of path arguments (ex: array('node', '5'))
helenkim’s picture

helenkim commented
StatusFileSize
new 2027351-2-array-example.patch1.67 KB

I found the four lists and tidy up the grammar related on the #2027183-6: hook_menu() title callback is ignored on routes.
- * An array of path arguments (ex: array('node', '5'))
+ * An array of path arguments, for example, array('node', '5').

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Component: menu system » documentation
Status: Active » Reviewed & tested by the community

Looks good.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Thanks! Good cleanup. :)

yesct’s picture

yesct commented

@jhodgdon did you mean to commit it? I didn't see it in the git log.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Haven't done so yet. Will soon. When I do, I'll put an unambiguous message in the issue and change the status. :)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs backport to D7

Actually... Can we fix the punctuation? The comma before "for example" should actually be a semi-colon. If we're going to clean this up, might as well not introduce a new error. :)

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 2027351-menu-example-docs-9.patch3.43 KB

I figured I should fix up all instances of "for example" in menu.inc at the same time.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
StatusFileSize
new 2027351-menu-example-docs-10.patch3.44 KB

Improved consistency a tiny bit more.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Thanks, this all looks good!

Except for one spot -- in the Translate function:

- *   An array of helper function (ex: array(2 => 'menu_tail_to_arg'))
+ *   An array of helper functions; for example, array(2 => 'menu_tail_to_arg'))

This is missing . at the end, and has an extra ) at the end.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 2027351-menu-example-docs-12.patch3.44 KB

Fixed as per #11.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Excellent! This is ready to commit. Thanks!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks again! Committed to 8.x. I think we should backport this to 7.x.

micnap’s picture

micnap commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 2027351-menu-example-docs-15.patch3.36 KB

A patch for changes in D7.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Looks good, thanks!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks again! Committed to 7.x.

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