jdaugherty commented on code in PR #16476:
URL: https://github.com/apache/grails-core/pull/16476#discussion_r4166172890
##########
build-logic/docs-core/src/main/groovy/grails/doc/DocPublisher.groovy:
##########
@@ -349,6 +349,7 @@ class DocPublisher {
chapterVars.sectionNumber = (i + 1).toString()
writeChapter(chapter, template, sectionTemplate, guideSrcDir,
refGuideDir.path, fullContents, chapterVars)
}
+ linkAcrossChapters(chapters.collect { new File(refGuideDir,
"${it.name}.html") })
Review Comment:
Follow-up, not blocking: this covers the chapter pages, but the reference
pages have the same kind of links. On 7.0.x these go nowhere:
- `ref/Controllers/redirect.adoc`: `<<namedMappings,named URL mapping>>`
- `ref/Services/scope.adoc`: `<<scopedServices,Scoped Services>>`
- `ref/Tags - Fields/field.adoc`: `link:#customizingFieldRendering[...]`,
and `link:#embeddedProperties[...]`, whose target exists on no page (the
section ID is `fieldsEmbeddedProperties`)
`pageById` already knows which chapter page holds each ID, so running the
same rewrite over the `ref/` pages once they are written, with `../../guide/`
in front of the chapter page (the `{guidePath}` the GSP tag pages already use),
would fix the first three and keep future `<<id>>` references in the reference
pages working. The fourth needs its source changed to
`<<fieldsEmbeddedProperties,...>>`.
##########
grails-doc/src/en/guide/theWebLayer/fields/customizingFieldRendering.adoc:
##########
@@ -296,7 +296,7 @@ NOTE: If the `bean` attribute was not supplied to `f:field`
then `bean`, `type`,
If the `label` attribute is not supplied to the `f:field` tag then the label
string passed to the field template is resolved by convention. The plugin uses
the following order of preference for the label:
-* An i18n message using the key '_beanClass_._path_.label'. For example when
using `<f:field bean="authorInstance" property="book.title"/>` the plugin will
try the i18n key `author.book.title.label`. If the property path contains any
index it is removed so `<f:field bean="authorInstance"
property="books<<0>>.title"/>` would use the key `author.books.title.label`.
+* An i18n message using the key '_beanClass_._path_.label'. For example when
using `<f:field bean="authorInstance" property="book.title"/>` the plugin will
try the i18n key `author.book.title.label`. If the property path contains any
index it is removed so `<f:field bean="authorInstance"
property="books[0].title"/>` would use the key `author.books.title.label`.
Review Comment:
Same problem as `books<<0>>` in a file this PR doesn't touch:
`ref/Plug-ins/codecs.adoc` has `` `encodeAs<<Codec>>` `` and ``
`decode<<Codec>>` ``, which render as links reading `[Codec]` to an ID that
doesn't exist. `encodeAs<Codec>`, as in the Command Line fix, would show
the intended placeholder. Fine as a follow-up.
##########
build-logic/docs-core/src/main/groovy/grails/doc/DocPublisher.groovy:
##########
@@ -514,6 +515,32 @@ class DocPublisher {
return varsCopy.content
}
+ /**
+ * Points each fragment link on a chapter page whose target is on another
chapter page at
Review Comment:
Follow-up, not blocking: the sub-section pages that `writePage` also writes
to `guide/pages/` (301 of them) aren't rewritten. A link scan of a
`publishGuide` build finds 762 links on those pages that lead nowhere. Nothing
in the guide links to `pages/`, so only old bookmarks and search results land
there, and the TODO in `writePage` already questions keeping them. Either give
them the same rewrite (with `../` in front of the chapter page) or stop
generating them; that seems worth its own issue.
##########
build-logic/docs-core/src/main/groovy/grails/doc/DocPublisher.groovy:
##########
@@ -514,6 +515,32 @@ class DocPublisher {
return varsCopy.content
}
+ /**
+ * Points each fragment link on a chapter page whose target is on another
chapter page at
+ * that page. A cross reference such as {@code <<unitTesting>>} is
rendered as
+ * {@code href="#unitTesting"}, which only works in the single-page guide.
When more than
+ * one chapter defines the target, the first one wins, as it does in the
single-page guide.
+ */
+ protected void linkAcrossChapters(List<File> chapterPages) {
+ Map<File, Set<String>> idsByPage = [:]
+ Map<String, String> pageById = [:]
+ for (page in chapterPages) {
+ Set<String> ids = (page.getText(encoding) =~
/(?<![\w-])id="([^"]+)"/).collect { it[1] } as Set<String>
+ idsByPage[page] = ids
+ ids.each { pageById.putIfAbsent(it, page.name) }
+ }
+ for (page in chapterPages) {
+ String html = page.getText(encoding)
Review Comment:
Nit: each chapter page is read twice, once in the loop above to collect its
IDs and again here. Keeping the text from the first read would avoid the
second. It makes no practical difference at the guide's size, so feel free to
ignore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
