dongjoon-hyun commented on code in PR #59547:
URL: https://github.com/apache/spark/pull/59547#discussion_r4318348888
##########
docs/_layouts/global.html:
##########
@@ -196,7 +196,18 @@ <h1 class="title">{{ page.title }}</h1>
<script src="{{ rel_path_to_root }}js/vendor/jquery.min.js"></script>
<script src="{{ rel_path_to_root
}}js/vendor/bootstrap.bundle.min.js"></script>
- <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js"></script>
+ <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js">
+ // Keep the version of AnchorJS used here in sync with what's used
on the main website.
+ // See:
https://github.com/apache/spark-website/blob/asf-site/js/vendor/anchor.min.js
+ </script>
+ <script>
+ // Display anchor links when hovering over headings.
+ // See: https://www.bryanbraun.com/anchorjs/
+ anchors.options = { placement: 'right' };
+ // Note: AnchorJS 4.0+ excludes h1 from the default selector, but
many
+ // Spark doc pages use h1 for section headings, so we include it
explicitly.
+ anchors.add('h1, h2, h3, h4, h5, h6');
Review Comment:
3. Minor: moving this call out of `main.js` can briefly double the anchors
right after a release. spark.apache.org serves `docs/latest/js/*.js` with only
`Last-Modified` and `ETag`, without `Cache-Control` or `Expires`, so browsers
may keep reusing cached copies for days after `docs/latest` moves to a new
release. A returning visitor who still has the old `main.js` and the 1.1.1
`anchor.min.js` cached runs this new `anchors.add(...)` first and the old
`anchors.add()` in `main.js` second. 1.1.1 has no check for an existing anchor,
so every heading shows two icons until the cache expires. It is cosmetic and
temporary. Could we keep the call in `main.js` as in item 5, which avoids this
mixed state?
##########
licenses/LICENSE-AnchorJS.txt:
##########
@@ -1,6 +1,6 @@
The MIT License (MIT)
-Copyright (c) <year> <copyright holders>
+Copyright (c) 2023 Bryan Braun
Review Comment:
**1. The fix does not reach the copy that ships.**
`licenses-binary/LICENSE-AnchorJS.txt:3` still reads `Copyright (c) <year>
<copyright holders>`, and that directory is what users get:
`dev/make-distribution.sh:258` copies it into the binary tarball, and
`python/packaging/classic/setup.py:108` packages it as `pyspark/licenses`.
Before this PR, every file present in both `licenses/` and `licenses-binary/`
was identical; now this is the only pair that differs. Since SPARK-48487
removed `docs/js/vendor/anchor.min.js` from `LICENSE-binary`, the binary copy
is an orphan, so removing it would also work.
Separately, the upstream `LICENSE` at tag 5.0.0 says `Copyright (c) 2021
Bryan Braun` under the title `MIT License`; 2023 is the year in the
`anchor.min.js` banner. Either year is fine, but the commit message says "to
match anchorjs upstream".
Could you apply the same change to `licenses-binary/LICENSE-AnchorJS.txt`,
or remove that file?
##########
docs/_layouts/global.html:
##########
@@ -196,7 +196,18 @@ <h1 class="title">{{ page.title }}</h1>
<script src="{{ rel_path_to_root }}js/vendor/jquery.min.js"></script>
<script src="{{ rel_path_to_root
}}js/vendor/bootstrap.bundle.min.js"></script>
- <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js"></script>
+ <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js">
+ // Keep the version of AnchorJS used here in sync with what's used
on the main website.
+ // See:
https://github.com/apache/spark-website/blob/asf-site/js/vendor/anchor.min.js
+ </script>
+ <script>
+ // Display anchor links when hovering over headings.
+ // See: https://www.bryanbraun.com/anchorjs/
+ anchors.options = { placement: 'right' };
Review Comment:
5. Minor: moving these lines out of `docs/js/main.js` also takes them out of
the lint check. `dev/lint-js` runs ESLint (`eslint:recommended`, which includes
`no-undef`) on `docs/js` only, and nothing lints `docs/_layouts`. So a later
typo here, such as `anchros.add(...)`, would pass CI and silently remove the
anchors from every page, while the same typo in `main.js` fails `no-undef`
(which is why `main.js` declared `/* global $, anchors */`). A separate ready
handler in `main.js` keeps both the lint check and the isolation you get from a
separate script, because jQuery 3.7.1 runs each ready handler in its own
promise chain, so an error in one does not skip the dropdown setup in the other:
```js
$(function() {
anchors.options = { placement: 'right' };
anchors.add('h1, h2, h3, h4, h5, h6');
});
```
Could you move the config back into `main.js` this way and restore `/*
global $, anchors */`?
##########
docs/_layouts/global.html:
##########
@@ -196,7 +196,18 @@ <h1 class="title">{{ page.title }}</h1>
<script src="{{ rel_path_to_root }}js/vendor/jquery.min.js"></script>
<script src="{{ rel_path_to_root
}}js/vendor/bootstrap.bundle.min.js"></script>
- <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js"></script>
+ <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js">
Review Comment:
6. Minor: browsers ignore the body of a `<script>` that has `src`. This is
the only `<script src>` in the layout with a body (L30, L196, L197, L211, and
L213 are empty), so a later edit that puts real code here would silently never
run. The note also ships in all 248 generated pages, and the link returns 404
until apache/spark-website#700 is merged. Could you close this tag empty and
move the note into a Jekyll `{% comment %}` block, as you did in
`docs/sql-error-conditions.md` for SPARK-46935, or into a `//` line in the
inline script below? apache/spark-website#700 uses the same pattern.
##########
docs/_layouts/global.html:
##########
@@ -196,7 +196,18 @@ <h1 class="title">{{ page.title }}</h1>
<script src="{{ rel_path_to_root }}js/vendor/jquery.min.js"></script>
<script src="{{ rel_path_to_root
}}js/vendor/bootstrap.bundle.min.js"></script>
- <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js"></script>
+ <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js">
+ // Keep the version of AnchorJS used here in sync with what's used
on the main website.
+ // See:
https://github.com/apache/spark-website/blob/asf-site/js/vendor/anchor.min.js
+ </script>
+ <script>
+ // Display anchor links when hovering over headings.
+ // See: https://www.bryanbraun.com/anchorjs/
+ anchors.options = { placement: 'right' };
+ // Note: AnchorJS 4.0+ excludes h1 from the default selector, but
many
+ // Spark doc pages use h1 for section headings, so we include it
explicitly.
+ anchors.add('h1, h2, h3, h4, h5, h6');
Review Comment:
**2. This selector also anchors the layout's own headings, not just the page
content.** It matches the fixed sidebar titles from
`_includes/nav-left-wrapper-*.html` (for example `<h3><a
href="sql-programming-guide.html">Spark SQL Guide</a></h3>`) and the hero
`<h1>` on the landing page. As a result:
- On the 151 `sql*.html` pages, the sidebar title gets an anchor to
`#spark-sql-guide` on the current page, and screen readers read one more
"Anchor" link there.
- On the 7 streaming pages titled "Structured Streaming Programming Guide",
the sidebar `<h3>` comes first in DOM order, so it takes
`#structured-streaming-programming-guide`, and the real page title gets
`#structured-streaming-programming-guide-1`.
This is not a regression, because 1.1.1's default selector matched the same
elements. But since this line is being rewritten anyway, could we scope it to
the content? Both layout branches use `id="content"`, so `anchors.add('#content
h1, #content h2, #content h3, #content h4, #content h5, #content h6');` keeps
the page-title and section anchors and drops the sidebar and hero ones.
##########
docs/js/main.js:
##########
@@ -89,12 +89,6 @@ function codeTabs() {
$(function() {
codeTabs();
- // Display anchor links when hovering over headers. For documentation of the
- // configuration options, see the AnchorJS documentation.
- anchors.options = {
- placement: 'right'
- };
- anchors.add();
// Make dropdown menus in nav bars show on hover instead of click
Review Comment:
11. Nit: this is pre-existing and outside this PR. The dropdown block below
(L100-L119) calls `$dropdown.hover(...)` again on every `load` or `resize`
event at 768px or wider, without unbinding the previous handlers. jQuery's
`hover()` is just `.on("mouseenter", ...).on("mouseleave", ...)`, so the
handlers pile up until the window becomes narrower than 768px. They are
idempotent, so this only wastes work. The 768px threshold also differs from the
navbar's `navbar-expand-lg` breakpoint (992px). This dates from SPARK-30654.
Could `$dropdown.off("mouseenter mouseleave").hover(...)` go into a small
follow-up?
##########
docs/js/vendor/anchor.min.js:
##########
@@ -1,6 +1,9 @@
-/*!
- * AnchorJS - v1.1.1 - 2015-05-23
- * https://github.com/bryanbraun/anchorjs
- * Copyright (c) 2015 Bryan Braun; Licensed MIT
- */
-function AnchorJS(A){"use
strict";this.options=A||{},this._applyRemainingDefaultOptions=function(A){this.options.icon=this.options.hasOwnProperty("icon")?A.icon:"",this.options.visible=this.options.hasOwnProperty("visible")?A.visible:"hover",this.options.placement=this.options.hasOwnProperty("placement")?A.placement:"right",this.options.class=this.options.hasOwnProperty("class")?A.class:""},this._applyRemainingDefaultOptions(A),this.add=function(A){var
e,t,o,n,i,s,a,l,c,r,h,g,B,Q;if(this._applyRemainingDefaultOptions(this.options),A){if("string"!=typeof
A)throw new Error("The selector provided to AnchorJS was invalid.")}else
A="h1, h2, h3, h4, h5,
h6";if(e=document.querySelectorAll(A),0===e.length)return!1;for(this._addBaselineStyles(),t=document.querySelectorAll("[id]"),o=[].map.call(t,function(A){return
A.id}),i=0;i<e.length;i++){if(e[i].hasAttribute("id"))n=e[i].getAttribute("id");else{s=e[i].textContent,a=s.replace(/[^\w\s-]/gi,"").replace(/\s+/g,"-").replace(/-{2,}/g,"-"
).substring(0,64).replace(/^-+|-+$/gm,"").toLowerCase(),r=a,c=0;do void
0!==l&&(r=a+"-"+c),l=o.indexOf(r),c+=1;while(-1!==l);l=void
0,o.push(r),e[i].setAttribute("id",r),n=r}h=n.replace(/-/g," "),g='<a
class="anchorjs-link '+this.options.class+'" href="#'+n+'" aria-label="Anchor
link for: '+h+'"
data-anchorjs-icon="'+this.options.icon+'"></a>',B=document.createElement("div"),B.innerHTML=g,Q=B.childNodes,"always"===this.options.visible&&(Q[0].style.opacity="1"),""===this.options.icon&&(Q[0].style.fontFamily="anchorjs-icons",Q[0].style.fontStyle="normal",Q[0].style.fontVariant="normal",Q[0].style.fontWeight="normal"),"left"===this.options.placement?(Q[0].style.position="absolute",Q[0].style.marginLeft="-1em",Q[0].style.paddingRight="0.5em",e[i].insertBefore(Q[0],e[i].firstChild)):(Q[0].style.paddingLeft="0.375em",e[i].appendChild(Q[0]))}return
this},this.remove=function(A){for(var
e,t=document.querySelectorAll(A),o=0;o<t.length;o++)e=t[o].querySelector(".anchorjs-link"),e&&t[o]
.removeChild(e);return
this},this._addBaselineStyles=function(){if(null===document.head.querySelector("style.anchorjs")){var
A,e=document.createElement("style"),t=" .anchorjs-link { opacity: 0;
text-decoration: none; -webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale; }",o=" *:hover > .anchorjs-link,
.anchorjs-link:focus { opacity: 1; }",n=' @font-face { font-family:
"anchorjs-icons"; font-style: normal; font-weight: normal; src:
url(data:application/x-font-ttf;charset=utf-8;base64,AAEAAAALAIAAAwAwT1MvMg8SBTUAAAC8AAAAYGNtYXAWi9QdAAABHAAAAFRnYXNwAAAAEAAAAXAAAAAIZ2x5Zgq29TcAAAF4AAABNGhlYWQEZM3pAAACrAAAADZoaGVhBhUDxgAAAuQAAAAkaG10eASAADEAAAMIAAAAFGxvY2EAKACuAAADHAAAAAxtYXhwAAgAVwAAAygAAAAgbmFtZQ5yJ3cAAANIAAAB2nBvc3QAAwAAAAAFJAAAACAAAwJAAZAABQAAApkCzAAAAI8CmQLMAAAB6wAzAQkAAAAAAAAAAAAAAAAAAAABEAAAAAAAAAAAAAAAAAAAAABAAADpywPA/8AAQAPAAEAAAAABAAAAAAAAAAAAAAAgAAAAAAADAAAAAwAAABwAAQADAAAAHAADAAEAAAAcAAQAOAAAAAoACAACAAIAAQAg6cv//f//AAAAAAAg6cv//f//AAH/4x
Y5AAMAAQAAAAAAAAAAAAAAAQAB//8ADwABAAAAAAAAAAAAAgAANzkBAAAAAAEAAAAAAAAAAAACAAA3OQEAAAAAAQAAAAAAAAAAAAIAADc5AQAAAAACADEARAJTAsAAKwBUAAABIiYnJjQ/AT4BMzIWFxYUDwEGIicmND8BNjQnLgEjIgYPAQYUFxYUBw4BIwciJicmND8BNjIXFhQPAQYUFx4BMzI2PwE2NCcmNDc2MhcWFA8BDgEjARQGDAUtLXoWOR8fORYtLTgKGwoKCjgaGg0gEhIgDXoaGgkJBQwHdR85Fi0tOAobCgoKOBoaDSASEiANehoaCQkKGwotLXoWOR8BMwUFLYEuehYXFxYugC44CQkKGwo4GkoaDQ0NDXoaShoKGwoFBe8XFi6ALjgJCQobCjgaShoNDQ0NehpKGgobCgoKLYEuehYXAAEAAAABAACiToc1Xw889QALBAAAAAAA0XnFFgAAAADRecUWAAAAAAJTAsAAAAAIAAIAAAAAAAAAAQAAA8D/wAAABAAAAAAAAlMAAQAAAAAAAAAAAAAAAAAAAAUAAAAAAAAAAAAAAAACAAAAAoAAMQAAAAAACgAUAB4AmgABAAAABQBVAAIAAAAAAAIAAAAAAAAAAAAAAAAAAAAAAAAADgCuAAEAAAAAAAEADgAAAAEAAAAAAAIABwCfAAEAAAAAAAMADgBLAAEAAAAAAAQADgC0AAEAAAAAAAUACwAqAAEAAAAAAAYADgB1AAEAAAAAAAoAGgDeAAMAAQQJAAEAHAAOAAMAAQQJAAIADgCmAAMAAQQJAAMAHABZAAMAAQQJAAQAHADCAAMAAQQJAAUAFgA1AAMAAQQJAAYAHACDAAMAAQQJAAoANAD4YW5jaG9yanMtaWNvbnMAYQBuAGMAaABvAHIAagBzAC0AaQBjAG8AbgBzVmVyc2lvbiAxLjAAVgBlAHIAcwBpAG8AbgAgADEALgAwYW5jaG9yanMtaWN
vbnMAYQBuAGMAaABvAHIAagBzAC0AaQBjAG8AbgBzYW5jaG9yanMtaWNvbnMAYQBuAGMAaABvAHIAagBzAC0AaQBjAG8AbgBzUmVndWxhcgBSAGUAZwB1AGwAYQByYW5jaG9yanMtaWNvbnMAYQBuAGMAaABvAHIAagBzAC0AaQBjAG8AbgBzRm9udCBnZW5lcmF0ZWQgYnkgSWNvTW9vbi4ARgBvAG4AdAAgAGcAZQBuAGUAcgBhAHQAZQBkACAAYgB5ACAASQBjAG8ATQBvAG8AbgAuAAAAAwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==)
format("truetype"); }',i=" [data-anchorjs-icon]::after { content:
attr(data-anchorjs-icon);
}";e.className="anchorjs",e.appendChild(document.createTextNode("")),A=document.head.querySelector('[rel="stylesheet"],
style'),void
0===A?document.head.appendChild(e):document.head.insertBefore(e,A),e.sheet.insertRule(t,e.sheet.cssRules.length),e.sheet.insertRule(o,e.sheet.cssRules.length),e.sheet.insertRule(i,e.sheet.cssRules.length),e.sheet.insertRule(n,e.sheet.cssRules.length)}}}var
anchors=new AnchorJS;
+// @license
magnet:?xt=urn:btih:d3d9a9a6595521f9666a5e94cc830dab83b65699&dn=expat.txt Expat
+//
+// AnchorJS - v5.0.0 - 2023-01-18
+// https://www.bryanbraun.com/anchorjs/
+// Copyright (c) 2023 Bryan Braun; Licensed MIT
+//
+// @license
magnet:?xt=urn:btih:d3d9a9a6595521f9666a5e94cc830dab83b65699&dn=expat.txt Expat
+!function(A,e){"use strict";"function"==typeof
define&&define.amd?define([],e):"object"==typeof
module&&module.exports?module.exports=e():(A.AnchorJS=e(),A.anchors=new
A.AnchorJS)}(globalThis,function(){"use strict";return function(A){function
u(A){A.icon=Object.prototype.hasOwnProperty.call(A,"icon")?A.icon:"",A.visible=Object.prototype.hasOwnProperty.call(A,"visible")?A.visible:"hover",A.placement=Object.prototype.hasOwnProperty.call(A,"placement")?A.placement:"right",A.ariaLabel=Object.prototype.hasOwnProperty.call(A,"ariaLabel")?A.ariaLabel:"Anchor",A.class=Object.prototype.hasOwnProperty.call(A,"class")?A.class:"",A.base=Object.prototype.hasOwnProperty.call(A,"base")?A.base:"",A.truncate=Object.prototype.hasOwnProperty.call(A,"truncate")?Math.floor(A.truncate):64,A.titleText=Object.prototype.hasOwnProperty.call(A,"titleText")?A.titleText:""}function
d(A){var e;if("string"==typeof A||A instanceof
String)e=[].slice.call(document.querySelectorAll(A));else{if(!(Array.isArray(A)|
|A instanceof NodeList))throw new TypeError("The selector provided to AnchorJS
was invalid.");e=[].slice.call(A)}return
e}this.options=A||{},this.elements=[],u(this.options),this.add=function(A){var
e,t,o,i,n,s,a,r,l,c,h,p=[];if(u(this.options),0!==(e=d(A=A||"h2, h3, h4, h5,
h6")).length){for(null===document.head.querySelector("style.anchorjs")&&((A=document.createElement("style")).className="anchorjs",A.appendChild(document.createTextNode("")),void
0===(h=document.head.querySelector('[rel="stylesheet"],style'))?document.head.appendChild(A):document.head.insertBefore(A,h),A.sheet.insertRule(".anchorjs-link{opacity:0;text-decoration:none;-webkit-font-smoothing:antialiased;-moz-osx-font-smoothing:grayscale}",A.sheet.cssRules.length),A.sheet.insertRule(":hover>.anchorjs-link,.anchorjs-link:focus{opacity:1}",A.sheet.cssRules.length),A.sheet.insertRule("[data-anchorjs-icon]::after{content:attr(data-anchorjs-icon)}",A.sheet.cssRules.length),A.sheet.insertRule('@font-face{font-family:ancho
rjs-icons;src:url(data:n/a;base64,AAEAAAALAIAAAwAwT1MvMg8yG2cAAAE4AAAAYGNtYXDp3gC3AAABpAAAAExnYXNwAAAAEAAAA9wAAAAIZ2x5ZlQCcfwAAAH4AAABCGhlYWQHFvHyAAAAvAAAADZoaGVhBnACFwAAAPQAAAAkaG10eASAADEAAAGYAAAADGxvY2EACACEAAAB8AAAAAhtYXhwAAYAVwAAARgAAAAgbmFtZQGOH9cAAAMAAAAAunBvc3QAAwAAAAADvAAAACAAAQAAAAEAAHzE2p9fDzz1AAkEAAAAAADRecUWAAAAANQA6R8AAAAAAoACwAAAAAgAAgAAAAAAAAABAAADwP/AAAACgAAA/9MCrQABAAAAAAAAAAAAAAAAAAAAAwABAAAAAwBVAAIAAAAAAAIAAAAAAAAAAAAAAAAAAAAAAAMCQAGQAAUAAAKZAswAAACPApkCzAAAAesAMwEJAAAAAAAAAAAAAAAAAAAAARAAAAAAAAAAAAAAAAAAAAAAQAAg//0DwP/AAEADwABAAAAAAQAAAAAAAAAAAAAAIAAAAAAAAAIAAAACgAAxAAAAAwAAAAMAAAAcAAEAAwAAABwAAwABAAAAHAAEADAAAAAIAAgAAgAAACDpy//9//8AAAAg6cv//f///+EWNwADAAEAAAAAAAAAAAAAAAAACACEAAEAAAAAAAAAAAAAAAAxAAACAAQARAKAAsAAKwBUAAABIiYnJjQ3NzY2MzIWFxYUBwcGIicmNDc3NjQnJiYjIgYHBwYUFxYUBwYGIwciJicmNDc3NjIXFhQHBwYUFxYWMzI2Nzc2NCcmNDc2MhcWFAcHBgYjARQGDAUtLXoWOR8fORYtLTgKGwoKCjgaGg0gEhIgDXoaGgkJBQwHdR85Fi0tOAobCgoKOBoaDSASEiANehoaCQkKGwotLXoWOR8BMwUFLYEuehYXFxYugC44CQkKGwo4GkoaDQ0
NDXoaShoKGwoFBe8XFi6ALjgJCQobCjgaShoNDQ0NehpKGgobCgoKLYEuehYXAAAADACWAAEAAAAAAAEACAAAAAEAAAAAAAIAAwAIAAEAAAAAAAMACAAAAAEAAAAAAAQACAAAAAEAAAAAAAUAAQALAAEAAAAAAAYACAAAAAMAAQQJAAEAEAAMAAMAAQQJAAIABgAcAAMAAQQJAAMAEAAMAAMAAQQJAAQAEAAMAAMAAQQJAAUAAgAiAAMAAQQJAAYAEAAMYW5jaG9yanM0MDBAAGEAbgBjAGgAbwByAGoAcwA0ADAAMABAAAAAAwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAH//wAP)
format("truetype")}',A.sheet.cssRules.length)),h=document.querySelectorAll("[id]"),t=[].map.call(h,function(A){return
A.id}),i=0;i<e.length;i++)if(this.hasAnchorJSLink(e[i]))p.push(i);else{if(e[i].hasAttribute("id"))o=e[i].getAttribute("id");else
if(e[i].hasAttribute("data-anchor-id"))o=e[i].getAttribute("data-anchor-id");else{for(r=a=this.urlify(e[i].textContent),s=0;n=t.indexOf(r=void
0!==n?a+"-"+s:r),s+=1,-1!==n;);n=void
0,t.push(r),e[i].setAttribute("id",r),o=r}(l=document.createElement("a")).className="anchorjs-link
"+this.options.class,l.setAttribute("aria-label",this.options.ariaLabel),l.setAttribute("data-anchorjs-
icon",this.options.icon),this.options.titleText&&(l.title=this.options.titleText),c=document.querySelector("base")?window.location.pathname+window.location.search:"",c=this.options.base||c,l.href=c+"#"+o,"always"===this.options.visible&&(l.style.opacity="1"),""===this.options.icon&&(l.style.font="1em/1
anchorjs-icons","left"===this.options.placement)&&(l.style.lineHeight="inherit"),"left"===this.options.placement?(l.style.position="absolute",l.style.marginLeft="-1.25em",l.style.paddingRight=".25em",l.style.paddingLeft=".25em",e[i].insertBefore(l,e[i].firstChild)):(l.style.marginLeft=".1875em",l.style.paddingRight=".1875em",l.style.paddingLeft=".1875em",e[i].appendChild(l))}for(i=0;i<p.length;i++)e.splice(p[i]-i,1);this.elements=this.elements.concat(e)}return
this},this.remove=function(A){for(var
e,t,o=d(A),i=0;i<o.length;i++)(t=o[i].querySelector(".anchorjs-link"))&&(-1!==(e=this.elements.indexOf(o[i]))&&this.elements.splice(e,1),o[i].removeChild(t));return
this},this.removeAll=f
unction(){this.remove(this.elements)},this.urlify=function(A){var
e=document.createElement("textarea");return
e.innerHTML=A,A=e.value,this.options.truncate||u(this.options),A.trim().replace(/'/gi,"").replace(/[&
+$,:;=?@"#{}|^~[`%!'<>\]./()*\\\n\t\b\v\u00A0]/g,"-").replace(/-{2,}/g,"-").substring(0,this.options.truncate).replace(/^-+|-+$/gm,"").toLowerCase()},this.hasAnchorJSLink=function(A){var
e=A.firstChild&&-1<(" "+A.firstChild.className+" ").indexOf(" anchorjs-link
"),A=A.lastChild&&-1<(" "+A.lastChild.className+" ").indexOf(" anchorjs-link
");return e||A||!1}}});
Review Comment:
4. Minor: 5.0.0 passes a bare `globalThis` to its UMD wrapper here, unlike
1.1.1 (plain ES5) and our Bootstrap 5.0.2 bundle (which checks `typeof
globalThis`). On Chrome < 71, Firefox < 65 (including ESR 60), and Safari <
12.1, which Bootstrap 5.0.2's `.browserslistrc` still lists, this line throws
`ReferenceError: globalThis is not defined`. Then `global.html:206` throws
`anchors is not defined`, and those browsers get no heading anchors at all. The
rest of the page still works, because the config runs in its own `<script>`.
These browsers have been out of support for years, so accepting this is fine
with me. Could you mention it in the PR description, or add `if (typeof
globalThis === 'undefined') { window.globalThis = window; }` before this script
if you want to keep them working?
##########
docs/_layouts/global.html:
##########
@@ -196,7 +196,18 @@ <h1 class="title">{{ page.title }}</h1>
<script src="{{ rel_path_to_root }}js/vendor/jquery.min.js"></script>
<script src="{{ rel_path_to_root
}}js/vendor/bootstrap.bundle.min.js"></script>
- <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js"></script>
+ <script src="{{ rel_path_to_root }}js/vendor/anchor.min.js">
+ // Keep the version of AnchorJS used here in sync with what's used
on the main website.
+ // See:
https://github.com/apache/spark-website/blob/asf-site/js/vendor/anchor.min.js
+ </script>
+ <script>
+ // Display anchor links when hovering over headings.
+ // See: https://www.bryanbraun.com/anchorjs/
+ anchors.options = { placement: 'right' };
Review Comment:
9. Nit: `placement: 'right'` is already the default in both 1.1.1 and 5.0.0,
and `add()` re-applies the defaults, so this line has no effect. It has been a
no-op since SPARK-12324 changed it from `'left'` in 2015. Keeping it as an
explicit choice is fine too. Could you drop it, here and in
apache/spark-website#700, if you prefer the shorter form?
##########
docs/js/vendor/anchor.min.js:
##########
@@ -1,6 +1,9 @@
-/*!
- * AnchorJS - v1.1.1 - 2015-05-23
- * https://github.com/bryanbraun/anchorjs
- * Copyright (c) 2015 Bryan Braun; Licensed MIT
- */
-function AnchorJS(A){"use
strict";this.options=A||{},this._applyRemainingDefaultOptions=function(A){this.options.icon=this.options.hasOwnProperty("icon")?A.icon:"",this.options.visible=this.options.hasOwnProperty("visible")?A.visible:"hover",this.options.placement=this.options.hasOwnProperty("placement")?A.placement:"right",this.options.class=this.options.hasOwnProperty("class")?A.class:""},this._applyRemainingDefaultOptions(A),this.add=function(A){var
e,t,o,n,i,s,a,l,c,r,h,g,B,Q;if(this._applyRemainingDefaultOptions(this.options),A){if("string"!=typeof
A)throw new Error("The selector provided to AnchorJS was invalid.")}else
A="h1, h2, h3, h4, h5,
h6";if(e=document.querySelectorAll(A),0===e.length)return!1;for(this._addBaselineStyles(),t=document.querySelectorAll("[id]"),o=[].map.call(t,function(A){return
A.id}),i=0;i<e.length;i++){if(e[i].hasAttribute("id"))n=e[i].getAttribute("id");else{s=e[i].textContent,a=s.replace(/[^\w\s-]/gi,"").replace(/\s+/g,"-").replace(/-{2,}/g,"-"
).substring(0,64).replace(/^-+|-+$/gm,"").toLowerCase(),r=a,c=0;do void
0!==l&&(r=a+"-"+c),l=o.indexOf(r),c+=1;while(-1!==l);l=void
0,o.push(r),e[i].setAttribute("id",r),n=r}h=n.replace(/-/g," "),g='<a
class="anchorjs-link '+this.options.class+'" href="#'+n+'" aria-label="Anchor
link for: '+h+'"
data-anchorjs-icon="'+this.options.icon+'"></a>',B=document.createElement("div"),B.innerHTML=g,Q=B.childNodes,"always"===this.options.visible&&(Q[0].style.opacity="1"),""===this.options.icon&&(Q[0].style.fontFamily="anchorjs-icons",Q[0].style.fontStyle="normal",Q[0].style.fontVariant="normal",Q[0].style.fontWeight="normal"),"left"===this.options.placement?(Q[0].style.position="absolute",Q[0].style.marginLeft="-1em",Q[0].style.paddingRight="0.5em",e[i].insertBefore(Q[0],e[i].firstChild)):(Q[0].style.paddingLeft="0.375em",e[i].appendChild(Q[0]))}return
this},this.remove=function(A){for(var
e,t=document.querySelectorAll(A),o=0;o<t.length;o++)e=t[o].querySelector(".anchorjs-link"),e&&t[o]
.removeChild(e);return
this},this._addBaselineStyles=function(){if(null===document.head.querySelector("style.anchorjs")){var
A,e=document.createElement("style"),t=" .anchorjs-link { opacity: 0;
text-decoration: none; -webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale; }",o=" *:hover > .anchorjs-link,
.anchorjs-link:focus { opacity: 1; }",n=' @font-face { font-family:
"anchorjs-icons"; font-style: normal; font-weight: normal; src:
url(data:application/x-font-ttf;charset=utf-8;base64,AAEAAAALAIAAAwAwT1MvMg8SBTUAAAC8AAAAYGNtYXAWi9QdAAABHAAAAFRnYXNwAAAAEAAAAXAAAAAIZ2x5Zgq29TcAAAF4AAABNGhlYWQEZM3pAAACrAAAADZoaGVhBhUDxgAAAuQAAAAkaG10eASAADEAAAMIAAAAFGxvY2EAKACuAAADHAAAAAxtYXhwAAgAVwAAAygAAAAgbmFtZQ5yJ3cAAANIAAAB2nBvc3QAAwAAAAAFJAAAACAAAwJAAZAABQAAApkCzAAAAI8CmQLMAAAB6wAzAQkAAAAAAAAAAAAAAAAAAAABEAAAAAAAAAAAAAAAAAAAAABAAADpywPA/8AAQAPAAEAAAAABAAAAAAAAAAAAAAAgAAAAAAADAAAAAwAAABwAAQADAAAAHAADAAEAAAAcAAQAOAAAAAoACAACAAIAAQAg6cv//f//AAAAAAAg6cv//f//AAH/4x
Y5AAMAAQAAAAAAAAAAAAAAAQAB//8ADwABAAAAAAAAAAAAAgAANzkBAAAAAAEAAAAAAAAAAAACAAA3OQEAAAAAAQAAAAAAAAAAAAIAADc5AQAAAAACADEARAJTAsAAKwBUAAABIiYnJjQ/AT4BMzIWFxYUDwEGIicmND8BNjQnLgEjIgYPAQYUFxYUBw4BIwciJicmND8BNjIXFhQPAQYUFx4BMzI2PwE2NCcmNDc2MhcWFA8BDgEjARQGDAUtLXoWOR8fORYtLTgKGwoKCjgaGg0gEhIgDXoaGgkJBQwHdR85Fi0tOAobCgoKOBoaDSASEiANehoaCQkKGwotLXoWOR8BMwUFLYEuehYXFxYugC44CQkKGwo4GkoaDQ0NDXoaShoKGwoFBe8XFi6ALjgJCQobCjgaShoNDQ0NehpKGgobCgoKLYEuehYXAAEAAAABAACiToc1Xw889QALBAAAAAAA0XnFFgAAAADRecUWAAAAAAJTAsAAAAAIAAIAAAAAAAAAAQAAA8D/wAAABAAAAAAAAlMAAQAAAAAAAAAAAAAAAAAAAAUAAAAAAAAAAAAAAAACAAAAAoAAMQAAAAAACgAUAB4AmgABAAAABQBVAAIAAAAAAAIAAAAAAAAAAAAAAAAAAAAAAAAADgCuAAEAAAAAAAEADgAAAAEAAAAAAAIABwCfAAEAAAAAAAMADgBLAAEAAAAAAAQADgC0AAEAAAAAAAUACwAqAAEAAAAAAAYADgB1AAEAAAAAAAoAGgDeAAMAAQQJAAEAHAAOAAMAAQQJAAIADgCmAAMAAQQJAAMAHABZAAMAAQQJAAQAHADCAAMAAQQJAAUAFgA1AAMAAQQJAAYAHACDAAMAAQQJAAoANAD4YW5jaG9yanMtaWNvbnMAYQBuAGMAaABvAHIAagBzAC0AaQBjAG8AbgBzVmVyc2lvbiAxLjAAVgBlAHIAcwBpAG8AbgAgADEALgAwYW5jaG9yanMtaWN
vbnMAYQBuAGMAaABvAHIAagBzAC0AaQBjAG8AbgBzYW5jaG9yanMtaWNvbnMAYQBuAGMAaABvAHIAagBzAC0AaQBjAG8AbgBzUmVndWxhcgBSAGUAZwB1AGwAYQByYW5jaG9yanMtaWNvbnMAYQBuAGMAaABvAHIAagBzAC0AaQBjAG8AbgBzRm9udCBnZW5lcmF0ZWQgYnkgSWNvTW9vbi4ARgBvAG4AdAAgAGcAZQBuAGUAcgBhAHQAZQBkACAAYgB5ACAASQBjAG8ATQBvAG8AbgAuAAAAAwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==)
format("truetype"); }',i=" [data-anchorjs-icon]::after { content:
attr(data-anchorjs-icon);
}";e.className="anchorjs",e.appendChild(document.createTextNode("")),A=document.head.querySelector('[rel="stylesheet"],
style'),void
0===A?document.head.appendChild(e):document.head.insertBefore(e,A),e.sheet.insertRule(t,e.sheet.cssRules.length),e.sheet.insertRule(o,e.sheet.cssRules.length),e.sheet.insertRule(i,e.sheet.cssRules.length),e.sheet.insertRule(n,e.sheet.cssRules.length)}}}var
anchors=new AnchorJS;
+// @license
magnet:?xt=urn:btih:d3d9a9a6595521f9666a5e94cc830dab83b65699&dn=expat.txt Expat
+//
+// AnchorJS - v5.0.0 - 2023-01-18
+// https://www.bryanbraun.com/anchorjs/
+// Copyright (c) 2023 Bryan Braun; Licensed MIT
+//
+// @license
magnet:?xt=urn:btih:d3d9a9a6595521f9666a5e94cc830dab83b65699&dn=expat.txt Expat
+!function(A,e){"use strict";"function"==typeof
define&&define.amd?define([],e):"object"==typeof
module&&module.exports?module.exports=e():(A.AnchorJS=e(),A.anchors=new
A.AnchorJS)}(globalThis,function(){"use strict";return function(A){function
u(A){A.icon=Object.prototype.hasOwnProperty.call(A,"icon")?A.icon:"",A.visible=Object.prototype.hasOwnProperty.call(A,"visible")?A.visible:"hover",A.placement=Object.prototype.hasOwnProperty.call(A,"placement")?A.placement:"right",A.ariaLabel=Object.prototype.hasOwnProperty.call(A,"ariaLabel")?A.ariaLabel:"Anchor",A.class=Object.prototype.hasOwnProperty.call(A,"class")?A.class:"",A.base=Object.prototype.hasOwnProperty.call(A,"base")?A.base:"",A.truncate=Object.prototype.hasOwnProperty.call(A,"truncate")?Math.floor(A.truncate):64,A.titleText=Object.prototype.hasOwnProperty.call(A,"titleText")?A.titleText:""}function
d(A){var e;if("string"==typeof A||A instanceof
String)e=[].slice.call(document.querySelectorAll(A));else{if(!(Array.isArray(A)|
|A instanceof NodeList))throw new TypeError("The selector provided to AnchorJS
was invalid.");e=[].slice.call(A)}return
e}this.options=A||{},this.elements=[],u(this.options),this.add=function(A){var
e,t,o,i,n,s,a,r,l,c,h,p=[];if(u(this.options),0!==(e=d(A=A||"h2, h3, h4, h5,
h6")).length){for(null===document.head.querySelector("style.anchorjs")&&((A=document.createElement("style")).className="anchorjs",A.appendChild(document.createTextNode("")),void
0===(h=document.head.querySelector('[rel="stylesheet"],style'))?document.head.appendChild(A):document.head.insertBefore(A,h),A.sheet.insertRule(".anchorjs-link{opacity:0;text-decoration:none;-webkit-font-smoothing:antialiased;-moz-osx-font-smoothing:grayscale}",A.sheet.cssRules.length),A.sheet.insertRule(":hover>.anchorjs-link,.anchorjs-link:focus{opacity:1}",A.sheet.cssRules.length),A.sheet.insertRule("[data-anchorjs-icon]::after{content:attr(data-anchorjs-icon)}",A.sheet.cssRules.length),A.sheet.insertRule('@font-face{font-family:ancho
rjs-icons;src:url(data:n/a;base64,AAEAAAALAIAAAwAwT1MvMg8yG2cAAAE4AAAAYGNtYXDp3gC3AAABpAAAAExnYXNwAAAAEAAAA9wAAAAIZ2x5ZlQCcfwAAAH4AAABCGhlYWQHFvHyAAAAvAAAADZoaGVhBnACFwAAAPQAAAAkaG10eASAADEAAAGYAAAADGxvY2EACACEAAAB8AAAAAhtYXhwAAYAVwAAARgAAAAgbmFtZQGOH9cAAAMAAAAAunBvc3QAAwAAAAADvAAAACAAAQAAAAEAAHzE2p9fDzz1AAkEAAAAAADRecUWAAAAANQA6R8AAAAAAoACwAAAAAgAAgAAAAAAAAABAAADwP/AAAACgAAA/9MCrQABAAAAAAAAAAAAAAAAAAAAAwABAAAAAwBVAAIAAAAAAAIAAAAAAAAAAAAAAAAAAAAAAAMCQAGQAAUAAAKZAswAAACPApkCzAAAAesAMwEJAAAAAAAAAAAAAAAAAAAAARAAAAAAAAAAAAAAAAAAAAAAQAAg//0DwP/AAEADwABAAAAAAQAAAAAAAAAAAAAAIAAAAAAAAAIAAAACgAAxAAAAAwAAAAMAAAAcAAEAAwAAABwAAwABAAAAHAAEADAAAAAIAAgAAgAAACDpy//9//8AAAAg6cv//f///+EWNwADAAEAAAAAAAAAAAAAAAAACACEAAEAAAAAAAAAAAAAAAAxAAACAAQARAKAAsAAKwBUAAABIiYnJjQ3NzY2MzIWFxYUBwcGIicmNDc3NjQnJiYjIgYHBwYUFxYUBwYGIwciJicmNDc3NjIXFhQHBwYUFxYWMzI2Nzc2NCcmNDc2MhcWFAcHBgYjARQGDAUtLXoWOR8fORYtLTgKGwoKCjgaGg0gEhIgDXoaGgkJBQwHdR85Fi0tOAobCgoKOBoaDSASEiANehoaCQkKGwotLXoWOR8BMwUFLYEuehYXFxYugC44CQkKGwo4GkoaDQ0
NDXoaShoKGwoFBe8XFi6ALjgJCQobCjgaShoNDQ0NehpKGgobCgoKLYEuehYXAAAADACWAAEAAAAAAAEACAAAAAEAAAAAAAIAAwAIAAEAAAAAAAMACAAAAAEAAAAAAAQACAAAAAEAAAAAAAUAAQALAAEAAAAAAAYACAAAAAMAAQQJAAEAEAAMAAMAAQQJAAIABgAcAAMAAQQJAAMAEAAMAAMAAQQJAAQAEAAMAAMAAQQJAAUAAgAiAAMAAQQJAAYAEAAMYW5jaG9yanM0MDBAAGEAbgBjAGgAbwByAGoAcwA0ADAAMABAAAAAAwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAH//wAP)
format("truetype")}',A.sheet.cssRules.length)),h=document.querySelectorAll("[id]"),t=[].map.call(h,function(A){return
A.id}),i=0;i<e.length;i++)if(this.hasAnchorJSLink(e[i]))p.push(i);else{if(e[i].hasAttribute("id"))o=e[i].getAttribute("id");else
if(e[i].hasAttribute("data-anchor-id"))o=e[i].getAttribute("data-anchor-id");else{for(r=a=this.urlify(e[i].textContent),s=0;n=t.indexOf(r=void
0!==n?a+"-"+s:r),s+=1,-1!==n;);n=void
0,t.push(r),e[i].setAttribute("id",r),o=r}(l=document.createElement("a")).className="anchorjs-link
"+this.options.class,l.setAttribute("aria-label",this.options.ariaLabel),l.setAttribute("data-anchorjs-
icon",this.options.icon),this.options.titleText&&(l.title=this.options.titleText),c=document.querySelector("base")?window.location.pathname+window.location.search:"",c=this.options.base||c,l.href=c+"#"+o,"always"===this.options.visible&&(l.style.opacity="1"),""===this.options.icon&&(l.style.font="1em/1
anchorjs-icons","left"===this.options.placement)&&(l.style.lineHeight="inherit"),"left"===this.options.placement?(l.style.position="absolute",l.style.marginLeft="-1.25em",l.style.paddingRight=".25em",l.style.paddingLeft=".25em",e[i].insertBefore(l,e[i].firstChild)):(l.style.marginLeft=".1875em",l.style.paddingRight=".1875em",l.style.paddingLeft=".1875em",e[i].appendChild(l))}for(i=0;i<p.length;i++)e.splice(p[i]-i,1);this.elements=this.elements.concat(e)}return
this},this.remove=function(A){for(var
e,t,o=d(A),i=0;i<o.length;i++)(t=o[i].querySelector(".anchorjs-link"))&&(-1!==(e=this.elements.indexOf(o[i]))&&this.elements.splice(e,1),o[i].removeChild(t));return
this},this.removeAll=f
unction(){this.remove(this.elements)},this.urlify=function(A){var
e=document.createElement("textarea");return
e.innerHTML=A,A=e.value,this.options.truncate||u(this.options),A.trim().replace(/'/gi,"").replace(/[&
+$,:;=?@"#{}|^~[`%!'<>\]./()*\\\n\t\b\v\u00A0]/g,"-").replace(/-{2,}/g,"-").substring(0,this.options.truncate).replace(/^-+|-+$/gm,"").toLowerCase()},this.hasAnchorJSLink=function(A){var
e=A.firstChild&&-1<(" "+A.firstChild.className+" ").indexOf(" anchorjs-link
"),A=A.lastChild&&-1<(" "+A.lastChild.className+" ").indexOf(" anchorjs-link
");return e||A||!1}}});
Review Comment:
10. Nit: FYI, 5.0.0's `add()` calls `document.querySelector('base')` inside
its per-heading loop (`anchor.js` L140 at tag 5.0.0). Our pages have no
`<base>`, so each heading triggers a scan of the whole document. On the largest
page, `sql-ref-functions-builtin.html` (about 1.5 MB, ~48K elements, 48
headings), that is about 48 extra scans during parsing. I have not measured it,
but it should be a few milliseconds. Nothing to change here, since this file
should stay identical to upstream. Could it be worth an upstream issue to move
that query out of the loop?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
