Copilot commented on code in PR #2845:
URL: https://github.com/apache/tika/pull/2845#discussion_r3324845000


##########
docs/publish-docs.sh:
##########
@@ -28,6 +28,30 @@ set -euo pipefail
 cd "$(dirname "$0")"
 
 PUBLISH_DIR="${1:?usage: publish-docs.sh <tika-site-publish-dir>}"
+
+# Guard the 'rm -rf' below: the publish dir must already exist (it's a
+# tika-site checkout, not something we create) and not be a dangerously
+# short/root path that a typo could expand to.
+if [[ ! -d "${PUBLISH_DIR}" ]]; then
+    echo "PUBLISH_DIR '${PUBLISH_DIR}' is not an existing directory." >&2
+    echo "Point it at a tika-site 'publish/' checkout." >&2
+    exit 1
+fi
+PUBLISH_DIR="$(cd "${PUBLISH_DIR}" && pwd -P)"
+if [[ "${#PUBLISH_DIR}" -lt 4 || "${PUBLISH_DIR}" != *"/"* ]]; then
+    echo "Refusing to operate on suspiciously short PUBLISH_DIR 
'${PUBLISH_DIR}'." >&2
+    exit 1
+fi
+# Confirm this looks like a tika-site 'publish/' dir: the documented argument
+# is always <tika-site-checkout>/publish, and the 'svn add publish/docs
+# publish/_' step downstream hardcodes that name. Refusing a non-'publish'
+# basename catches a wrong-but-valid checkout before we 'rm -rf' inside it.

Review Comment:
   This comment explains the `publish/` basename check by referencing 
downstream SVN steps hardcoding paths, but it only mentions `publish/docs` and 
`publish/_`. With this script now writing `search-index.js` at the publish 
root, downstream steps also need to account for `publish/search-index.js`, so 
the comment is now incomplete/misleading.



##########
docs/supplemental-ui/partials/header-content.hbs:
##########
@@ -1,26 +1,25 @@
 <header class="header">
-  <nav class="navbar">
+  <nav class="navbar" aria-label="Main">
     <div class="navbar-brand">
       <a class="navbar-item" href="{{or siteRootPath (or site.url '/')}}">
         <img src="{{{uiRootPath}}}/img/ASF_Tika-colour.svg" alt="Apache Tika" 
style="height: 2rem; margin-right: 0.5rem; background: white; padding: 2px 4px; 
border-radius: 3px;">
         {{site.title}}
       </a>
-      <button class="navbar-burger" aria-label="Toggle navigation" 
data-target="topbar-nav">
+      {{#if env.SITE_SEARCH_PROVIDER}}
+      <div class="navbar-item search hide-for-print" role="search">
+        <div id="search-field" class="field">
+          <input id="search-input" type="search" aria-label="Search the docs" 
placeholder="Search the docs"{{#if page.home}} autofocus{{/if}}>
+        </div>
+      </div>
+      {{/if}}

Review Comment:
   The search box is now gated on `env.SITE_SEARCH_PROVIDER`, but this repo’s 
Antora build config doesn’t set that environment variable (no occurrences in 
`docs/pom.xml` or playbooks). As a result, the search input will never render 
in typical local/CI builds even though the Lunr extension is enabled, 
effectively disabling search UI.



##########
docs/publish-docs.sh:
##########
@@ -40,12 +64,22 @@ mkdir -p "${DOCS_DIR}"
 
 # Strip the 'tika/' component dir prefix so URLs are /docs/X.Y.Z/...
 cp -r target/site/tika/* "${DOCS_DIR}/"
-# UI assets one level above docs/, since HTML uses ../../_/ relative paths
-cp -r target/site/_/ "${PUBLISH_DIR}/_/"
+# UI assets one level above docs/, since HTML uses ../../_/ relative paths.
+# Replace wholesale: cp -r into an existing directory nests source as a
+# subdirectory (publish/_/_/), so remove first to keep the layout flat.
+# Refuse if '_' is a symlink: 'rm -rf _/' would follow it and wipe the
+# target's contents, and the cp below needs a real directory here anyway.
+if [[ -L "${PUBLISH_DIR}/_" ]]; then
+    echo "Refusing to remove '${PUBLISH_DIR}/_': it is a symlink, not a 
directory." >&2
+    exit 1
+fi
+rm -rf "${PUBLISH_DIR}/_"
+cp -r target/site/_ "${PUBLISH_DIR}/_"
 # Fix the root redirect and sitemap to match the flattened layout
 sed 's|tika/||g' target/site/index.html > "${DOCS_DIR}/index.html"
 sed 's|/docs/tika/|/docs/|g' target/site/sitemap.xml > 
"${DOCS_DIR}/sitemap.xml"
 cp target/site/404.html "${DOCS_DIR}/"
-cp target/site/search-index.js "${DOCS_DIR}/"
+# Lunr index lives next to _/ (one level above docs/), since HTML uses 
../../search-index.js
+cp target/site/search-index.js "${PUBLISH_DIR}/"

Review Comment:
   `search-index.js` used to be published under `${DOCS_DIR}/` and is now 
published at the publish root. If someone previously ran the script, a stale 
`${DOCS_DIR}/search-index.js` will be left behind, which can cause confusion 
(and potentially be picked up by older pages/links). It’s safer to remove the 
old location before copying the new index.



-- 
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]

Reply via email to