Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (14)
Tip You can disable the changed files summary in the walkthrough.Disable the ✨ Finishing Touches
🧪 Generate unit tests✅ Copyable Unit Test edits generated.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
2 similar comments
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 64
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
csv-test/dotpipe.js (10)
534-536: Columns: “autopx” bug for height; treat numeric vs keyword units correctly.
height.includes('px') ? height :${height}px`` makes"auto" → "autopx". Same for widths that are plain numbers.- columnsContainer.style.height = height.includes('px') ? height : `${height}px`; - columnsContainer.style.width = width.includes('%') || width.includes('px') ? width : `${width}px`; + const toCssLen = v => /^\d+$/.test(v) ? `${v}px` : v; // numbers → px, otherwise leave as-is (%, px, auto, etc.) + columnsContainer.style.height = toCssLen(height); + columnsContainer.style.width = toCssLen(width);
653-671: Stop using deprecated DOMNodeInserted; use MutationObserver to detect insertions.
DOMNodeInsertedis deprecated and expensive. Replace with a scoped observer on the target column.- // Set up event listener to handle when content is loaded - document.addEventListener('DOMNodeInserted', function handler(event) { - if (event.target.parentNode && event.target.parentNode.id === column.id) { - // Content has been inserted into the column - setTimeout(() => { - // Remove loading indicator if it exists - const loadingIndicator = column.querySelector('.column-loading'); - if (loadingIndicator) { - loadingIndicator.remove(); - } - - // Dispatch content loaded event - dispatchColumnContentLoaded(column); - - // Remove event listener - document.removeEventListener('DOMNodeInserted', handler); - }, 100); - } - }); + // Observe insertions into this column only + const observer = new MutationObserver(() => { + // Remove loading indicator if it exists + const loadingIndicator = column.querySelector('.column-loading'); + if (loadingIndicator) loadingIndicator.remove(); + dispatchColumnContentLoaded(column); + observer.disconnect(); + }); + observer.observe(column, { childList: true, subtree: true });
1122-1149: Same deprecated DOMNodeInserted usage in refreshHtmlContent; migrate to MutationObserver.Mirror the approach used for columns to keep behavior stable and efficient.
- // Set up event listener to handle when content is loaded - document.addEventListener('DOMNodeInserted', function handler(event) { - if (event.target.parentNode && event.target.parentNode.id === targetElement.id) { - // Content has been inserted into the target element - setTimeout(() => { - // Remove loading indicator if it exists - const loadingIndicator = targetElement.querySelector('.refresh-loading'); - if (loadingIndicator) { - loadingIndicator.remove(); - } - // Dispatch refresh completed event - dispatchRefreshEvent(targetElement, 'refreshCompleted', { - targetId: targetElement.id, - pageUrl: htmlUrl, - success: true - }); - // Remove event listener - document.removeEventListener('DOMNodeInserted', handler); - // Remove the pipe element - if (document.body.contains(pipeElement)) { - document.body.removeChild(pipeElement); - } - }, 100); - } - }); + // Observe insertions into the target only + const observer = new MutationObserver(() => { + const loadingIndicator = targetElement.querySelector('.refresh-loading'); + if (loadingIndicator) loadingIndicator.remove(); + dispatchRefreshEvent(targetElement, 'refreshCompleted', { + targetId: targetElement.id, + pageUrl: htmlUrl, + success: true + }); + observer.disconnect(); + if (document.body.contains(pipeElement)) document.body.removeChild(pipeElement); + }); + observer.observe(targetElement, { childList: true, subtree: true });
4650-4655: modalaHead: wrong variable used when creating CSS links.Inside the
cssbranch, you loopoptsArray.forEach((e, f) => { … cssvar.href = v … }), but should usee.- optsArray.forEach((e, f) => { - var cssvar = document.createElement("link"); - cssvar.href = v; - cssvar.rel = "stylesheet"; - document.head.appendChild(cssvar); - }); + optsArray.forEach((e) => { + const cssvar = document.createElement("link"); + cssvar.href = e; + cssvar.rel = "stylesheet"; + document.head.appendChild(cssvar); + });
4889-4902: Typo:values['controls']should bevalue['controls'].This makes audio/video controls logic dead and will throw if
valuesis undefined.- gth.controls = (values['controls'] != undefined && value['controls'] != false) ? true : false; + gth.controls = (value['controls'] != undefined && value['controls'] != false); @@ - gth.controls = (values['controls'] != undefined && value['controls'] != false) ? true : false; + gth.controls = (value['controls'] != undefined && value['controls'] != false);
5058-5067: Bug: using bitwise OR in string comparisons ('audio' | 'video') breaks carousel media handling.
'audio' | 'video'evaluates to a number, so those branches never match as intended.- if (elem.getAttribute("type").toLowerCase() == ('audio' | 'video')) + const t = elem.getAttribute("type").toLowerCase(); + if (t === 'audio' || t === 'video') newClone = document.createElement(elem.getAttribute("source")); @@ - else if (elem.getAttribute("type").toLowerCase() == ('php' | 'html')) { + else if (t === 'php' || t === 'html') {Apply the same fix in
shiftFilesRight.Also applies to: 5107-5116
5424-5431: Bug in delete attribute handler: references undefinedg.The code splits into
[id, name]but checksg[0]/g[1].- js.split(";").forEach((e, f) => { - var [id, name] = e.split(":"); - if (g[0] != '' && g[0] != undefined && g[1] != undefined) { - document.getElementById(id).removeAttribute(name); - } - }); + js.split(";").forEach((e) => { + const [id, name] = e.split(":"); + if (id && name) { + const node = document.getElementById(id); + if (node) node.removeAttribute(name); + } + });
192-208: Implement nonces on all dynamically created<style>and<script>elementsThe client-side CSP meta you inject (lines 192–208) will only apply to tags bearing a matching nonce—and browsers generally honor only the first CSP they encounter, so late-added metas can be ignored. Any styles or scripts you add after DOMContentLoaded without
nonce=PAGE_NONCEwill be blocked.Please update the following in csv-test/dotpipe.js (and its siblings) to carry the nonce:
- addColumnsStyles (around line 719)
- addRefreshStyles (around line 1227)
- Inline login-tabs script injection (around line 2758)
- Dynamic external scripts via
optsArray.forEach(around lines 4662 and 4937)Example patch:
@@ function addColumnsStyles() { - const style = document.createElement('style'); + const style = document.createElement('style'); + if (typeof PAGE_NONCE !== 'undefined') style.nonce = PAGE_NONCE; @@ function addRefreshStyles() { - const style = document.createElement('style'); + const style = document.createElement('style'); + if (typeof PAGE_NONCE !== 'undefined') style.nonce = PAGE_NONCE; @@ inline login script injection - const script = document.createElement('script'); + const script = document.createElement('script'); + if (typeof PAGE_NONCE !== 'undefined') script.nonce = PAGE_NONCE; @@ dynamic external script loader (optsArray) - const js = document.createElement("script"); + const js = document.createElement("script"); + if (typeof PAGE_NONCE !== 'undefined') js.nonce = PAGE_NONCE;Consider migrating CSP to an HTTP response header (preferred) and ensuring every dynamic tag carries the nonce.
5539-5555: Refactor AJAX plumbing in csv-test/dotpipe.jsThe existing
setAJAXOpts/navigatepairing mixes object and Map APIs, applies invalid header defaults, and duplicates fetch-like behavior via XHR. A targeted refactor will simplify maintenance and eliminate insecure or unused code.• In setAJAXOpts (lines 5536–5555),
optsis accessed as both an object (opts["method"]) and a Map (opts.set(…)), yet you always pass a Map. Accessingopts["mode"]/opts["cred"]returnsundefined, so you fallback to strings like'{"Access-Control-Allow-Origin":"*"}', which aren’t valid Fetch/XHR options and may be misleading.
• In navigate (lines 5619–5627), you build aRequestinstance vianew Request(elem_qstring)that is never used. You then callrawFile.open(…)on an XHR, ignoring all previously configured Map entries exceptmethod.
• You encode the entire query string withencodeURI, which doesn’t properly escape parameter values (useencodeURIComponentper-param orURLSearchParams).
• The ad-hoc merge of fetch-style option setup and legacy XHR flow duplicates effort and risks inconsistent behavior (CORS, credentials, redirects).➡️ Suggest refactor:
- Replace
setAJAXOpts+ XHR innavigatewith a singlefetch()call.- Build the final URL using
URLSearchParams(form data + query).- Pass a proper options object to
fetch(method, headers Map →{ headers: … }, mode, cache, credentials, redirect, referrer).- Preserve your “render strategies” (
json,text-html,plain-text,tree-view,modala) by branching on the XHR/fetch response instead of swapping transport.Happy to help draft a fetch-based
navigatethat covers all branches.
4683-4684: FixmodalaHeadreturn value and its usagesThe helper
modalaHead(value)is intended to build and return anHTMLElement(per its JSDoc) but currently ends withreturn;, causing it to returnundefined. Call sites then do:const tmp = modalaHead(data, temp, root, id); document.head.appendChild(tmp);which tries to append
undefined—a runtime error.To resolve, you must either return the constructed element (and update nested calls to attach children) or stop treating its return value as an appendable node.
• In
csv-test/dotpipe.jsat the end ofmodalaHead(around line 4660), change:- return; + return temp;• Also inside the
.forEachwhere you recurse for nested objects (around line 4630), attach returned children totemp:- if (v instanceof Object) { - modalaHead(v); - } + if (v instanceof Object) { + const child = modalaHead(v); + if (child) temp.appendChild(child); + }• At each call site that appends the return value (e.g. lines 4671 and any others), ensure you pass only the
valueargument and then append the returned node:- const tmp = modalaHead(data, temp, root, id); - document.head.appendChild(tmp); + const tmp = modalaHead(data); + document.head.appendChild(tmp);Alternatively, if you prefer
modalaHeadto handle its own appends, remove theappendChild(tmp)calls and move those responsibilities insidemodalaHead.cart/dotpipe.js (1)
340-349: Order-of-operations: register() runs after initial pipes() calls
- domContentLoad() calls pipes() on tags before calling dotPipe.register(). If those elements also have inline, runInline runs before registration and does nothing. Register first.
Move register to the top of domContentLoad and keep one call:
@@ function domContentLoad (again = false) { - doc_set = document.getElementsByTagName("pipe"); + // Register inline elements before any initial processing + dotPipe.register(); + doc_set = document.getElementsByTagName("pipe"); @@ - processColumnsTags(); - dotPipe.register(); + processColumnsTags();Also applies to: 382-383
🧹 Nitpick comments (55)
inline/index.html (3)
18-20: Avoid double registration; dotPipe already registers on DOMContentLoaded.addons/dotpipe.js calls register() on DOMContentLoaded via domContentLoad(). Calling it here redoes the work and can surprise users if they mutate dpVars before/after registration.
Apply this diff to remove the redundant call:
-// Initialize dotPipe -dotPipe.register();If you keep it for demo purposes, at least leave a note that the library also registers automatically.
26-28: Guard dpVars access until after registration.If you remove the manual register() above (recommended), this line can run before the matrix is populated.
Apply one of these:
Option A (run after DOM ready):
-// You can update dpVars anytime — does not auto-trigger -dotPipe.matrix['example'].dpVars.greeting = "Goodbye!"; +// You can update dpVars anytime — does not auto-trigger +document.addEventListener('DOMContentLoaded', () => { + dotPipe.matrix['example'] && (dotPipe.matrix['example'].dpVars.greeting = "Goodbye!"); +});Option B (explicitly after first run):
document.getElementById('run').addEventListener('click', () => { dotPipe.runInline('example'); + // mutate after a first run + dotPipe.matrix['example'] && (dotPipe.matrix['example'].dpVars.greeting = "Goodbye!"); });
1-5: Example pipeline is fine; consider dropping “!” in $…:var for clarity.$outputDiv:!greeting works, but the current parser also resolves $outputDiv:greeting. Using the simpler form reduces DSL noise.
addons/dotpipe.js (3)
313-323: Unknown inline verbs are silently ignored. Emit a warning.Right now the catch-all regex matches everything but does nothing if verb isn’t found.
Apply this diff:
- if (typeof this.verbs[verb] === 'function') { - currentValue = await this.verbs[verb](...params); - } - continue; + if (typeof this.verbs[verb] === 'function') { + try { + currentValue = await this.verbs[verb](...params); + } catch (e) { + console.error(`Error in inline verb '${verb}':`, e); + } + } else { + console.warn(`Unknown inline verb: '${verb}' in segment:`, seg); + } + continue;You can then remove the unreachable console.warn at Lines 322–323.
- console.warn('Unknown pipe segment:', seg);
382-382: Re-registering inline elements inside domContentLoad().dotPipe.register() is also triggered at top-level DOMContentLoaded callback. Re-registering can cause churn if called multiple times. Consider guarding or making register idempotent (e.g., skip elements already in matrix).
5334-5336: Asynchronous runInline() is unawaited; surface errors to console.Uncaught rejections in async pipelines are hard to debug. Either await (and make pipes async) or explicitly ignore but log errors.
Apply this diff:
- if (elem.hasAttribute("inline")) - dotPipe.runInline(elem.id); + if (elem.hasAttribute("inline")) { + void dotPipe.runInline(elem.id).catch(err => console.error('runInline error:', err)); + }csv-test/dotpipe.js (7)
213-224: Matrix keys: Symbol fallback makes runInline unreachable for elements without id.When an element lacks
idandpipe, you store it under aSymbol()key, but external code (andpipes(elem.id)) can’t retrieve that entry.
- Require
idon any element usinginline(docs already recommend unique ids), or- Add an overload
runInlineByElement(el)or allowrunInlineto accept an element as the key and resolve to its matrix entry.
382-383: Docs promise auto-exec on DOMContentLoaded; code only registers.If you want first-run behavior to match the docs, trigger
.runInlineafterregister():- dotPipe.register(); + dotPipe.register(); + // Optional: auto-run on load to match docs + document.querySelectorAll('[inline][id]').forEach(el => { + try { void dotPipe.runInline(el.id); } catch (e) { console.warn('inline init failed for', el.id, e); } + });If that’s too aggressive, gate with a boolean attribute like
inline-auto.
5334-5336: Don’t fire-and-forget an async inline run; handle errors.
runInlineis async. Calling it without await/handling can produce silent failures.Safer fire-and-forget with error logging:
- if (elem.hasAttribute("inline")) - dotPipe.runInline(elem.id); + if (elem.hasAttribute("inline") && elem.id) { + void dotPipe.runInline(elem.id).catch(err => console.error('inline error:', elem.id, err)); + }Converting
pipestoasyncand awaiting this is also fine, but more invasive.
4745-4748: Dead/unreachable return; strayfvariable.
getJSONFilereturns in thethenand then attemptsreturn f;wherefis undefined.- return resp.then(function (res) { - return res; - }); - return f; + return resp.then(res => res);
5274-5283: Event listener management won’t remove handlers;hasPipeListeneris ineffective.
flashClickListenertries to remove a listener created with an inline arrow function, which won’t match the original reference; removal is a no-op.
hasPipeListenercheckselem.onclickbut you useaddEventListener, so it always returns false and may duplicate listeners.Maintain a WeakMap of elements to bound handlers to prevent duplicates and allow removals.
Or set a sentinel data-attr/class after binding.
Also applies to: 5303-5306
606-631: Promises in modal/json pipelines aren’t guarded; errors can leak UI state.When
modal(...)or fetches fail, you insert partial scaffolding or leave loaders visible.
- Wrap
.then()with finally-like cleanup for loaders.- Surface failure via events consistently (you already dispatch events in refresh paths; mirror that).
Also applies to: 1071-1100
4466-4497: Use async Clipboard API instead of deprecated execCommand.
document.execCommand('copy')is deprecated and may fail under strict CSP.- // Copy the selected text to the clipboard - document.execCommand('copy'); + // Copy using modern API + await navigator.clipboard.writeText(textarea.value);Also consider graceful fallback if
navigator.clipboardis unavailable.login/dotpipe.js (4)
270-279: Guard property assignment against undefinedAvoid setting
el[prop] = undefinedwhen the variable is missing.- if (targetEl) { - let value = varName.startsWith('!') ? entry.dpVars[varName.slice(1)] : entry.dpVars[varName]; - targetEl[prop] = value; - } + if (targetEl) { + let value = varName.startsWith('!') ? entry.dpVars[varName.slice(1)] : entry.dpVars[varName]; + if (value !== undefined) targetEl[prop] = value; + }
210-212: Add an allowlist for callable functionsAdd a dedicated container to register safe functions callable from inline macros instead of defaulting to the global namespace.
const dotPipe = { - matrix: {}, + matrix: {}, + // Optional allowlist for %funcName calls + functions: {},
312-320: Colon-based param splitting breaks URLs; add quoting or smarter parsing
params = m[2].split(':')fails for URLs likehttps://...(common forajax). Consider bracket-quoting (e.g.,ajax:[https://...]:GET) or parsing from the right for known verbs so the last token is method and the rest is the URL.If you want a non-breaking improvement now, special-case
ajax:
- treat the last token as method if it’s
GET|POST|PUT|PATCH|DELETE,- join the preceding tokens back with
:as the URL.
96-99: Fix example to match the parserUntil
!prefix normalization is merged, the example should avoid!nop. Recommend:-<div id="fa" inline="ajax:/api/new:GET|!nop:newData|$resultsDiv:newData"></div> +<div id="fa" inline="ajax:/api/new:GET|nop:newData|$resultsDiv:newData"></div>If you keep the example, merge the
runInlinenormalization I suggested to support!nop.dotpipe.js (4)
293-310: Function-call args parsing is brittle; can’t handle quoted strings, commas, or JSON.
%func:[a,b,c]splits on commas without honoring quotes or brackets, so arguments like"hello, world"or arrays/objects break.
- Accept JSON-y args: if the entire bracket content parses as JSON array, use it; otherwise, fall back to the current split.
- Or add simple quoting support (parse
"...", '...'and unescape).Happy to provide a small parser if you want to support
"...",'...', numbers, booleans,#var,@id.prop.
312-320: Generic “verb” dispatch can swallow typos silently. Consider stricter matching.
/^([a-zA-Z0-9_]+):?(.*)$/will match almost anything not caught earlier, but if a verb is misspelled you’ll just get a no-op ifthis.verbs[verb]is missing.
- Warn when the verb name is unknown (you already
console.warnlater, but unreachable here).- Alternatively, reorder: test known verbs first and only then fall back to a terminal “unknown segment” branch.
338-339: Export the API on window for external callers.Docs show
dotPipe.runInline('fa')usage. Withoutwindow.dotPipe = dotPipe, external scripts can’t reliably access the object in all environments.Apply this diff just after the object literal:
}; +// Expose public API +window.dotPipe = dotPipe;
382-382: Re-registering on each domContentLoad is fine; be mindful of stale entries.Because
register()doesn’t clear removed elements,matrixcan accumulate dead entries. If that becomes an issue, consider pruning keys whoseelement.isConnected === false.I can provide a tiny
gc()helper to sweepmatrixperiodically or duringregister().landing/dotpipe.js (3)
322-323: Unreachable “Unknown pipe segment” warningBecause the generic verb regex matches any segment, this warning never fires. After implementing explicit verb-not-found logging in the fallback (previous comment), remove or keep this as a fallback of last resort.
339-339: Expose the public API on windowDocs advertise a “public API object dotPipe,” but
const dotPipe = {…}is not attached towindow. Expose it so external scripts and consoles can invokedotPipe.runInline().}; +// Public API +window.dotPipe = window.dotPipe || dotPipe;
82-106: Docs vs implementation: fix operators and auto-run claim
- Operators list and examples use a leading
!before op names (e.g.,!nop:var). Without the normalization fix suggested earlier, these fail. Update docs or implement the normalization (preferred).- The comment says “Inline macros can be automatically executed on DOMContentLoaded,” but the code only calls
register()on DOMContentLoaded; execution occurs insidepipes(). Consider either:
- Implementing an opt-in autorun (e.g.,
inline-autoattribute) that callsrunInline()post-register(), or- Rewording docs to state “run on interaction or when
pipes()processes the element.”I can draft either doc updates or an
autorunimplementation if you want this in the same PR.Also applies to: 154-156, 167-169
cart/dotpipe.js (3)
294-310: Inline %func args parsing is brittle (no quoting/escaping) and lacks error surfacing
args = m[2].split(',')fails for strings containing commas. Also, if the function throws or returns a rejected Promise, the exception is swallowed (no log beyond segment warning).Consider JSON-like args or quoted strings; minimally, wrap invocation in try/catch:
- if (typeof window[funcName] === 'function') { - currentValue = await window[funcName](...args); - } + if (typeof window[funcName] === 'function') { + try { + currentValue = await window[funcName](...args); + } catch (err) { + console.error(`dotPipe %${funcName} failed:`, err); + } + } else { + console.warn(`dotPipe: function ${funcName} not found`); + }If you want, I can add a minimal quoted-arg parser that respects commas within quotes.
8-9: Doc/operator list formatting and examples: align with actual grammar
- The operator list prefixes segments with “|…”. Internally, you split on “|”, so segments begin with &, $, @, #, %, etc. The examples are fine, but the bulleted list can mislead into trying a literal leading “|&…”. Also, the
$id:varNamedescription mentions both plain and!varNameforms, but code treats both the same; consider documenting one form.Recommendation:
- Keep examples as-is (they show the full pipeline with “|” separators).
- In the operator bullets, drop the leading “|” to denote “segment” form (e.g., “&var:value”, “$id:varName”, “%fn:[args]”).
- Remove mention of
|!unless implemented, or link it to the new no-op behavior (see previous comment).Also applies to: 34-35, 154-156
210-338: Consider centralizing dotPipe to avoid drift across modules
- This engine appears replicated (API/dotpipe.js et al.). Code duplication across modules risks subtle divergences (e.g., small regex or verb behavior differences).
Extract dotPipe into a single module (UMD/ESM) and import it wherever needed. If that’s not feasible right now, add a lightweight version check and keep one canonical source-of-truth to diff against during releases. I can sketch the module split if you want.
API/dotpipe.js (5)
296-305: Argument parsing for%func:[args]is fragile; commas inside strings/arrays will break.
m[2].split(',')cannot handle quoted strings, nested arrays/objects, or escaped commas. Use a small parser that supports quotes and simple bracket matching.Apply this minimal change, then add the helper below:
- let args = m[2].split(',').map(a => a.trim()).map(arg => { + let args = parseInlineArgs(m[2]).map(arg => { if (arg.startsWith('!')) return entry.dpVars[arg.slice(1)]; if (arg.startsWith('#')) return entry.dpVars[arg.slice(1)]; if (arg.startsWith('@')) { const [elId, prop] = arg.slice(1).split('.'); const targetEl = document.getElementById(elId); return targetEl ? targetEl[prop] : undefined; } return arg; });Add this helper (outside the object):
function parseInlineArgs(input) { const out = []; let cur = '', depth = 0, inQuote = false, quoteChar = ''; for (let i = 0; i < input.length; i++) { const ch = input[i]; if ((ch === '"' || ch === "'")) { if (!inQuote) { inQuote = true; quoteChar = ch; cur += ch; continue; } if (inQuote && ch === quoteChar && input[i-1] !== '\\') { inQuote = false; quoteChar = ''; cur += ch; continue; } } if (!inQuote && (ch === '[' || ch === '{' || ch === '(')) depth++; if (!inQuote && (ch === ']' || ch === '}' || ch === ')')) depth--; if (!inQuote && depth === 0 && ch === ',') { out.push(cur.trim()); cur = ''; continue; } cur += ch; } if (cur.trim() !== '') out.push(cur.trim()); return out; }
338-339: Expose the engine globally for external usage as documented.Docs and examples refer to
dotPipe.runInline('fa')from outside this module. Bind towindowexplicitly.}; +// Public exposure +window.dotPipe = dotPipe;
382-383: Optional: auto-run inline macros on DOMContentLoaded to match docs.You call
dotPipe.register(), but no automatic execution follows. If the intent is to run on page load (per docs), trigger execution for all registered elements here.- dotPipe.register(); + dotPipe.register(); + // Auto-run inline macros on load (fire-and-forget) + document.querySelectorAll('[inline]').forEach(el => { if (el.id) { void dotPipe.runInline(el.id); }});
239-246: State management: consider preserving dpVars across multiple runs or allow reset.Currently
dpVarsis per element and persists, but there's no way to reset between runs. Provide a way to clear per-element state (e.g., aclearDpVarsverb) or add an option inrunInline(key, { reset: true }).If desired, I can draft a patch adding
verbs.reset()and an optionalresetparameter inrunInline.Also applies to: 248-253, 281-289
210-224: Memory safety: prefer WeakMap keyed by element rather than string ids formatrix.If dynamic content is created/destroyed,
matrixcan retain orphaned entries. A WeakMap keyed byHTMLElementavoids leaks and id collisions. This is a broader change; consider for a follow-up.guitar/dotpipe.js (2)
34-35: Potential typo in documentation: Missing closing parenthesis.Line 35 appears to have an incomplete or incorrectly formatted example with pipe operators. The documentation shows:
Supports operators: |, |!, |$, |$id:varName, nop:varName, |@id.prop:varName, |#varName:id.prop, |%funcName:[args]but the operators list seems to be missing proper formatting or delimiters.- * Supports operators: |, |!, |$, |$id:varName, nop:varName, |@id.prop:varName, |#varName:id.prop, |%funcName:[args] + * Supports operators: | (pipe), |! (pass value), |$ (inject), |$id:varName, nop:varName, |@id.prop:varName, |#varName:id.prop, |%funcName:[args]
307-307: Function execution limited to window scope.The function execution at line 307 only checks
window[funcName], which limits flexibility. Consider supporting namespaced functions.- if (typeof window[funcName] === 'function') { - currentValue = await window[funcName](...args); + // Support both window functions and namespaced functions + let func = window[funcName]; + if (funcName.includes('.')) { + const parts = funcName.split('.'); + func = parts.reduce((obj, key) => obj?.[key], window); + } + if (typeof func === 'function') { + currentValue = await func(...args); }doted/dotpipe.js (8)
269-279: Set element properties defensively; fall back to attributes when the property doesn’t exist.Some props aren’t on the element instance;
setAttributeis safer in those cases.- let value = varName.startsWith('!') ? entry.dpVars[varName.slice(1)] : entry.dpVars[varName]; - targetEl[prop] = value; + let value = varName.startsWith('!') ? entry.dpVars[varName.slice(1)] : entry.dpVars[varName]; + if (prop in targetEl) { + targetEl[prop] = value; + } else { + targetEl.setAttribute(prop, value); + }
281-291: Read from attribute when a property isn’t present. Also normalize currentValue.- if (targetEl) { - entry.dpVars[varName] = targetEl[prop]; - currentValue = entry.dpVars[varName]; - } + if (targetEl) { + const v = (prop in targetEl) ? targetEl[prop] : targetEl.getAttribute(prop); + entry.dpVars[varName] = v; + currentValue = v; + }
293-310: Gate dynamic function calls and surface errors.Calling arbitrary
window[funcName]is powerful but risky. Consider an allowlist (e.g.,dotPipe.fns) and error handling so broken steps don’t silently stall the pipeline.I can draft a small allowlist mechanism if you want.
210-213: Expose the API on window to make it a true global surface.Top-level
constisn’t attached towindowin browsers. If other modules referencewindow.dotPipe, this ensures availability.const dotPipe = { @@ }; + +// Expose global +window.dotPipe = dotPipe;Also applies to: 338-339
382-383: Auto-run inline macros on DOMContentLoaded to match the documentation.Docs say inline macros run automatically; currently you only register them. Triggering their execution here aligns behavior.
- dotPipe.register(); + dotPipe.register(); + // Execute all registered inline macros once on load + document.querySelectorAll('[inline]').forEach(el => { + if (el.id) { void dotPipe.runInline(el.id); } + });If you want to avoid repeated runs, add an
inline-onceattribute check or aprocessedmarker class.
5334-5336: Explicitly ignore the returned Promise from runInline (or await it).For clarity in async error handling, either
awaitit or prefix withvoidto signal intentional fire-and-forget.- if (elem.hasAttribute("inline")) - dotPipe.runInline(elem.id); + if (elem.hasAttribute("inline") && elem.id) { + void dotPipe.runInline(elem.id); + }If inline steps feed into subsequent work in
pipes(), considerawaitinstead.
239-245: Optional: parse literals into types for &var:value.Right now all literals are strings. You could upgrade
&count:42→ number,&flag:true→ boolean, or detect JSON ({}/[]) for objects.- const value = m[2]; + let value = m[2]; + try { + // Basic type coercion + if (/^(true|false|\d+(\.\d+)?|\{.*\}|\[.*\])$/.test(value.trim())) { + value = JSON.parse(value); + } + } catch (_) { /* keep as string */ }
8-9: Nice inline macro documentation; please align operator list and system flow with actual behavior.
- If you keep "|!" out of the engine, remove it from docs.
- If you don’t auto-run inline on load, adjust the “SYSTEM FLOW” note or accept the earlier suggestion to auto-run.
I can update the doc blocks after you decide which behavior you want.
Also applies to: 34-35, 167-169
carousel/dotpipe.js (2)
5334-5336: Call runInline with the element (and consider async ordering)
- With the refactor above,
runInlinesupports both ids and elements. Passing the element is more robust and avoids id dependency.- Inline steps can be async (e.g., ajax). If subsequent logic assumes inline effects are complete, consider making
pipesreturn a Promise and awaitingrunInline. For now, this keeps fire-and-forget behavior.Apply this minimal diff:
- if (elem.hasAttribute("inline")) - dotPipe.runInline(elem.id); + if (elem.hasAttribute("inline")) + dotPipe.runInline(elem);If you want strict sequencing later, we can introduce
async function pipes(...)and thread promises through the few call sites that need ordering guarantees.
8-9: Align inline-macro docs with implementation and add safety note
- The pipeline delimiter is
|, but operators themselves should be documented without that prefix to avoid confusion.$without an id targets the current element (with the code change above).- Default injection is text; document
!htmlsuffix to opt into HTML injection to avoid XSS.Apply this doc tweak:
- * Inline macros can be used via the 'inline' attribute for dynamic, per-element logic. - * See below for supported elements, attributes, and inline macro usage. + * Inline macros can be used via the 'inline' attribute for dynamic, per-element logic. + * See below for supported elements, attributes, and inline macro usage. The `|` character is the pipeline delimiter. @@ - * inline Optional inline macro string to define dynamic logic per element. - * Supports operators: |, |!, |$, |$id:varName, nop:varName, |@id.prop:varName, |#varName:id.prop, |%funcName:[args] + * inline Optional inline macro string to define dynamic logic per element. + * Operators (used between `|` delimiters): + * &varName:value Store literal into var + * nop:varName Store current value in dpVars + * $[:varName] Inject into current element (text by default; use varName!html for HTML) + * $id[:varName] Inject into element by ID (text by default; use varName!html for HTML) + * @id.prop:varName Write stored var into target element property + * #varName:id.prop Read element property into variable + * %funcName:[args] Call function with args (supports #varName and @id.prop) @@ - * | Self-retained (current element context passes forward) - * |! Pass previous return value forward - * |$id Inject current value into element by ID - * |$id:varName Inject stored variable into element by ID - * nop:varName Store current value in dpVars for later use - * |@id.prop:varName Write stored variable into target element property - * |#varName:id.prop Read element property into variable - * |%funcName:[args] Execute function with arguments (supports #varName and @id.prop) + * &name:value Store literal into dpVars and set current value + * nop:varName Store current value in dpVars for later use + * $ Inject current value into current element (text) + * $:varName Inject dpVars[varName] into current element (append !html to varName to inject as HTML) + * $id Inject current value into element by ID (text) + * $id:varName Inject dpVars[varName] into element by ID (append !html to varName to inject as HTML) + * @id.prop:varName Write stored variable into target element property + * #varName:id.prop Read element property into variable + * %funcName:[args] Execute function with arguments (supports #varName and @id.prop) @@ - * <div id="fa" inline="ajax:/api/new:GET|!nop:newData|$resultsDiv:newData"></div> + * <div id="fa" inline="ajax:https://api.example.com/new:GET|nop:newData|$resultsDiv:newData!html"></div> @@ - * - Inline macros are parsed via regex, support all pipe operators, and store variables per element in dpVars. + * - Inline macros are parsed via regex, support the operators above, and store variables per element in dpVars. + * - Security: text is the default output; opt into HTML output by suffixing the var name with !html to avoid XSS.Also applies to: 34-36, 82-106, 154-157, 165-171
tree-view/dotpipe.js (4)
8-9: Clarify inline macro requirement for element keys (id vs pipe).Docs say "Inline macros can be used via the 'inline' attribute," but runtime requires a stable key to execute (you call runInline(elem.id) later). Please add that elements using inline must have either a unique id or a pipe attribute; otherwise runInline won’t find the matrix entry.
- * Inline macros can be used via the 'inline' attribute for dynamic, per-element logic. + * Inline macros can be used via the 'inline' attribute for dynamic, per-element logic. + * IMPORTANT: Elements must have a stable key (id preferred; pipe attribute supported) for inline execution.
34-35: Document the dpVars deref prefix and reserved segment tokens.Your implementation supports dereferencing dpVars via a leading "!" (e.g., $id:!varName), but the operator list doesn’t mention the "!" prefix. Add this to avoid confusion and to align examples with behavior.
- * Supports operators: |, |!, |$, |$id:varName, nop:varName, |@id.prop:varName, |#varName:id.prop, |%funcName:[args] + * Supports operators: |, |! (no-op/pass-through), |$, |$id:varName, nop:varName, |@id.prop:varName, |#varName:id.prop, |%funcName:[args] + * Variable deref: prefix a var with "!" to read from dpVars (e.g., $out:!name).
167-169: "Inline macros are parsed via regex" – call out argument parsing limitations.Given you split %func args by comma, nested commas and quoted strings will break. Either document this limitation here, or adopt a safer arg parser (see suggestion below for %func implementation).
382-382: Re-registration strategy: idempotency and dynamic content.Calling dotPipe.register() once on DOMContentLoaded won’t catch inline elements inserted later. Either:
- call register() again inside domContentLoad() after major mutations, or
- make register() idempotent and cheap (skip already-registered keys).
- dotPipe.register(); + dotPipe.register(); // consider calling again after dynamic inserts/updatesIf you want, I can wire post-insert hooks (e.g., after navigate/modala) to re-register new [inline] nodes.
activeMenu/dotpipe.js (3)
234-324: Add per-segment error isolation inrunInline()to avoid aborting entire macro.A single failing segment currently aborts the whole run. Wrap each segment’s handling in a
try/catchso other segments still execute and errors are observable.- for (let seg of segments) { - seg = seg.trim(); - let m; + for (let seg of segments) { + seg = seg.trim(); + let m; + try { // |&varName:value → store literal if (m = /^\&([a-zA-Z0-9_]+):(.+)$/.exec(seg)) { ... continue; } ... - console.warn('Unknown pipe segment:', seg); - } + console.warn('Unknown pipe segment:', seg); + } catch (err) { + console.error('dotPipe.runInline error at segment:', seg, err); + } + }This improves resilience when user-supplied macros evolve or external functions throw.
382-383: Avoid repeated full-DOM scans on everydomContentLoad()pass.Calling
dotPipe.register()unconditionally (Line 383) rescans the entire document each timedomContentLoad()runs. For dynamic pages, prefer:
- Using a
MutationObserverto register only newly added nodes with[inline], or- Filtering to unregistered nodes (e.g.,
[inline]:not([data-dotpipe-registered])and marking as processed duringregister()).This reduces unnecessary work and prevents matrix churn.
5334-5336: Asynchronous inline macros run fire-and-forget; confirm ordering assumptions.
dotPipe.runInline(elem.id)is async and not awaited. Any subsequent logic inpipes()will proceed without waiting for macros (e.g.,ajaxsegments) to complete. If inline effects must be visible before otherpipes()behaviors, convertpipes()toasyncandawaitthe call, or explicitly document that inline execution is non-blocking.If non-blocking is intended, consider prefixing with
voidto make intent clear and silence linters about unhandled promises:- dotPipe.runInline(elem.id); + void dotPipe.runInline(elem.id);columns/dotpipe.js (4)
214-224: Preserve per-element state and skip unaddressable entries in register()
- Re-registering on dynamic content wipes
dpVars. Preserve it between registrations for the same key.- Using
Symbol()for elements without id/pipe makes them impossible to address withrunInline(key)elsewhere. Either require an id or avoid registering items you cannot run.- elements.forEach(el => { - const key = el.id || el.getAttribute('pipe') || Symbol(); - this.matrix[key] = { - element: el, - inlineMacro: el.getAttribute('inline'), - dpVars: {} // plain storage; no auto-run - }; - }); + elements.forEach(el => { + const key = el.id || el.getAttribute('pipe'); + if (!key) return; // skip unaddressable elements + const existing = this.matrix[key]; + this.matrix[key] = { + element: el, + inlineMacro: el.getAttribute('inline'), + dpVars: existing?.dpVars ?? {} // preserve state across re-registers + }; + });
382-383: Auto-run opt-in to match docs without surprising network callsDocs claim “Inline macros can be automatically executed on DOMContentLoaded.” Consider an opt-in attribute (
inline-auto="true") to trigger runs immediately after registration.- dotPipe.register(); + window.dotPipe.register(); + // Auto-run only for elements that opt in + Object.keys(window.dotPipe.matrix).forEach(key => { + const entry = window.dotPipe.matrix[key]; + if (entry?.element?.getAttribute('inline-auto') === 'true') { + window.dotPipe.runInline(key); + } + });
97-99: Docs: keep examples aligned with implemented semanticsIf you choose not to implement
!pass-through handling, update examples to remove!beforenop:so they run today. If you do implement the!handling as suggested, no doc change is needed.Example fix (only needed if you skip the parser change):
-<div id="fa" inline="ajax:/api/new:GET|!nop:newData|$resultsDiv:newData"></div> +<div id="fa" inline="ajax:/api/new:GET|nop:newData|$resultsDiv:newData"></div> -<div id="fb" inline="ajax:/api/list:GET|!nop:list|%processData:[#list,@outputDiv.innerText]|@outputDiv.innerText:list"></div> +<div id="fb" inline="ajax:/api/list:GET|nop:list|%processData:[#list,@outputDiv.innerText]|@outputDiv.innerText:list"></div>Also applies to: 154-157
100-105: Docs vs behavior: clarify when inline macros auto-runCurrent code registers on DOMContentLoaded but doesn’t auto-execute except when
pipes()touches the element (often on click). Either:
- implement the optional auto-run as suggested, or
- clarify in docs that macros execute on interaction or when
pipes()processes the element.Also applies to: 167-169
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
API/dotpipe.js(8 hunks)activeMenu/dotpipe.js(8 hunks)addons/dotpipe.js(8 hunks)carousel/dotpipe.js(8 hunks)cart/dotpipe.js(8 hunks)columns/dotpipe.js(8 hunks)csv-test/dotpipe.js(8 hunks)doted/dotpipe.js(8 hunks)dotpipe.js(8 hunks)guitar/dotpipe.js(8 hunks)inline/index.html(1 hunks)landing/dotpipe.js(8 hunks)login/dotpipe.js(8 hunks)tree-view/dotpipe.js(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
tree-view/dotpipe.js (1)
dotpipe.js (26)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)varName(240-240)varName(249-249)varName(257-257)varName(273-273)varName(283-283)value(241-241)value(260-260)value(275-275)value(1612-1612)prop(272-272)prop(285-285)funcName(295-295)args(296-305)elId(300-300)verb(314-314)params(315-315)res(329-329)text(330-330)
API/dotpipe.js (1)
dotpipe.js (31)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)varName(240-240)varName(249-249)varName(257-257)varName(273-273)varName(283-283)value(241-241)value(260-260)value(275-275)value(1612-1612)targetId(256-256)targetEl(258-258)targetEl(271-271)targetEl(284-284)targetEl(301-301)prop(272-272)prop(285-285)funcName(295-295)args(296-305)elId(300-300)verb(314-314)params(315-315)res(329-329)text(330-330)
columns/dotpipe.js (2)
API/dotpipe.js (31)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)varName(240-240)varName(249-249)varName(257-257)varName(273-273)varName(283-283)value(241-241)value(260-260)value(275-275)value(1612-1612)targetId(256-256)targetEl(258-258)targetEl(271-271)targetEl(284-284)targetEl(301-301)prop(272-272)prop(285-285)funcName(295-295)args(296-305)elId(300-300)verb(314-314)params(315-315)res(329-329)text(330-330)dotpipe.js (18)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)targetId(256-256)prop(272-272)prop(285-285)funcName(295-295)args(296-305)elId(300-300)verb(314-314)params(315-315)res(329-329)text(330-330)
guitar/dotpipe.js (1)
dotpipe.js (31)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)varName(240-240)varName(249-249)varName(257-257)varName(273-273)varName(283-283)value(241-241)value(260-260)value(275-275)value(1612-1612)targetId(256-256)targetEl(258-258)targetEl(271-271)targetEl(284-284)targetEl(301-301)prop(272-272)prop(285-285)funcName(295-295)args(296-305)elId(300-300)verb(314-314)params(315-315)res(329-329)text(330-330)
cart/dotpipe.js (2)
API/dotpipe.js (31)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)varName(240-240)varName(249-249)varName(257-257)varName(273-273)varName(283-283)value(241-241)value(260-260)value(275-275)value(1612-1612)targetId(256-256)targetEl(258-258)targetEl(271-271)targetEl(284-284)targetEl(301-301)prop(272-272)prop(285-285)funcName(295-295)args(296-305)elId(300-300)verb(314-314)params(315-315)res(329-329)text(330-330)dotpipe.js (18)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)targetId(256-256)prop(272-272)prop(285-285)funcName(295-295)args(296-305)elId(300-300)verb(314-314)params(315-315)res(329-329)text(330-330)
landing/dotpipe.js (1)
dotpipe.js (25)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)varName(240-240)value(241-241)value(260-260)value(275-275)value(1612-1612)targetEl(258-258)targetEl(271-271)targetEl(284-284)targetEl(301-301)prop(272-272)prop(285-285)funcName(295-295)args(296-305)verb(314-314)params(315-315)res(329-329)text(330-330)
login/dotpipe.js (2)
API/dotpipe.js (31)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)varName(240-240)varName(249-249)varName(257-257)varName(273-273)varName(283-283)value(241-241)value(260-260)value(275-275)value(1612-1612)targetId(256-256)targetEl(258-258)targetEl(271-271)targetEl(284-284)targetEl(301-301)prop(272-272)prop(285-285)funcName(295-295)args(296-305)elId(300-300)verb(314-314)params(315-315)res(329-329)text(330-330)dotpipe.js (18)
dotPipe(210-338)elements(215-215)key(217-217)entry(228-228)currentValue(231-231)currentValue(4408-4408)segments(232-232)m(236-236)targetId(256-256)prop(272-272)prop(285-285)funcName(295-295)args(296-305)elId(300-300)verb(314-314)params(315-315)res(329-329)text(330-330)
| * inline Optional inline macro string to define dynamic logic per element. | ||
| * Supports operators: |, |!, |$, |$id:varName, nop:varName, |@id.prop:varName, |#varName:id.prop, |%funcName:[args] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Docs list unsupported operators (“|!”, bare “|$”) and omit supported “&var:value”.
The documented operator set doesn’t match the parser. “|!” and “!nop:…” aren’t implemented; “&var:value” is supported but undocumented here.
Update the operator list to the actually supported set:
- * Supports operators: |, |!, |$, |$id:varName, nop:varName, |@id.prop:varName, |#varName:id.prop, |%funcName:[args]
+ * Supports operators:
+ * &var:value Store a literal value into dpVars[var], set currentValue
+ * $id Inject currentValue into element by ID (text)
+ * $id:var Inject dpVars[var] into element by ID (text)
+ * @id.prop:var Write dpVars[var] to element property
+ * #var:id.prop Read element property into dpVars[var] and currentValue
+ * %func:[args...] Call allowed function with args (supports !var/#var/@id.prop)
+ * Notes:
+ * - “|” separates segments.
+ * - “|!” is not needed/recognized (currentValue flows by default).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * inline Optional inline macro string to define dynamic logic per element. | |
| * Supports operators: |, |!, |$, |$id:varName, nop:varName, |@id.prop:varName, |#varName:id.prop, |%funcName:[args] | |
| * inline Optional inline macro string to define dynamic logic per element. | |
| * Supports operators: | |
| * &var:value Store a literal value into dpVars[var], set currentValue | |
| * $id Inject currentValue into element by ID (text) | |
| * $id:var Inject dpVars[var] into element by ID (text) | |
| * @id.prop:var Write dpVars[var] to element property | |
| * #var:id.prop Read element property into dpVars[var] and currentValue | |
| * %func:[args...] Call allowed function with args (supports !var/#var/@id.prop) | |
| * Notes: | |
| * - “|” separates segments. | |
| * - “|!” is not needed/recognized (currentValue flows by default). |
🤖 Prompt for AI Agents
In addons/dotpipe.js around lines 34–35, the operator list in the inline macro
docs is incorrect: remove the unsupported bare "|!" and bare "|$" entries and
instead document the actual supported operators used by the parser; update the
line to enumerate the real operators (for example: |, |$id:varName, nop:varName,
|@id.prop:varName, |#varName:id.prop, |%funcName:[args], and the supported
&var:value form) so the docs match the implementation.
| * INLINE MACROS | ||
| * | ||
| * The 'inline' attribute allows per-element dynamic logic using dotPipe pipe operators: | ||
| * | ||
| * Operators: | ||
| * | Self-retained (current element context passes forward) | ||
| * |! Pass previous return value forward | ||
| * |$id Inject current value into element by ID | ||
| * |$id:varName Inject stored variable into element by ID | ||
| * nop:varName Store current value in dpVars for later use | ||
| * |@id.prop:varName Write stored variable into target element property | ||
| * |#varName:id.prop Read element property into variable | ||
| * |%funcName:[args] Execute function with arguments (supports #varName and @id.prop) | ||
| * | ||
| * Example usage: | ||
| * <div id="fa" inline="ajax:/api/new:GET|!nop:newData|$resultsDiv:newData"></div> | ||
| * <div id="fb" inline="ajax:/api/list:GET|!nop:list|%processData:[#list,@outputDiv.innerText]|@outputDiv.innerText:list"></div> | ||
| * | ||
| * Inline macros can be automatically executed on DOMContentLoaded or triggered manually via: | ||
| * dotPipe.runInline('fa'); | ||
| * | ||
| * Variables used in inline macros are scoped per element in a dpVars object. | ||
| * Functions called with |% can be native, plugin-registered, or globally available. | ||
| * | ||
| * ────────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inline macro examples include “!nop:…” which the parser will not recognize.
Parser expects “nop:var”; the leading “!” makes it an unknown segment. Examples also imply HTML injection by default.
Apply this diff to align examples and operator descriptions:
- * | Self-retained (current element context passes forward)
- * |! Pass previous return value forward
- * |$id Inject current value into element by ID
- * |$id:varName Inject stored variable into element by ID
+ * | Segment separator
+ * $id Inject current value into element by ID (as textContent)
+ * $id:varName Inject dpVars[varName] into element by ID (as textContent)
* nop:varName Store current value in dpVars for later use
* |@id.prop:varName Write stored variable into target element property
* |#varName:id.prop Read element property into variable
* |%funcName:[args] Execute function with arguments (supports #varName and @id.prop)
@@
- * <div id="fa" inline="ajax:/api/new:GET|!nop:newData|$resultsDiv:newData"></div>
- * <div id="fb" inline="ajax:/api/list:GET|!nop:list|%processData:[#list,@outputDiv.innerText]|@outputDiv.innerText:list"></div>
+ * <div id="fa" inline="ajax:/api/new:GET|nop:newData|$resultsDiv:newData"></div>
+ * <div id="fb" inline="ajax:/api/list:GET|nop:list|%processData:[#list,@outputDiv.innerText]|@outputDiv.innerText:list"></div>If you intend to support “|!” or “!nop:…”, extend the parser accordingly instead of changing docs.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * INLINE MACROS | |
| * | |
| * The 'inline' attribute allows per-element dynamic logic using dotPipe pipe operators: | |
| * | |
| * Operators: | |
| * | Self-retained (current element context passes forward) | |
| * |! Pass previous return value forward | |
| * |$id Inject current value into element by ID | |
| * |$id:varName Inject stored variable into element by ID | |
| * nop:varName Store current value in dpVars for later use | |
| * |@id.prop:varName Write stored variable into target element property | |
| * |#varName:id.prop Read element property into variable | |
| * |%funcName:[args] Execute function with arguments (supports #varName and @id.prop) | |
| * | |
| * Example usage: | |
| * <div id="fa" inline="ajax:/api/new:GET|!nop:newData|$resultsDiv:newData"></div> | |
| * <div id="fb" inline="ajax:/api/list:GET|!nop:list|%processData:[#list,@outputDiv.innerText]|@outputDiv.innerText:list"></div> | |
| * | |
| * Inline macros can be automatically executed on DOMContentLoaded or triggered manually via: | |
| * dotPipe.runInline('fa'); | |
| * | |
| * Variables used in inline macros are scoped per element in a dpVars object. | |
| * Functions called with |% can be native, plugin-registered, or globally available. | |
| * | |
| * ────────────────────────────────────────────────────────────── | |
| * INLINE MACROS | |
| * | |
| * The 'inline' attribute allows per-element dynamic logic using dotPipe pipe operators: | |
| * | |
| * Operators: | |
| * | Segment separator | |
| * $id Inject current value into element by ID (as textContent) | |
| * $id:varName Inject dpVars[varName] into element by ID (as textContent) | |
| * nop:varName Store current value in dpVars for later use | |
| * |@id.prop:varName Write stored variable into target element property | |
| * |#varName:id.prop Read element property into variable | |
| * |%funcName:[args] Execute function with arguments (supports #varName and @id.prop) | |
| * | |
| * Example usage: | |
| * <div id="fa" inline="ajax:/api/new:GET|nop:newData|$resultsDiv:newData"></div> | |
| * <div id="fb" inline="ajax:/api/list:GET|nop:list|%processData:[#list,@outputDiv.innerText]|@outputDiv.innerText:list"></div> | |
| * | |
| * Inline macros can be automatically executed on DOMContentLoaded or triggered manually via: | |
| * dotPipe.runInline('fa'); | |
| * | |
| * Variables used in inline macros are scoped per element in a dpVars object. | |
| * Functions called with |% can be native, plugin-registered, or globally available. | |
| * | |
| * ────────────────────────────────────────────────────────────── |
| const dotPipe = { | ||
| matrix: {}, | ||
|
|
||
| // Register all elements with inline macros | ||
| register: function(selector = '[inline]') { | ||
| const elements = document.querySelectorAll(selector); | ||
| elements.forEach(el => { | ||
| const key = el.id || el.getAttribute('pipe') || Symbol(); | ||
| this.matrix[key] = { | ||
| element: el, | ||
| inlineMacro: el.getAttribute('inline'), | ||
| dpVars: {} // plain storage; no auto-run | ||
| }; | ||
| }); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure stable keys and auto-assign IDs for [inline] elements without id.
Using Symbol() as a key prevents later lookups (e.g., runInline('…')). Also, storing by missing/empty id makes the API brittle.
Apply this diff:
- register: function(selector = '[inline]') {
+ register: function(selector = '[inline]') {
const elements = document.querySelectorAll(selector);
elements.forEach(el => {
- const key = el.id || el.getAttribute('pipe') || Symbol();
+ let key = (el.id && el.id.trim()) ? el.id : (el.getAttribute('pipe') || `dp-${Math.random().toString(36).slice(2)}`);
+ if (!el.id) el.id = key; // make it addressable
this.matrix[key] = {
element: el,
inlineMacro: el.getAttribute('inline'),
dpVars: {} // plain storage; no auto-run
};
});
},Optional: expose the API explicitly for consumers loaded in different script scopes:
-};
+};
+// Optional: explicit global export
+window.dotPipe = window.dotPipe || dotPipe;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dotPipe = { | |
| matrix: {}, | |
| // Register all elements with inline macros | |
| register: function(selector = '[inline]') { | |
| const elements = document.querySelectorAll(selector); | |
| elements.forEach(el => { | |
| const key = el.id || el.getAttribute('pipe') || Symbol(); | |
| this.matrix[key] = { | |
| element: el, | |
| inlineMacro: el.getAttribute('inline'), | |
| dpVars: {} // plain storage; no auto-run | |
| }; | |
| }); | |
| }, | |
| const dotPipe = { | |
| matrix: {}, | |
| // Register all elements with inline macros | |
| register: function(selector = '[inline]') { | |
| const elements = document.querySelectorAll(selector); | |
| elements.forEach(el => { | |
| // Use existing id if present, otherwise use 'pipe' attribute or generate a stable dp-... ID | |
| let key = (el.id && el.id.trim()) | |
| ? el.id | |
| : (el.getAttribute('pipe') || `dp-${Math.random().toString(36).slice(2)}`); | |
| if (!el.id) el.id = key; // ensure the element becomes addressable by id | |
| this.matrix[key] = { | |
| element: el, | |
| inlineMacro: el.getAttribute('inline'), | |
| dpVars: {} // plain storage; no auto-run | |
| }; | |
| }); | |
| }, | |
| // …other methods… | |
| }; | |
| // Optional: explicit global export for cross-scope consumers | |
| window.dotPipe = window.dotPipe || dotPipe; |
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | ||
| const targetId = m[1]; | ||
| const varName = m[2]; | ||
| const targetEl = document.getElementById(targetId); | ||
| if (targetEl) { | ||
| let value; | ||
| if (!varName) value = currentValue; | ||
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | ||
| else value = entry.dpVars[varName]; | ||
| targetEl.innerHTML = value; | ||
| } | ||
| continue; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
InnerHTML injection → XSS vector; write textContent by default.
Assigning unsanitized values to innerHTML from dpVars/currentValue enables XSS if any segment returns attacker-controlled data. Prefer textContent; let authors opt into HTML via @id.innerHTML:var.
Apply this diff:
- targetEl.innerHTML = value;
+ // Safer default: text
+ targetEl.textContent = (value ?? '');Authors can still do HTML intentionally with “@target.innerHTML:var”.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | |
| const targetId = m[1]; | |
| const varName = m[2]; | |
| const targetEl = document.getElementById(targetId); | |
| if (targetEl) { | |
| let value; | |
| if (!varName) value = currentValue; | |
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | |
| else value = entry.dpVars[varName]; | |
| targetEl.innerHTML = value; | |
| } | |
| continue; | |
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | |
| const targetId = m[1]; | |
| const varName = m[2]; | |
| const targetEl = document.getElementById(targetId); | |
| if (targetEl) { | |
| let value; | |
| if (!varName) value = currentValue; | |
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | |
| else value = entry.dpVars[varName]; | |
| // Safer default: text | |
| targetEl.textContent = (value ?? ''); | |
| } | |
| continue; |
🤖 Prompt for AI Agents
In addons/dotpipe.js around lines 255–266, the code assigns untrusted
dpVars/currentValue to targetEl.innerHTML which creates an XSS vector; change
the default assignment to targetEl.textContent = String(value ?? '') to render
as text, and modify the segment parsing so that innerHTML is only used when the
author explicitly requests HTML via the @targetId.innerHTML:var form (detect a
".innerHTML" suffix on the target token and in that case assign to innerHTML
instead); ensure varName resolution and the fallback currentValue logic remain
the same and that value is normalized to a string before assignment.
| if (typeof window[funcName] === 'function') { | ||
| currentValue = await window[funcName](...args); | ||
| } | ||
| continue; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Arbitrary global function invocation from markup.
Calling window[funcName] allows any global to be invoked via inline attribute. That’s a security and maintenance hazard.
Introduce an allowlist and fall back with a warning:
- if (typeof window[funcName] === 'function') {
- currentValue = await window[funcName](...args);
- }
+ const fn = (this.funcs && typeof this.funcs[funcName] === 'function')
+ ? this.funcs[funcName]
+ : (typeof window[funcName] === 'function' ? window[funcName] : null);
+ if (fn) {
+ currentValue = await fn(...args);
+ } else {
+ console.warn(`Inline macro attempted to call unknown or disallowed function: ${funcName}`);
+ }Add an allowlist container to the API:
const dotPipe = {
matrix: {},
+ funcs: {}, // optional allowlist: dotPipe.funcs.myFn = (...args) => {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof window[funcName] === 'function') { | |
| currentValue = await window[funcName](...args); | |
| } | |
| continue; | |
| const dotPipe = { | |
| matrix: {}, | |
| funcs: {}, // optional allowlist: dotPipe.funcs.myFn = (...args) => {} | |
| // …other properties… | |
| }; |
| if (typeof window[funcName] === 'function') { | |
| currentValue = await window[funcName](...args); | |
| } | |
| continue; | |
| // …previous context… | |
| const fn = (this.funcs && typeof this.funcs[funcName] === 'function') | |
| ? this.funcs[funcName] | |
| : (typeof window[funcName] === 'function' ? window[funcName] : null); | |
| if (fn) { | |
| currentValue = await fn(...args); | |
| } else { | |
| console.warn(`Inline macro attempted to call unknown or disallowed function: ${funcName}`); | |
| } | |
| continue; | |
| // …following context… |
🤖 Prompt for AI Agents
In addons/dotpipe.js around lines 306 to 309, the code directly invokes
window[funcName] which allows arbitrary global functions to be called from
markup; replace this with an allowlist-driven check: add an allowlist container
to the API (e.g., expose/register a safeFunctions map or array on the module or
options), verify funcName exists in that allowlist before calling it, and if not
present log a warning and skip invocation (do not call window[funcName]); ensure
the allowlist is configurable (default empty) and update callers/docs to
register safe functions rather than relying on global scope.
| // Run inline macro for a given element manually | ||
| runInline: async function(key) { | ||
| const entry = this.matrix[key]; | ||
| if (!entry || !entry.inlineMacro) return; | ||
|
|
||
| let currentValue = null; | ||
| const segments = entry.inlineMacro.split('|').filter(s => s.trim() !== ''); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make runInline robust: accept an Element or id; self-register on first run
Today runInline requires a pre-registered key. If called before register() or for elements without ids, it no-ops. Accepting an Element enables direct invocation and opportunistic registration.
- // Run inline macro for a given element manually
- runInline: async function(key) {
- const entry = this.matrix[key];
- if (!entry || !entry.inlineMacro) return;
+ // Run inline macro for a given element or id
+ runInline: async function(keyOrEl) {
+ let entry = null;
+ if (keyOrEl && keyOrEl.nodeType === 1) {
+ // Element passed in
+ const el = keyOrEl;
+ const key = el.id || (el.id = `dp-${(crypto?.randomUUID?.() || Math.random().toString(36).slice(2))}`);
+ entry = this.matrix[key] || (this.matrix[key] = {
+ element: el,
+ inlineMacro: el.getAttribute('inline'),
+ dpVars: this.matrix[key]?.dpVars || {}
+ });
+ } else {
+ const key = keyOrEl;
+ entry = this.matrix[key] || (document.getElementById(key) ? {
+ element: document.getElementById(key),
+ inlineMacro: document.getElementById(key).getAttribute('inline'),
+ dpVars: {}
+ } : null);
+ }
+ if (!entry || !entry.inlineMacro) return;
let currentValue = null;
const segments = entry.inlineMacro.split('|').filter(s => s.trim() !== '');
for (let seg of segments) {
seg = seg.trim();
+ // compat: allow segments prefixed with '!' (e.g., '!nop:x' -> 'nop:x')
+ if (seg.startsWith('!')) seg = seg.slice(1);
let m;🤖 Prompt for AI Agents
In login/dotpipe.js around lines 226-233, runInline currently only accepts a
pre-registered key and silently no-ops for Elements or unregistered nodes;
change it so it accepts either an Element or an id string, resolves an Element
to an id (if the Element has no id, generate a unique id, set it on the Element,
and proceed), and if no matrix entry exists for that id, call
this.register(element) or otherwise create the entry opportunistically before
continuing; then continue using the entry.inlineMacro as before so inline macros
run on first invocation for elements not previously registered.
| // $id or $id:varName → inject value into element | ||
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | ||
| const targetId = m[1]; | ||
| const varName = m[2]; | ||
| const targetEl = document.getElementById(targetId); | ||
| if (targetEl) { | ||
| let value; | ||
| if (!varName) value = currentValue; | ||
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | ||
| else value = entry.dpVars[varName]; | ||
| targetEl.innerHTML = value; | ||
| } | ||
| continue; |
There was a problem hiding this comment.
XSS risk: writing untrusted content via innerHTML
$id injects directly into innerHTML. If the value comes from AJAX or user input, this enables XSS. Prefer textContent by default; reserve HTML injection for an explicit, opt-in verb.
- targetEl.innerHTML = value;
+ // safer default: treat value as text
+ targetEl.textContent = value ?? '';Follow-up (optional): introduce a dedicated verb (e.g., html) to perform sanitized HTML injection using a sanitizer (DOMPurify) and gate its use.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // $id or $id:varName → inject value into element | |
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | |
| const targetId = m[1]; | |
| const varName = m[2]; | |
| const targetEl = document.getElementById(targetId); | |
| if (targetEl) { | |
| let value; | |
| if (!varName) value = currentValue; | |
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | |
| else value = entry.dpVars[varName]; | |
| targetEl.innerHTML = value; | |
| } | |
| continue; | |
| // $id or $id:varName → inject value into element | |
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | |
| const targetId = m[1]; | |
| const varName = m[2]; | |
| const targetEl = document.getElementById(targetId); | |
| if (targetEl) { | |
| let value; | |
| if (!varName) value = currentValue; | |
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | |
| else value = entry.dpVars[varName]; | |
| // safer default: treat value as text | |
| targetEl.textContent = value ?? ''; | |
| } | |
| continue; |
🤖 Prompt for AI Agents
In login/dotpipe.js around lines 254–266 the code writes untrusted values
directly to targetEl.innerHTML; change this to set targetEl.textContent =
String(value ?? '') so values are inserted as plain text; if HTML injection is
required add an explicit verb (e.g., `html`) that calls a sanitizer like
DOMPurify.sanitize(value) and only then assigns targetEl.innerHTML = sanitized;
ensure DOMPurify is imported/available, coerce null/undefined to an empty
string, and only use the sanitized path when the verb is the new explicit html
opt‑in.
| if (m = /^\%([a-zA-Z0-9_]+):\[(.+)\]$/.exec(seg)) { | ||
| const funcName = m[1]; | ||
| let args = m[2].split(',').map(a => a.trim()).map(arg => { | ||
| if (arg.startsWith('!')) return entry.dpVars[arg.slice(1)]; | ||
| if (arg.startsWith('#')) return entry.dpVars[arg.slice(1)]; | ||
| if (arg.startsWith('@')) { | ||
| const [elId, prop] = arg.slice(1).split('.'); | ||
| const targetEl = document.getElementById(elId); | ||
| return targetEl ? targetEl[prop] : undefined; | ||
| } | ||
| return arg; | ||
| }); | ||
| if (typeof window[funcName] === 'function') { | ||
| currentValue = await window[funcName](...args); | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unrestricted global function calls from inline macros
Calling any window[funcName] is dangerous and brittle. Provide an allowlist (e.g., dotPipe.functions) and fall back to window only if explicitly enabled.
- if (typeof window[funcName] === 'function') {
- currentValue = await window[funcName](...args);
- }
+ const fn = (this.functions && this.functions[funcName]) || window[funcName];
+ if (typeof fn === 'function') {
+ currentValue = await fn(...args);
+ } else {
+ console.warn(`dotPipe: function '${funcName}' is not registered/available`);
+ }Also add the allowlist container at the top of the object (see next comment).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (m = /^\%([a-zA-Z0-9_]+):\[(.+)\]$/.exec(seg)) { | |
| const funcName = m[1]; | |
| let args = m[2].split(',').map(a => a.trim()).map(arg => { | |
| if (arg.startsWith('!')) return entry.dpVars[arg.slice(1)]; | |
| if (arg.startsWith('#')) return entry.dpVars[arg.slice(1)]; | |
| if (arg.startsWith('@')) { | |
| const [elId, prop] = arg.slice(1).split('.'); | |
| const targetEl = document.getElementById(elId); | |
| return targetEl ? targetEl[prop] : undefined; | |
| } | |
| return arg; | |
| }); | |
| if (typeof window[funcName] === 'function') { | |
| currentValue = await window[funcName](...args); | |
| } | |
| continue; | |
| } | |
| if (m = /^\%([a-zA-Z0-9_]+):\[(.+)\]$/.exec(seg)) { | |
| const funcName = m[1]; | |
| let args = m[2].split(',').map(a => a.trim()).map(arg => { | |
| if (arg.startsWith('!')) return entry.dpVars[arg.slice(1)]; | |
| if (arg.startsWith('#')) return entry.dpVars[arg.slice(1)]; | |
| if (arg.startsWith('@')) { | |
| const [elId, prop] = arg.slice(1).split('.'); | |
| const targetEl = document.getElementById(elId); | |
| return targetEl ? targetEl[prop] : undefined; | |
| } | |
| return arg; | |
| }); | |
| // use an allowlist on this.functions, fall back to window if missing | |
| const fn = (this.functions && this.functions[funcName]) || window[funcName]; | |
| if (typeof fn === 'function') { | |
| currentValue = await fn(...args); | |
| } else { | |
| console.warn(`dotPipe: function '${funcName}' is not registered/available`); | |
| } | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In login/dotpipe.js around lines 294 to 310, the code directly calls
window[funcName] which allows arbitrary global function invocation; replace this
with a whitelist check against a dedicated container (e.g., dotPipe.functions)
placed at the top of the module and only call window as a last-resort if an
explicit flag (e.g., dotPipe.allowWindowFunctions) is true. Concretely: look up
the function first on dotPipe.functions[funcName], call it if present and is a
function; if not present and dotPipe.allowWindowFunctions is true, then check
window[funcName] and call it if it is a function; otherwise skip/throw a
controlled error. Ensure args handling and await behavior remain the same and do
not change other control flow.
| verbs: { | ||
| async ajax(url, method = 'GET') { | ||
| const res = await fetch(url, { method }); | ||
| const text = await res.text(); | ||
| return text; | ||
| }, | ||
| log(value) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden ajax verb with error handling and same-origin defaults
No res.ok check and no catch means silent failures; also consider constraining cross-origin by default.
- async ajax(url, method = 'GET') {
- const res = await fetch(url, { method });
- const text = await res.text();
- return text;
- },
+ async ajax(url, method = 'GET') {
+ try {
+ const res = await fetch(url, { method, credentials: 'same-origin' });
+ if (!res.ok) throw new Error(`${res.status} ${res.statusText}`);
+ return await res.text();
+ } catch (err) {
+ console.error('dotPipe.ajax failed:', err);
+ throw err;
+ }
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| verbs: { | |
| async ajax(url, method = 'GET') { | |
| const res = await fetch(url, { method }); | |
| const text = await res.text(); | |
| return text; | |
| }, | |
| log(value) { | |
| verbs: { | |
| async ajax(url, method = 'GET') { | |
| try { | |
| const res = await fetch(url, { method, credentials: 'same-origin' }); | |
| if (!res.ok) throw new Error(`${res.status} ${res.statusText}`); | |
| return await res.text(); | |
| } catch (err) { | |
| console.error('dotPipe.ajax failed:', err); | |
| throw err; | |
| } | |
| }, | |
| log(value) { |
🤖 Prompt for AI Agents
In login/dotpipe.js around lines 327 to 333, the ajax verb currently calls fetch
without checking res.ok or handling exceptions and allows cross-origin requests
by default; update it to wrap the fetch call in a try/catch, set conservative
fetch options like credentials: 'same-origin' (and mode: 'same-origin' if
needed), check res.ok after awaiting the response and throw or return a clear
error including status and statusText when not ok, and propagate or handle
network errors in the catch so failures are not silent.
| if (elem.hasAttribute("inline")) | ||
| dotPipe.runInline(elem.id); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Invoke inline macros with the Element and handle async errors
Passing only elem.id fails for id-less nodes; also, execution is async. Fire-and-forget is fine, but at least pass the element and surface errors.
- if (elem.hasAttribute("inline"))
- dotPipe.runInline(elem.id);
+ if (elem.hasAttribute("inline")) {
+ // pass the element; ignore result but log errors
+ void dotPipe.runInline(elem).catch(console.error);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (elem.hasAttribute("inline")) | |
| dotPipe.runInline(elem.id); | |
| if (elem.hasAttribute("inline")) { | |
| // pass the element; ignore result but log errors | |
| void dotPipe.runInline(elem).catch(console.error); | |
| } |
🤖 Prompt for AI Agents
In login/dotpipe.js around lines 5334 to 5336, the inline macro invocation
currently passes only elem.id (which breaks for id-less elements) and ignores
async errors; change the call to pass the Element itself
(dotPipe.runInline(elem)) and treat it as fire-and-forget while attaching a
.catch handler to surface errors (e.g., .catch(err => console.error("runInline
error", err)) or use the module's logger if available). Ensure the call remains
non-blocking but logs or reports any rejection so async failures are not
swallowed.
| * INLINE MACROS | ||
| * | ||
| * The 'inline' attribute allows per-element dynamic logic using dotPipe pipe operators: | ||
| * | ||
| * Operators: | ||
| * | Self-retained (current element context passes forward) | ||
| * |! Pass previous return value forward | ||
| * |$id Inject current value into element by ID | ||
| * |$id:varName Inject stored variable into element by ID | ||
| * nop:varName Store current value in dpVars for later use | ||
| * |@id.prop:varName Write stored variable into target element property | ||
| * |#varName:id.prop Read element property into variable | ||
| * |%funcName:[args] Execute function with arguments (supports #varName and @id.prop) | ||
| * | ||
| * Example usage: | ||
| * <div id="fa" inline="ajax:/api/new:GET|!nop:newData|$resultsDiv:newData"></div> | ||
| * <div id="fb" inline="ajax:/api/list:GET|!nop:list|%processData:[#list,@outputDiv.innerText]|@outputDiv.innerText:list"></div> | ||
| * | ||
| * Inline macros can be automatically executed on DOMContentLoaded or triggered manually via: | ||
| * dotPipe.runInline('fa'); | ||
| * | ||
| * Variables used in inline macros are scoped per element in a dpVars object. | ||
| * Functions called with |% can be native, plugin-registered, or globally available. | ||
| * | ||
| * ────────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
Inline-macro docs don’t match the parser; !nop: example won’t execute.
- The docs show segments like
|!nop:newData(Line 97) but after splitting on|, the segment becomes!nop:newData. The current regex only matches^nop:…$(Lines 247-251), so!nop:won’t match and the variable won’t be stored. - Either update the docs to remove
!beforenop:or make the parser accept an optional!.
Apply this minimal parser fix so the documented example works:
- if (m = /^nop:([a-zA-Z0-9_]+)$/.exec(seg)) {
+ if (m = /^!?nop:([a-zA-Z0-9_]+)$/.exec(seg)) {
const varName = m[1];
entry.dpVars[varName] = currentValue;
continue;
}Also consider clarifying the role of the standalone | operator in docs, since it’s not a segment after splitting and has no dedicated handling in the parser.
Also applies to: 247-251
🤖 Prompt for AI Agents
In activeMenu/dotpipe.js around lines 82-106 and specifically 247-251, the
inline-macro docs show a segment `|!nop:varName` but the parser only matches
`^nop:...$`, so `!nop:` never stores the variable; update the parser to accept
an optional leading exclamation for the nop command by changing the match to
allow an optional '!' (e.g. use /^!?nop:(.+)$/ or otherwise strip a leading '!'
before matching), ensuring the nop branch stores dpVars[varName] as before;
additionally, consider leaving a comment near the operator parsing to clarify
that a standalone '|' is a no-op separator and not produced as a segment after
splitting.
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | ||
| const targetId = m[1]; | ||
| const varName = m[2]; | ||
| const targetEl = document.getElementById(targetId); | ||
| if (targetEl) { | ||
| let value; | ||
| if (!varName) value = currentValue; | ||
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | ||
| else value = entry.dpVars[varName]; | ||
| targetEl.innerHTML = value; | ||
| } | ||
| continue; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
InnerHTML assignment enables XSS; default to textContent.
Injecting raw values into innerHTML (Line 264) is dangerous when values originate from ajax or user input. Default to textContent. If HTML injection is truly needed, introduce an explicit “unsafe HTML” verb (e.g., html:$id:var) or a safe sanitizer.
- targetEl.innerHTML = value;
+ // safer default: no HTML interpretation
+ targetEl.textContent = value ?? '';This aligns with secure defaults while still allowing an explicit, auditable escape hatch for trusted HTML.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | |
| const targetId = m[1]; | |
| const varName = m[2]; | |
| const targetEl = document.getElementById(targetId); | |
| if (targetEl) { | |
| let value; | |
| if (!varName) value = currentValue; | |
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | |
| else value = entry.dpVars[varName]; | |
| targetEl.innerHTML = value; | |
| } | |
| continue; | |
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | |
| const targetId = m[1]; | |
| const varName = m[2]; | |
| const targetEl = document.getElementById(targetId); | |
| if (targetEl) { | |
| let value; | |
| if (!varName) value = currentValue; | |
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | |
| else value = entry.dpVars[varName]; | |
| // safer default: no HTML interpretation | |
| targetEl.textContent = value ?? ''; | |
| } | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In activeMenu/dotpipe.js around lines 255 to 266, the code assigns untrusted
values to targetEl.innerHTML which enables XSS; change this to assign to
targetEl.textContent by default, and if you need to allow HTML introduce an
explicit, auditable verb (for example a prefix like "html:$id:var") or run the
value through a trusted sanitizer before assigning to innerHTML. Update the
branch that resolves value (currentValue / entry.dpVars) to set
targetEl.textContent = value; and add handling for the new explicit unsafe-html
verb path that only sets innerHTML after sanitization or explicit opt-in.
| async ajax(url, method = 'GET') { | ||
| const res = await fetch(url, { method }); | ||
| const text = await res.text(); | ||
| return text; | ||
| }, | ||
| log(value) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden ajax verb: check HTTP status and surface errors.
Currently failures (non-2xx) still call res.text() and return misleading content. Add an ok check, include status in the error, and let callers decide how to handle it.
- async ajax(url, method = 'GET') {
- const res = await fetch(url, { method });
- const text = await res.text();
- return text;
- },
+ async ajax(url, method = 'GET') {
+ const res = await fetch(url, { method });
+ if (!res.ok) {
+ throw new Error(`dotPipe.ajax failed: ${res.status} ${res.statusText} (${url})`);
+ }
+ return res.text();
+ },Optionally consider credentials: 'same-origin' when relevant.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async ajax(url, method = 'GET') { | |
| const res = await fetch(url, { method }); | |
| const text = await res.text(); | |
| return text; | |
| }, | |
| log(value) { | |
| async ajax(url, method = 'GET') { | |
| const res = await fetch(url, { method }); | |
| if (!res.ok) { | |
| throw new Error(`dotPipe.ajax failed: ${res.status} ${res.statusText} (${url})`); | |
| } | |
| return res.text(); | |
| }, | |
| log(value) { |
🤖 Prompt for AI Agents
In activeMenu/dotpipe.js around lines 328 to 333, the ajax helper currently
always calls res.text() and returns it even for non-2xx responses; change it to
check res.ok and, on failure, read the response body (await res.text()) and
throw an Error that includes HTTP status, statusText and the response body so
callers can decide how to handle it; keep the successful path returning the body
as before, and consider adding fetch option credentials: 'same-origin' if this
endpoint requires same-origin cookies or auth.
| const dotPipe = { | ||
| matrix: {}, | ||
|
|
||
| // Register all elements with inline macros | ||
| register: function(selector = '[inline]') { | ||
| const elements = document.querySelectorAll(selector); | ||
| elements.forEach(el => { | ||
| const key = el.id || el.getAttribute('pipe') || Symbol(); | ||
| this.matrix[key] = { | ||
| element: el, | ||
| inlineMacro: el.getAttribute('inline'), | ||
| dpVars: {} // plain storage; no auto-run | ||
| }; | ||
| }); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden per-element state storage; avoid Symbol keys and id-only lookups
Registering elements under either id, a pipe attribute, or a Symbol() causes two issues:
- Elements without ids are stored under a Symbol that cannot be referenced later by
runInline(elem.id), so those entries are effectively unreachable. - Using a plain object for
matrixrisks memory leaks because entries are never released when nodes are removed from the DOM.
Use a WeakMap keyed by the element itself and let runInline accept either an element or an id. Also auto-assign an id for elements missing one, so users can still call dotPipe.runInline('id') if they want.
Apply this diff:
-const dotPipe = {
- matrix: {},
+const dotPipe = {
+ // Store per-element state; auto-GC when DOM nodes are gone
+ matrix: new WeakMap(),
+ autoId: 0,
@@
- register: function(selector = '[inline]') {
+ register: function(selector = '[inline]') {
const elements = document.querySelectorAll(selector);
elements.forEach(el => {
- const key = el.id || el.getAttribute('pipe') || Symbol();
- this.matrix[key] = {
- element: el,
- inlineMacro: el.getAttribute('inline'),
- dpVars: {} // plain storage; no auto-run
- };
+ if (!el.id) {
+ el.id = `dp-inline-${++this.autoId}`;
+ }
+ this.matrix.set(el, {
+ element: el,
+ inlineMacro: el.getAttribute('inline'),
+ dpVars: {} // per-element storage; no auto-run
+ });
});
},
@@
- runInline: async function(key) {
- const entry = this.matrix[key];
- if (!entry || !entry.inlineMacro) return;
+ runInline: async function(target) {
+ const el = (typeof target === 'string') ? document.getElementById(target) : target;
+ if (!el) return;
+ // Ensure entry exists even if register() wasn't called yet or macro changed
+ let entry = this.matrix.get(el);
+ if (!entry) {
+ entry = { element: el, inlineMacro: el.getAttribute('inline'), dpVars: {} };
+ this.matrix.set(el, entry);
+ } else {
+ entry.inlineMacro = el.getAttribute('inline');
+ }
+ if (!entry.inlineMacro) return;Also applies to: 226-233
🤖 Prompt for AI Agents
In carousel/dotpipe.js around lines 210-224 (and also apply same changes to
226-233), the current matrix uses object keys (id/pipe/Symbol) which creates
unreachable entries for Symbol-keyed elements and prevents cleanup; replace
matrix with a WeakMap keyed by the actual Element to avoid leaks and make
per-element state retrievable, change register to always key by the element
(assign a stable auto-generated id attribute when missing so callers can still
use dotPipe.runInline('id')), store state object as the WeakMap value (element,
inlineMacro, dpVars), and update runInline to accept either an Element or an id
string (resolve id -> element via document.getElementById then lookup the
WeakMap); remove use of Symbol keys and ensure no plain-object storage of
element-specific state.
| // $id or $id:varName → inject value into element | ||
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | ||
| const targetId = m[1]; | ||
| const varName = m[2]; | ||
| const targetEl = document.getElementById(targetId); | ||
| if (targetEl) { | ||
| let value; | ||
| if (!varName) value = currentValue; | ||
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | ||
| else value = entry.dpVars[varName]; | ||
| targetEl.innerHTML = value; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| // @id.prop:varName → set element property | ||
| if (m = /^@([a-zA-Z0-9_-]+)\.([a-zA-Z0-9_]+):([a-zA-Z0-9_!]+)$/.exec(seg)) { | ||
| const targetEl = document.getElementById(m[1]); | ||
| const prop = m[2]; | ||
| const varName = m[3]; | ||
| if (targetEl) { | ||
| let value = varName.startsWith('!') ? entry.dpVars[varName.slice(1)] : entry.dpVars[varName]; | ||
| targetEl[prop] = value; | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Default to safe text injection; provide explicit opt‑in for HTML
The $... branch uses innerHTML directly, which enables XSS if values originate from remote content. Make text the default and allow HTML explicitly (e.g., variable name suffix !html). Also add support for bare $ to target the current element, aligning with the docs.
Apply this diff:
- // $id or $id:varName → inject value into element
- if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) {
- const targetId = m[1];
- const varName = m[2];
- const targetEl = document.getElementById(targetId);
- if (targetEl) {
- let value;
- if (!varName) value = currentValue;
- else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)];
- else value = entry.dpVars[varName];
- targetEl.innerHTML = value;
- }
- continue;
- }
+ // $ or $:varName → inject into current element (safe by default)
+ if (m = /^\$(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) {
+ const varName = m[1];
+ const targetEl = entry.element;
+ if (targetEl) {
+ const rawHtml = !!(varName && /!html$/.test(varName));
+ const key = varName
+ ? (varName.replace(/!html$/, '').startsWith('!') ? varName.replace(/^!/, '').replace(/!html$/, '') : varName.replace(/!html$/, ''))
+ : null;
+ const value = (varName ? entry.dpVars[key] : currentValue);
+ if (rawHtml) targetEl.innerHTML = value;
+ else targetEl.textContent = value;
+ }
+ continue;
+ }
+
+ // $id or $id:varName → inject into element by id (safe by default)
+ if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) {
+ const targetId = m[1];
+ const varName = m[2];
+ const targetEl = document.getElementById(targetId);
+ if (targetEl) {
+ const rawHtml = !!(varName && /!html$/.test(varName));
+ const key = varName
+ ? (varName.replace(/!html$/, '').startsWith('!') ? varName.replace(/^!/, '').replace(/!html$/, '') : varName.replace(/!html$/, ''))
+ : null;
+ const value = (varName ? entry.dpVars[key] : currentValue);
+ if (rawHtml) targetEl.innerHTML = value;
+ else targetEl.textContent = value;
+ }
+ continue;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // $id or $id:varName → inject value into element | |
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | |
| const targetId = m[1]; | |
| const varName = m[2]; | |
| const targetEl = document.getElementById(targetId); | |
| if (targetEl) { | |
| let value; | |
| if (!varName) value = currentValue; | |
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | |
| else value = entry.dpVars[varName]; | |
| targetEl.innerHTML = value; | |
| } | |
| continue; | |
| } | |
| // @id.prop:varName → set element property | |
| if (m = /^@([a-zA-Z0-9_-]+)\.([a-zA-Z0-9_]+):([a-zA-Z0-9_!]+)$/.exec(seg)) { | |
| const targetEl = document.getElementById(m[1]); | |
| const prop = m[2]; | |
| const varName = m[3]; | |
| if (targetEl) { | |
| let value = varName.startsWith('!') ? entry.dpVars[varName.slice(1)] : entry.dpVars[varName]; | |
| targetEl[prop] = value; | |
| } | |
| continue; | |
| } | |
| // $ or $:varName → inject into current element (safe by default) | |
| if (m = /^\$(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | |
| const varName = m[1]; | |
| const targetEl = entry.element; | |
| if (targetEl) { | |
| // detect explicit HTML opt-in via "!html" suffix | |
| const rawHtml = !!(varName && /!html$/.test(varName)); | |
| // strip "!html" and leading "!" (for legacy var-negation) to get the dpVars key | |
| const key = varName | |
| ? ( | |
| varName | |
| .replace(/!html$/, '') | |
| .replace(/^!/, '') | |
| ) | |
| : null; | |
| const value = varName | |
| ? entry.dpVars[key] | |
| : currentValue; | |
| if (rawHtml) targetEl.innerHTML = value; | |
| else targetEl.textContent = value; | |
| } | |
| continue; | |
| } | |
| // $id or $id:varName → inject into element by id (safe by default) | |
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | |
| const targetId = m[1]; | |
| const varName = m[2]; | |
| const targetEl = document.getElementById(targetId); | |
| if (targetEl) { | |
| const rawHtml = !!(varName && /!html$/.test(varName)); | |
| const key = varName | |
| ? ( | |
| varName | |
| .replace(/!html$/, '') | |
| .replace(/^!/, '') | |
| ) | |
| : null; | |
| const value = varName | |
| ? entry.dpVars[key] | |
| : currentValue; | |
| if (rawHtml) targetEl.innerHTML = value; | |
| else targetEl.textContent = value; | |
| } | |
| continue; | |
| } | |
| // @id.prop:varName → set element property | |
| if (m = /^@([a-zA-Z0-9_-]+)\.([a-zA-Z0-9_]+):([a-zA-Z0-9_!]+)$/.exec(seg)) { | |
| const targetEl = document.getElementById(m[1]); | |
| const prop = m[2]; | |
| const varName = m[3]; | |
| if (targetEl) { | |
| const value = varName.startsWith('!') | |
| ? entry.dpVars[varName.slice(1)] | |
| : entry.dpVars[varName]; | |
| targetEl[prop] = value; | |
| } | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In carousel/dotpipe.js around lines 254 to 279, the $... branch currently
assigns values using innerHTML (enabling XSS) and doesn't support bare $ to
target the current element; change behavior so text injection is default using
textContent, add an explicit HTML opt‑in when the varName equals or ends with a
special marker (e.g., '!html' or the existing '!' semantics updated to
distinguish html vs variable negation), and allow a bare '$' (no id) to mean the
current element rather than looking up by id. Concretely: detect if seg is '$'
or matches '$id(:varName)?' — if bare '$' use the current element reference,
otherwise getElementById(id); determine value from currentValue or entry.dpVars
as before; if the varName indicates HTML opt‑in (e.g., endsWith(':!html') or a
'!html' token) assign to innerHTML, otherwise assign to textContent to avoid
XSS; preserve existing support for varName starting with '!' to reference dpVars
but treat '!html' specially as the HTML opt‑in. Ensure similar HTML‑opt in logic
is applied consistently to the @... property branch if HTML assignment is
relevant.
| const dotPipe = { | ||
| matrix: {}, | ||
|
|
||
| // Register all elements with inline macros | ||
| register: function(selector = '[inline]') { | ||
| const elements = document.querySelectorAll(selector); | ||
| elements.forEach(el => { | ||
| const key = el.id || el.getAttribute('pipe') || Symbol(); | ||
| this.matrix[key] = { | ||
| element: el, | ||
| inlineMacro: el.getAttribute('inline'), | ||
| dpVars: {} // plain storage; no auto-run | ||
| }; | ||
| }); | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid Symbol keys for matrix entries; align keying with pipes() usage.
register() stores entries keyed by el.id || el.getAttribute('pipe') || Symbol(). However, pipes() later calls runInline(elem.id), so any Symbol-keyed registrations are unreachable. Prefer requiring a stable key (id or pipe) and warn when missing.
- register: function(selector = '[inline]') {
+ register: function(selector = '[inline]') {
const elements = document.querySelectorAll(selector);
elements.forEach(el => {
- const key = el.id || el.getAttribute('pipe') || Symbol();
- this.matrix[key] = {
+ const key = el.id || el.getAttribute('pipe');
+ if (!key) {
+ console.warn('dotPipe.register: element with inline attribute missing id/pipe:', el);
+ return;
+ }
+ this.matrix[key] = {
element: el,
inlineMacro: el.getAttribute('inline'),
dpVars: {} // plain storage; no auto-run
};
});
},If you need support for non-id elements, I can add an internal data attribute (e.g., data-dp-key) to bridge registration and execution.
🏁 Script executed:
#!/bin/bash
# Locate dotpipe.js
FILE=$(find . -type f -name "dotpipe.js" | head -n1)
echo "dotpipe.js path: $FILE"
# Show register() snippet around lines 190–260
echo "----- Lines 190–260 of dotpipe.js -----"
sed -n '190,260p' "$FILE"
# Show pipes() definition
echo "----- Searching for pipes() function -----"
grep -R -n "function pipes" -n "$FILE" || echo "No pipes() function found"
# Show runInline usage
echo "----- Searching for runInline -----"
grep -R -n "runInline" -n "$FILE" || echo "No runInline found in dotpipe.js"
# Inspect matrix usage across repository
echo "----- matrix usage in codebase -----"
grep -R -n "matrix" -n .Length of output: 5824
🏁 Script executed:
#!/bin/bash
# Inspect tree-view/dotpipe.js around register() and runInline() to verify key handling and invocation
FILE=./tree-view/dotpipe.js
echo "----- Lines 180–260 of tree-view/dotpipe.js -----"
sed -n '180,260p' "$FILE"
echo
echo "----- Searching for function pipes in tree-view/dotpipe.js -----"
grep -R -n "function pipes" -n "$FILE" || echo "No pipes() definition in tree-view/dotpipe.js"
echo
# If pipes() found, show context
grep -R -n "function pipes" -n "$FILE" | while IFS=":" read -r path line _; do
start=$((line - 10))
end=$((line + 50))
echo "----- Lines $start–$end around pipes() in tree-view/dotpipe.js -----"
sed -n "${start},${end}p" "$FILE"
done
echo
echo "----- Searching for dotPipe.runInline usage in tree-view/dotpipe.js -----"
grep -R -n "dotPipe.runInline" -n "$FILE" || echo "No runInline calls in tree-view/dotpipe.js"Length of output: 3723
🏁 Script executed:
#!/bin/bash
# Extract pipes() definition context to confirm key usage
FILE=./tree-view/dotpipe.js
# Locate pipes() definition
line=$(grep -n "function pipes" "$FILE" | head -n1 | cut -d: -f1)
if [ -z "$line" ]; then
echo "pipes() not found in tree-view/dotpipe.js"
exit 0
fi
echo "pipes() defined at line $line"
start=$(( line > 10 ? line - 10 : 1 ))
end=$(( line + 30 ))
echo "----- Lines $start–$end around pipes() in tree-view/dotpipe.js -----"
sed -n "${start},${end}p" "$FILE"Length of output: 1712
Align registration keys with runtime invocation in pipes(), and remove unreachable Symbol fallback
The current register() implementation in tree-view/dotpipe.js (lines 211–224) uses
const key = el.id || el.getAttribute('pipe') || Symbol();but the pipes() handler at line 5325 always calls
dotPipe.runInline(elem.id);– never using the pipe attribute or any Symbol-generated key. As a result, any registration under a Symbol or a non-matching pipe value will never be executed.
Action items:
- In
tree-view/dotpipe.jsregister() (≈ lines 211–224), remove thegetAttribute('pipe')andSymbol()fallbacks. Require and warn on a stableidonly. - In
tree-view/dotpipe.jspipes() (≈ line 5325), continue usingelem.id, ensuring consistency. - Optionally, if you intend to support a
pipe-attribute key, modifypipes()to derive its key fromelem.getAttribute('pipe')instead ofelem.id.
Suggested refactor:
// File: tree-view/dotpipe.js
// Register all elements with inline macros
register: function(selector = '[inline]') {
const elements = document.querySelectorAll(selector);
elements.forEach(el => {
- const key = el.id || el.getAttribute('pipe') || Symbol();
- this.matrix[key] = {
+ const key = el.id;
+ if (!key) {
+ console.warn('dotPipe.register: inline element missing id:', el);
+ return;
+ }
+ this.matrix[key] = {
element: el,
inlineMacro: el.getAttribute('inline'),
dpVars: {} // plain storage; no auto-run
};
});
},// File: tree-view/dotpipe.js, in pipes()
- if (elem.hasAttribute("inline"))
- dotPipe.runInline(elem.id);
+ if (elem.hasAttribute("inline")) {
+ // Only id-keyed entries are registered/run
+ dotPipe.runInline(elem.id);
+ }🤖 Prompt for AI Agents
In tree-view/dotpipe.js around lines 211–224, the register() function assigns
keys using el.id || el.getAttribute('pipe') || Symbol(), but runtime always
looks up by element id (dotPipe.runInline(elem.id)), so registrations using the
pipe attribute or Symbol are unreachable; change register() to require a stable
id only (use el.id), remove getAttribute('pipe') and Symbol() fallbacks, and log
or warn when an element lacks an id instead of silently registering under a
non-matching key; keep pipes() as-is using elem.id (or if you prefer
pipe-attribute semantics, update pipes() to derive the key from
elem.getAttribute('pipe') instead—pick one consistent approach).
| // $id or $id:varName → inject value into element | ||
| if (m = /^\$([a-zA-Z0-9_-]+)(?::([a-zA-Z0-9_!]+))?$/.exec(seg)) { | ||
| const targetId = m[1]; | ||
| const varName = m[2]; | ||
| const targetEl = document.getElementById(targetId); | ||
| if (targetEl) { | ||
| let value; | ||
| if (!varName) value = currentValue; | ||
| else if (varName.startsWith('!')) value = entry.dpVars[varName.slice(1)]; | ||
| else value = entry.dpVars[varName]; | ||
| targetEl.innerHTML = value; | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
XSS risk: using innerHTML for $id and $id:varName injections.
Setting innerHTML directly with values from dpVars or verb results can execute script markup. Unless you explicitly intend HTML injection, default to textContent; provide an opt-in for HTML.
- targetEl.innerHTML = value;
+ // Safer default to avoid XSS
+ targetEl.textContent = value ?? '';Optional: support an explicit HTML variant like $idHtml:varName and handle it as innerHTML.
| // %funcName:[args] → call function | ||
| if (m = /^\%([a-zA-Z0-9_]+):\[(.+)\]$/.exec(seg)) { | ||
| const funcName = m[1]; | ||
| let args = m[2].split(',').map(a => a.trim()).map(arg => { | ||
| if (arg.startsWith('!')) return entry.dpVars[arg.slice(1)]; | ||
| if (arg.startsWith('#')) return entry.dpVars[arg.slice(1)]; | ||
| if (arg.startsWith('@')) { | ||
| const [elId, prop] = arg.slice(1).split('.'); | ||
| const targetEl = document.getElementById(elId); | ||
| return targetEl ? targetEl[prop] : undefined; | ||
| } | ||
| return arg; | ||
| }); | ||
| if (typeof window[funcName] === 'function') { | ||
| currentValue = await window[funcName](...args); | ||
| } | ||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
%func args parsing is brittle (breaks with commas in strings/JSON); also add error handling.
Using split(',') fails for quoted strings and nested arrays/objects. Replace with a small parser that respects quotes/brackets and map tokens using your existing !/#/@ deref rules. Also wrap the function call in try/catch to prevent one segment from breaking the whole pipeline.
- if (m = /^\%([a-zA-Z0-9_]+):\[(.+)\]$/.exec(seg)) {
+ if (m = /^\%([a-zA-Z0-9_]+):\[(.+)\]$/.exec(seg)) {
const funcName = m[1];
- let args = m[2].split(',').map(a => a.trim()).map(arg => {
- if (arg.startsWith('!')) return entry.dpVars[arg.slice(1)];
- if (arg.startsWith('#')) return entry.dpVars[arg.slice(1)];
- if (arg.startsWith('@')) {
- const [elId, prop] = arg.slice(1).split('.');
- const targetEl = document.getElementById(elId);
- return targetEl ? targetEl[prop] : undefined;
- }
- return arg;
- });
- if (typeof window[funcName] === 'function') {
- currentValue = await window[funcName](...args);
- }
+ const args = parseInlineArgs(m[2], entry);
+ if (typeof window[funcName] === 'function') {
+ try {
+ currentValue = await window[funcName](...args);
+ } catch (err) {
+ console.error('dotPipe %func error:', funcName, err);
+ }
+ } else {
+ console.warn('dotPipe: function not found:', funcName);
+ }
continue;
}Add this helper outside of the object (or as a method), which safely parses comma-separated args with quotes/brackets:
function parseInlineArgs(argList, entry) {
const out = [];
let buf = '', depth = 0, inStr = false, quote = '';
for (let i = 0; i < argList.length; i++) {
const c = argList[i];
if ((c === '"' || c === "'") && argList[i-1] !== '\\') {
if (!inStr) { inStr = true; quote = c; }
else if (quote === c) { inStr = false; quote = ''; }
buf += c;
continue;
}
if (!inStr && (c === '[' || c === '{' || c === '(')) depth++;
if (!inStr && (c === ']' || c === '}' || c === ')')) depth--;
if (!inStr && depth === 0 && c === ',') {
out.push(resolveArg(buf.trim(), entry));
buf = '';
} else {
buf += c;
}
}
if (buf.trim()) out.push(resolveArg(buf.trim(), entry));
return out;
}
function resolveArg(arg, entry) {
if (arg.startsWith('!') || arg.startsWith('#')) return entry.dpVars[arg.slice(1)];
if (arg.startsWith('@')) {
const [elId, ...rest] = arg.slice(1).split('.');
const targetEl = document.getElementById(elId);
if (!targetEl) return undefined;
// support nested props, e.g., @el.style.color
return rest.reduce((obj, key) => (obj ? obj[key] : undefined), targetEl);
}
// strip quotes if present
if ((arg.startsWith('"') && arg.endsWith('"')) || (arg.startsWith("'") && arg.endsWith("'"))) {
return arg.slice(1, -1);
}
return arg; // literal
}🤖 Prompt for AI Agents
In tree-view/dotpipe.js around lines 293 to 311, the current args parsing for
%funcName:[args] uses simple split(',') which breaks on quoted strings/JSON and
lacks error handling; replace that split with a small robust parser (e.g.,
parseInlineArgs) that walks the arg string honoring quotes and bracket depth,
tokenizes correctly, then map each token through a resolver (resolveArg) that
handles !/# deref, @element.prop with nested property traversal, and strips
surrounding quotes for string literals; finally wrap the window[funcName]
invocation in a try/catch so any error is logged/handled and does not break the
pipeline, and call the parser/resolver before invoking the function.
| // Standard verb: e.g., ajax:url:GET | ||
| if (m = /^([a-zA-Z0-9_]+):?(.*)$/.exec(seg)) { | ||
| const verb = m[1]; | ||
| const params = m[2] ? m[2].split(':') : []; | ||
| if (typeof this.verbs[verb] === 'function') { | ||
| currentValue = await this.verbs[verb](...params); | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Colon-splitting for standard verbs breaks URLs (e.g., https://…).
params = m[2].split(':') splits scheme/host too. Use bracketed args like you did for %func (%verb:[...]) or split only the first colon. Here’s a backward-compatible tweak: treat [args] specially, else split once.
- const params = m[2] ? m[2].split(':') : [];
+ const rest = m[2] || '';
+ let params = [];
+ if (rest.startsWith('[') && rest.endsWith(']')) {
+ params = parseInlineArgs(rest.slice(1, -1), entry);
+ } else if (rest) {
+ const idx = rest.indexOf(':');
+ params = idx === -1 ? [rest] : [rest.slice(0, idx), rest.slice(idx + 1)];
+ }This preserves ajax:/api:x while allowing ajax:[ "https://ex.com/a?b=c", "GET" ].
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Standard verb: e.g., ajax:url:GET | |
| if (m = /^([a-zA-Z0-9_]+):?(.*)$/.exec(seg)) { | |
| const verb = m[1]; | |
| const params = m[2] ? m[2].split(':') : []; | |
| if (typeof this.verbs[verb] === 'function') { | |
| currentValue = await this.verbs[verb](...params); | |
| } | |
| continue; | |
| } | |
| // Standard verb: e.g., ajax:url:GET | |
| if (m = /^([a-zA-Z0-9_]+):?(.*)$/.exec(seg)) { | |
| const verb = m[1]; | |
| const rest = m[2] || ''; | |
| let params = []; | |
| if (rest.startsWith('[') && rest.endsWith(']')) { | |
| params = parseInlineArgs(rest.slice(1, -1), entry); | |
| } else if (rest) { | |
| const idx = rest.indexOf(':'); | |
| params = idx === -1 | |
| ? [rest] | |
| : [rest.slice(0, idx), rest.slice(idx + 1)]; | |
| } | |
| if (typeof this.verbs[verb] === 'function') { | |
| currentValue = await this.verbs[verb](...params); | |
| } | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In tree-view/dotpipe.js around lines 312 to 320, the current params parsing uses
m[2].split(':') which incorrectly splits URLs; change the logic to detect
bracketed args (e.g., starting with '[' and ending with ']') and parse that as a
single JSON/CSV-like argument list, otherwise split only on the first colon (or
use m[2].split(/:(.+)/) to get a single split) so existing shorthand like
ajax:/api:x remains unchanged while ajax:[ "...", "GET" ] and URLs like
https://... are preserved.
| // Built-in verbs (AJAX, log, etc.) | ||
| verbs: { | ||
| async ajax(url, method = 'GET') { | ||
| const res = await fetch(url, { method }); | ||
| const text = await res.text(); | ||
| return text; | ||
| }, | ||
| log(value) { | ||
| console.log(value); | ||
| return value; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve verbs.ajax robustness and surface failures.
Fetch without res.ok check returns error pages as content. Also propagate errors to let pipelines react.
async ajax(url, method = 'GET') {
- const res = await fetch(url, { method });
- const text = await res.text();
- return text;
+ const res = await fetch(url, { method });
+ if (!res.ok) {
+ const body = await res.text().catch(() => '');
+ throw new Error(`ajax(${method} ${url}) failed: ${res.status} ${res.statusText} ${body?.slice(0,200)}`);
+ }
+ return await res.text();
},Optionally add a json verb for convenience (verbs.json(url, method='GET') returning res.json()).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Built-in verbs (AJAX, log, etc.) | |
| verbs: { | |
| async ajax(url, method = 'GET') { | |
| const res = await fetch(url, { method }); | |
| const text = await res.text(); | |
| return text; | |
| }, | |
| log(value) { | |
| console.log(value); | |
| return value; | |
| } | |
| } | |
| // Built-in verbs (AJAX, log, etc.) | |
| verbs: { | |
| async ajax(url, method = 'GET') { | |
| const res = await fetch(url, { method }); | |
| if (!res.ok) { | |
| const body = await res.text().catch(() => ''); | |
| throw new Error(`ajax(${method} ${url}) failed: ${res.status} ${res.statusText} ${body?.slice(0,200)}`); | |
| } | |
| return await res.text(); | |
| }, | |
| log(value) { | |
| console.log(value); | |
| return value; | |
| } | |
| } |
🤖 Prompt for AI Agents
In tree-view/dotpipe.js around lines 326 to 337, the ajax verb currently returns
response text even for non-2xx responses and silently swallows network errors;
update ajax to check res.ok and throw an Error with status and body (or
statusText) when res.ok is false, and ensure any fetch/network exceptions are
allowed to bubble up so callers/pipelines can react; additionally add an
optional verbs.json(url, method='GET') that awaits fetch, checks res.ok
similarly, and returns await res.json() for convenience.
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Caution CodeRabbit will not generate a local commit with generated unit tests for a closed pull request. Try using copyable edits or generating a pull request. |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
2 similar comments
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Here are the copyable unit test edits: Copyable Editstests/dotpipe.test.jsThis is a new file. tests/inline-index.test.jsThis is a new file. |
|
Here are the copyable unit test edits: Copyable Editstests/dotpipeVariants.spec.jsThis is a new file. package.json@@ -4,7 +4,7 @@
"description": "HTML Extension Library Framework",
"main": "dotpipe.js",
"scripts": {
- "test": "modala(j.json, 'id-of-element');"
+ "test": "node --test"
},
"repository": {
"type": "git",test/package-json.test.jsThis is a new file. |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Here are the copyable unit test edits: Copyable Editstests/dotpipeModules.test.jsThis is a new file. |
|
Caution CodeRabbit timed out while planning changes for tests/doted/dotpipe.test.js. Skipping this file. |
|
Caution CodeRabbit timed out while planning changes for test/cart/dotpipe.test.js. Skipping this file. |
|
Caution CodeRabbit timed out while planning changes for tests/activeMenu/dotpipe.test.js. Skipping this file. |
|
Yikes, that's a lot of comments. o.0 |
Newly organized inline coding within the spectrum of the usage of dotPipe and the DOM primarily. You'll love it.
Summary by CodeRabbit
New Features
Documentation