Phoenix Code | Code Creatively

diff --git a/.github/workflows/prod-deploy.yml b/.github/workflows/prod-deploy.yml index 155cde1002..7fa96c00b5 100644 --- a/.github/workflows/prod-deploy.yml +++ b/.github/workflows/prod-deploy.yml @@ -1,4 +1,4 @@ -name: Deploy Notification To production phcode.dev +name: Deploy Notification To production web.phcode.dev # Please note to add branch protection rules for the prod branch in your repository. on: push: @@ -22,10 +22,10 @@ jobs: PRO_REPO_ACCESS_TOKEN: ${{ secrets.PRO_REPO_ACCESS_TOKEN }} run: | npm run release:prod - - name: Deploy Notification To production repository phcode.dev + - name: Deploy Notification To production repository web.phcode.dev uses: peter-evans/repository-dispatch@v2 with: token: ${{ secrets.PAT_PHOENIX_BOT_PUBLIC_REPO_ACCESS }} - repository: phcode-dev/phcode.dev + repository: phcode-dev/web.phcode.dev event-type: deploy-production client-payload: '{"source":"${{github.repositoryUrl}}", "workflow":"${{github.workflow}}", "run_id":"${{github.run_id}}", "run_number":"${{github.run_number}}", "ref": "${{ github.ref }}", "sha": "${{ github.sha }}"}' diff --git a/CLAUDE.md b/CLAUDE.md index be13693257..0f095c4618 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,7 +19,7 @@ - **`src/cacheManifest.json`** is a generated build artifact (gitignored, produced by `gulpfile.js/index.js`). It lists files + hashes for the service-worker cache. Never hand-edit or commit it — it is regenerated by the build, so edits are overwritten and won't be tracked anyway. When you add/remove/rename source files, just let the build regenerate it. ## Translations / i18n -- All user-visible strings must go in `src/nls/root/strings.js` — never hardcode English in source files. +- All user-visible strings must go in `src/nls/root/strings.js` — never hardcode English in source files. This applies only to genuinely translatable natural-language text. Content that must render identically in every locale — literal code syntax, keyword/identifier examples, brand names — is not translatable and must NOT go in strings.js; keep it as a local constant in the source file instead. Reason: `src/nls/root/strings.js` values are sent as-is to an automated AI translation pass (`gulpfile.js/translateStrings.js`) with no awareness that a given string represents code rather than prose, so a translatable-looking word embedded in code syntax (e.g. `name` in `function name() {...}`) can get mistranslated into garbled pseudo-code in other locales. - Use `const Strings = require("strings");` then `Strings.KEY_NAME`. - For parameterized strings use `StringUtils.format(Strings.KEY, arg0, arg1)` with `{0}`, `{1}` placeholders. - Keys use UPPER_SNAKE_CASE grouped by feature prefix (e.g. `AI_CHAT_*`). diff --git a/docs/API-Reference/editor/Editor.md b/docs/API-Reference/editor/Editor.md index 4db5c3420a..e139e3fbb8 100644 --- a/docs/API-Reference/editor/Editor.md +++ b/docs/API-Reference/editor/Editor.md @@ -1703,3 +1703,18 @@ Constant: Bulls-eye mode, strictly center the text always. ## CENTERING\_MARGIN **Kind**: global constant + + +## getMarkOptionTabstopOutline() +Mark option for a subdued outline box, used to show every remaining stop of an active +snippet/tab-stop session (see editor/TabstopManager.js) so the user can see at a glance how +many fields are left and where, even for the ones they haven't tabbed to yet. + +**Kind**: global function + + +## getMarkOptionTabstopOutlineActive() +Mark option for the bold/active variant of the above, layered on top of it for whichever stop +is currently selected in an active snippet/tab-stop session. + +**Kind**: global function diff --git a/package.json b/package.json index 8714283e69..41d182138a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "phoenix", - "version": "5.2.6-0", - "apiVersion": "5.2.6", + "version": "5.3.0-0", + "apiVersion": "5.3.0", "homepage": "https://core.ai", "issues": { "url": "https://github.com/phcode-dev/phoenix/issues" diff --git a/src-node/claude-code-agent.js b/src-node/claude-code-agent.js index f3f5394037..c81b7ae484 100644 --- a/src-node/claude-code-agent.js +++ b/src-node/claude-code-agent.js @@ -39,6 +39,64 @@ const CLARIFICATION_HINT = " IMPORTANT: The user has typed a follow-up clarification while you were working." + " Call the getUserClarification tool to read it before proceeding."; +// Nudge the model when it has edited files that render in the user's live +// preview without ever looking at the result. Deliberately phrased as an FYI +// the model may act on or ignore — whether a change is worth verifying, and +// with which tool, is its call. +// +// Stated as a count rather than "you just edited…" because the PostToolUse +// fallback path can deliver this a tool call after the edit, and because +// naming the number makes it read as a summary rather than a per-edit echo. +function _livePreviewHintText(count) { + return "FYI: " + count + " file(s) you edited are rendered in the user's live preview," + + " and you have not inspected it since. Decide for yourself whether looking is worth" + + " a tool call here — a trivial or self-evident change usually is not. If it is, you" + + " pick the tool: execJsInLivePreview to read the DOM / computed styles / console," + + " takeScreenshot with selector='#panel-live-preview-frame' for a visual check, or" + + " resizeLivePreview for responsive behavior."; +} + +// Reason returned when a file-rewriting shell command is stopped on its first +// attempt. A speed bump, not a wall: re-running the identical command goes +// through (see _shellEditNeedsConfirm). That protects the first edit — which a +// note after the fact cannot, since by then undo is already gone — while +// leaving the final call with the model, at the cost of one extra round trip. +// +// Preferring Edit/Write is a default, not a rule. Shell rewrites genuinely win +// on mechanical bulk changes and on large files where Edit would burn tokens +// re-reading to change a little, so the text asks the model to weigh that +// against the lost undo rather than treating the shell as forbidden. One round +// trip is negligible next to the bulk operation it is gating. +function _shellEditDenyText(what) { + return "Phoenix did not run that. It rewrites a file from the shell (" + what + "), which" + + " bypasses the editor: the user's open buffer is not refreshed, no reviewable diff is" + + " rendered, and the change cannot be undone from the AI panel's Undo button. For an" + + " ordinary content change, use Edit for existing files or Write for new ones — those" + + " keep all three. But this is a default, not a rule: if the shell is genuinely the" + + " better tool here — the user named this command, the change is mechanical across many" + + " files or matches, doing it with Edit would mean dozens of calls or reading a very" + + " large file to change a little of it, or the target is generated / build output / a" + + " log — then run it again unchanged and it will go through. Weigh the token cost" + + " against the user losing undo for that file, and tell them which way you went."; +} + +// Nudge on the first unverified live preview edit, then stay quiet until this +// many more pile up without the model ever looking at the preview. +const LP_NUDGE_REPEAT_AFTER = 5; + +// Hard ceiling per user request. Without it a long unverified run (30 edits) +// would emit ~6 nudges, and every one persists in the transcript. If two +// haven't changed the model's behavior, a third won't either. +const LP_MAX_NUDGES_PER_REQUEST = 2; + +// Calling any of these means the model is already looking at the preview, so +// there is nothing to nag about — seeing one resets the pending count. +const LP_INSPECT_TOOLS = [ + "mcp__phoenix-editor__takeScreenshot", + "mcp__phoenix-editor__execJsInLivePreview", + "mcp__phoenix-editor__resizeLivePreview" +]; + // Lazy-loaded ESM module reference let queryModule = null; @@ -103,11 +161,14 @@ let _planApproved = false; let _queuedClarification = null; // Module-level "runtime" permission mode that hooks read at decision time. -// Updated on every sendPrompt and via the setPermissionMode peer when the -// user cycles the panel's permission bar mid-stream — without this, the -// Bash hook would close over the value at query start and continue -// prompting for confirmation even after the user has flipped to Full Auto. -let _runtimePermissionMode = "acceptEdits"; +// One of "plan" | "acceptEdits" | "auto" (SDK classifier-approved) | +// "bypassPermissions" (Allow Everything). Updated on every sendPrompt and +// via the setPermissionMode peer when the user cycles the panel's +// permission bar mid-stream — without this, the Bash hook would close over +// the value at query start and continue prompting for confirmation even +// after the user has flipped to Allow Everything. Defaults to "auto" to +// match the browser's default (see AIChatPanel.js's _permissionMode). +let _runtimePermissionMode = "auto"; const nodeConnector = global.createNodeConnector(CONNECTOR_ID, exports); @@ -174,6 +235,119 @@ const _SAFE_BASH_PATTERNS = [ /^pnpm\s+--version$/ ]; +// Shell constructs whose purpose is rewriting a file in place. Bash is not +// interchangeable with Edit/Write here: the Edit/Write PostToolUse hooks +// refresh the open buffer, paint the diff card that backs the panel's Undo +// button, and carry the live preview signal. A shell rewrite skips all +// three, so the user silently loses undo for that change. +// +// A match stops the command once and offers a retry (see _shellEditDenyText), +// so the cost of a false positive is one wasted round trip rather than a +// refusal. Still worth keeping narrow: only constructs that exist to rewrite +// files belong here. +const _INPLACE_EDIT_PATTERNS = [ + // sed -i / -i.bak / -ri / --in-place. The lookahead stops at a pipe or + // separator so `grep -i x | sed 's/a/b/'` isn't caught by the grep flag. + { rx: /\bsed\b(?=[^|;&]*\s-(?:-in-place|[a-zA-Z]*i))/, what: "sed -i" }, + // perl -pi -e / perl -i.bak + { rx: /\bperl\b(?=[^|;&]*\s-[a-zA-Z]*i)/, what: "perl -i" }, + { rx: /\bawk\b(?=[^|;&]*\s-i\s+inplace)/, what: "awk -i inplace" }, + { rx: /\bed\s+-s\b/, what: "ed -s" }, + { rx: /\bex\s+-s(c|\s)/, what: "ex -s" }, + // PowerShell equivalents — on Windows the model may reach for these + // instead of sed. Set-Content/Add-Content/Out-File all rewrite a file. + { rx: /\b(?:Set-Content|Add-Content|Out-File)\b/i, what: "PowerShell Set-Content / Out-File" } +]; + +// Redirection / tee targets that aren't the user's files: device sinks and +// scratch dirs. `> /dev/null` and `> $TMPDIR/x` are ubiquitous and carry no +// undo cost, so hinting about them would be pure noise. +// +// Covers all three platforms, since the model may be driving bash, PowerShell +// or cmd depending on where Phoenix is running: macOS puts TMPDIR under +// /var/folders, Windows under %TEMP% / AppData\Local\Temp, and the null sink +// is /dev/null, NUL or $null respectively. +const _EXEMPT_WRITE_TARGETS = [ + /^\/dev\//, + /^\/proc\//, + /^\/(?:private\/)?tmp\//, + /^\/var\/(?:tmp|folders)\//, + /^(?:nul|\$null)$/i, + /^\$\{?TMPDIR\}?[\\/]/i, + /^%(?:TEMP|TMP)%[\\/]/i, + /^[a-zA-Z]:[\\/](?:temp|tmp)[\\/]/i, + // Git Bash rewrites C:\Temp to MSYS form (/c/temp), and it is the shell + // the Bash tool actually uses on Windows. + /^\/[a-zA-Z]\/(?:temp|tmp)\//i, + /[\\/]AppData[\\/]Local[\\/]Temp[\\/]/i, + /^[a-zA-Z]:[\\/]Windows[\\/]Temp[\\/]/i +]; + +function _isExemptWriteTarget(target) { + if (target === "-") { return true; } + return _EXEMPT_WRITE_TARGETS.some(function (rx) { return rx.test(target); }); +} + +// Walk the command tracking quote state so a `>` inside a string literal +// (`echo "a > b"`, `python -c "print(1 > 0)"`) is not mistaken for a +// redirection. Returns the write destinations found outside quotes. +// A heuristic guard, not a shell parser. +function _shellWriteTargets(rawCmd) { + // Drop file-descriptor duplications (2>&1, >&2, 1>&2) up front. + const cmd = (rawCmd || "").replace(/\d*>&\d*/g, " "); + const targets = []; + let quote = null; + for (let i = 0; i < cmd.length; i++) { + const ch = cmd[i]; + if (quote) { + if (ch === quote && cmd[i - 1] !== "\\") { quote = null; } + continue; + } + if (ch === "\"" || ch === "'") { quote = ch; continue; } + if (ch !== ">") { continue; } + // Skip the rest of a `>>` pair, then the whitespace before the target. + let j = i + 1; + while (cmd[j] === ">") { j++; } + while (cmd[j] === " " || cmd[j] === "\t") { j++; } + // Read the target, honouring quotes around a path with spaces. + let target = ""; + if (cmd[j] === "\"" || cmd[j] === "'") { + const closer = cmd[j]; + j++; + while (j < cmd.length && cmd[j] !== closer) { target += cmd[j++]; } + } else { + while (j < cmd.length && !/[\s;|&()]/.test(cmd[j])) { target += cmd[j++]; } + } + if (target) { targets.push(target); } + i = j - 1; + } + // tee writes to its path arguments rather than via redirection. + const tee = /\btee\s+(?:-a\s+)?("[^"]*"|'[^']*'|[^\s;|&()-][^\s;|&()]*)/g; + let m; + while ((m = tee.exec(cmd)) !== null) { + targets.push(m[1].replace(/^["']|["']$/g, "")); + } + return targets.filter(function (t) { return t && !_isExemptWriteTarget(t); }); +} + +/** + * Classify a Bash command that would rewrite file content instead of going + * through Edit/Write. Returns a short description of what was matched, or + * null when the command is fine to run. + */ +function _describeInPlaceFileEdit(rawCmd) { + const cmd = (rawCmd || "").trim(); + if (!cmd) { return null; } + for (const entry of _INPLACE_EDIT_PATTERNS) { + if (entry.rx.test(cmd)) { return entry.what; } + } + const targets = _shellWriteTargets(cmd); + if (targets.length) { + return "shell redirection to " + targets[0]; + } + return null; +} + function _isSafeReadOnlyBash(rawCmd) { const cmd = (rawCmd || "").trim(); if (!cmd) { return false; } @@ -661,7 +835,7 @@ exports.answerPlanModeWriteConfirm = async function (params) { * Apply a mid-stream permission-mode change so hooks running for the rest * of the turn use the new value. Called from the browser when the user * cycles the permission bar (so e.g. Bash stops prompting immediately - * after switching from Edit Mode to Full Auto). The next sendPrompt also + * after switching from Edit Mode to Allow Everything). The next sendPrompt also * passes permissionMode in params, so this peer is only strictly required * during streaming — but calling it on every cycle keeps the agent's * tracker authoritative. @@ -758,7 +932,7 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, // Sync the runtime mutable that hooks read for permission decisions — // setPermissionMode (peer) updates this same variable when the user // cycles modes mid-stream. - _runtimePermissionMode = permissionMode || "acceptEdits"; + _runtimePermissionMode = permissionMode || "auto"; let editCount = 0; let toolCounter = 0; // SDK tool_use id (e.g. "toolu_01...") → our sequential toolCounter so a @@ -769,6 +943,39 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, // turn skip the prompt and use the cached "allow" decision so a multi-edit // turn doesn't pop a dialog before every edit. let _planExitApprovedThisTurn = false; + // Live preview nudge bookkeeping, per request so each new user prompt + // re-arms it. _lpPendingEdits counts live-preview-related edits since the + // model last inspected the preview; _lpNudgeCount enforces the hard cap. + let _lpPendingEdits = 0; + let _lpNudgeCount = 0; + // Shell-rewrite confirmation, scoped per request rather than per + // conversation: a new user prompt is a new intent, so the next request's + // first rewrite gets its own speed bump instead of riding on a + // confirmation given for something else. + let _shellEditAwaitingRetry = null; + let _shellEditConfirmed = false; + + // True when this command should be stopped and offered a retry. The + // identical command coming back means it was meant, so it goes through — + // and having confirmed once, the rest of the request goes through too. + // + // That last part matters: the model often has to fix its own command after + // the first attempt (BSD `sed -i ''` failing on GNU sed, say). Keying only + // on the exact string charged a second bump for what is one operation, so + // one confirmation now covers the request. The first edit is still + // protected, which is the whole point of the bump. + function _shellEditNeedsConfirm(command) { + if (_shellEditConfirmed) { + return false; + } + if (_shellEditAwaitingRetry === command) { + _shellEditAwaitingRetry = null; + _shellEditConfirmed = true; + return false; + } + _shellEditAwaitingRetry = command; + return true; + } let queryFn; let connectionTimer = null; @@ -901,7 +1108,7 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, } }, mcpServers: { "phoenix-editor": editorMcpServer }, - permissionMode: permissionMode || "acceptEdits", + permissionMode: permissionMode || "auto", appendSystemPrompt: "When modifying an existing file, always prefer the Edit tool " + "(find-and-replace) instead of the Write tool. The Write tool should ONLY be used " + @@ -909,6 +1116,22 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, "multiple Edit calls to make targeted changes rather than rewriting the entire " + "file with Write. This is critical because Write replaces the entire file content " + "which is slow and loses undo history." + + "\n\nThe user's project root is " + (projectPath || process.cwd()) + ". For files " + + "under it, default to Edit and Write over shell rewrites (sed -i, perl -i, tee, " + + "Set-Content/Out-File, `>` / `>>` redirection). Phoenix routes Edit and Write " + + "through the editor, so they refresh the user's open buffer, render a reviewable " + + "diff, and stay undoable from the AI panel; a shell rewrite skips all three, and " + + "the user cannot undo it. Outside the project root — scratch files, temp output, " + + "logs — the shell is fine and needs no thought. " + + "\nThis is a default, not a prohibition. The shell is the better call when the " + + "change is mechanical across many files or matches, when Edit would mean dozens of " + + "calls or reading a large file to alter a little of it, or when the target is " + + "generated output. Phoenix stops the first shell rewrite of each command and " + + "explains why; re-run it unchanged and it goes through. Judge it on the merits — " + + "tokens saved against undo lost — and tell the user when you take the shell route. " + + "When the saving would be marginal, take Edit: one shell call and one Edit call " + + "cost about the same, so a handful of files is not a reason to give up undo. The " + + "shell has to earn it." + "\n\nALWAYS call getEditorState as your FIRST tool call on any question that " + "references the user's current work — not just \"what file am I on\". This includes " + "implicit-context questions like \"the page\", \"this layout\", \"the nav bar\", " + @@ -945,7 +1168,8 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, "phoenix-editor.resizeLivePreview, and phoenix-editor.controlEditor cover virtually " + "every \"look at / poke at the page\" need. Only fall back to chrome-devtools or " + "another browser MCP if the user explicitly asks for a non-Phoenix browser context. " + - "These tools are for active iteration, not just final verification:" + + "These tools are for active iteration AND for checking your own work — " + + "use them as you go, not only when the user asks:" + "\n- takeScreenshot: see the rendered HTML preview, the rendered Markdown preview, " + "the editor, or any panel. Use it to confirm visual output, diagnose layout/styling " + "bugs, or check that HTML or Markdown rendered as expected. Simple selector rule: " + @@ -957,7 +1181,7 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, "JS edits) — saves a tool call vs. reloading separately." + "\n- execJsInLivePreview: run JS inside the HTML preview iframe to read the DOM, " + "query computed styles, click elements, or capture console output. Use it to debug " + - "behavior, not just to verify." + + "behavior and to confirm an edit actually took effect." + "\n- resizeLivePreview: change the preview viewport width to test responsive " + "breakpoints." + "\n- controlEditor: open files, move the cursor, change selection, toggle the live " + @@ -982,6 +1206,15 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, "feature-docs URL and the GitHub source repo URL. Call once near the start of any " + "non-trivial editor-control task; then Read / Grep the apiDocsPath and WebFetch the " + "featureDocsURL as needed. Do NOT search the codebase blindly when this exists." + + "\n\nEDITS THAT LAND IN THE LIVE PREVIEW: when you edit the file getEditorState " + + "reported as livePreviewFile — or a CSS / JS / SVG file it links to — the user is " + + "watching the result render. Whether that is worth checking is your judgement call, " + + "and so is how: execJsInLivePreview to read the DOM / computed styles / console, " + + "takeScreenshot with selector='#panel-live-preview-frame' for a visual check, " + + "resizeLivePreview for responsive behavior, or nothing at all when the change is " + + "trivial or self-evident. Weigh it at meaningful checkpoints (after a section lands, " + + "before you report done) rather than after every small edit. Files outside the live " + + "preview do not raise the question at all." + "\n\nName-collision rule: \"Phoenix Code\" (the editor the user is sitting inside) " + "and \"Claude Code\" (the SDK / CLI you happen to run on) BOTH have settings, " + "configs, auto-update toggles, themes, etc. When the user says \"set / change / " + @@ -1286,17 +1519,45 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, matcher: "Bash", hooks: [ async (input) => { + // Stop a file rewrite the first time it is tried, + // in every permission mode — "auto" hands the call + // to the SDK classifier, which happily approves + // sed -i. Denying here is what actually protects + // the edit: a note after the fact arrives once undo + // is already gone. Re-running the same command + // confirms intent and goes through. + const command = (input.tool_input && input.tool_input.command) || ""; + const inPlaceEdit = _describeInPlaceFileEdit(command); + if (inPlaceEdit && _shellEditNeedsConfirm(command)) { + console.log("[Phoenix AI] Stopped shell file rewrite (" + + inPlaceEdit + "), offering retry: " + command.slice(0, 70)); + return { + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: _shellEditDenyText(inPlaceEdit) + } + }; + } + if (inPlaceEdit) { + console.log("[Phoenix AI] Shell file rewrite confirmed by retry: " + + command.slice(0, 70)); + } // Read from the runtime mutable so mid-stream // permission-mode flips (e.g. user switches Edit - // Mode → Full Auto while bash is in flight) take - // effect on the NEXT bash call without waiting - // for the next prompt. + // Mode → Allow Everything while bash is in flight) + // take effect on the NEXT bash call without + // waiting for the next prompt. if (_runtimePermissionMode !== "acceptEdits") { - // Plan mode: SDK handles. Full Auto: allow freely. + // Plan mode: SDK handles. Auto: SDK's own + // classifier decides. Allow Everything: allow + // freely. Either way, Phoenix's own + // confirm-dialog/safe-bash-allowlist below is + // only for Edit Mode's manual approval flow. return {}; } - // Edit Mode: ask user confirmation before running bash - const command = input.tool_input.command || ""; + // Edit Mode: ask user confirmation before running bash. + // `command` is read above, for the rewrite check. // Skip prompting for well-known read-only commands // that mirror the Claude Code CLI's default safe // patterns. Cuts down on prompt fatigue during @@ -1431,7 +1692,14 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, console.warn("[Phoenix AI] Edit refresh fallback failed:", filePath, err.message); } } - // 2. Trigger aiToolEdit so the AI panel renders the + // 2. Count it toward the live preview nudge. Only + // incrementing here — the read-and-clear happens + // in one owner, since PostToolUse hooks can run + // concurrently for parallel tool calls. + if (result.isLivePreviewRelated) { + _lpPendingEdits++; + } + // 3. Trigger aiToolEdit so the AI panel renders the // diff card and the snapshot store records it. const counterId = _toolUseIdToCounter[toolUseID]; if (counterId !== undefined) { @@ -1466,6 +1734,9 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, } catch (err) { console.warn("[Phoenix AI] Write refresh failed:", filePath, err.message); } + if (refreshResult.isLivePreviewRelated) { + _lpPendingEdits++; + } const counterId = _toolUseIdToCounter[toolUseID]; if (counterId !== undefined) { nodeConnector.triggerPeer("aiToolEdit", { @@ -1489,13 +1760,45 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, // tool. Edit/Write/Read have their own hooks above, but // any tool can be a meaningful checkpoint (Bash, Grep, // Glob, WebFetch, Task, the Phoenix MCP tools, etc.) so - // we register one matcher-less hook that just returns - // the clarification context if any is queued. Once + // we register one matcher-less hook that returns the + // clarification context if any is queued. Once // getUserClarification runs and clears _queuedClarification, - // _maybeClarifyContext returns {} and this becomes a no-op. + // that part becomes a no-op. + // + // It also carries the live preview nudge as a fallback for + // Claude CLI versions that predate PostToolBatch: the batch + // hook below is the primary path, but we run the user's + // global CLI (findGlobalClaudeCli) so we can't assume it. + // Whichever fires first takes the hint; the other sees a + // cleared counter. + hooks: [ + async (input) => { + return _buildPostToolUseHint(input); + } + ] + } + ], + PostToolBatch: [ + { + // Primary emit point for the live preview nudge. Fires once + // after every tool call in a batch resolves, so unlike + // PostToolUse (which may run concurrently for parallel tool + // calls) it can safely read-and-clear shared state, and it + // sees the whole batch — including whether the model already + // inspected the preview itself. hooks: [ - async () => { - return _maybeClarifyContext(); + async (input) => { + const names = (input.tool_calls || []).map(function (call) { + return call.tool_name; + }); + const hint = _takeLivePreviewHint(names); + if (!hint) { return {}; } + return { + hookSpecificOutput: { + hookEventName: "PostToolBatch", + additionalContext: hint + } + }; } ] } @@ -1503,17 +1806,59 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, } }; - // Returns a PostToolUse SyncHookJSONOutput that injects the clarification - // hint as additionalContext when the user has typed a follow-up while the - // AI is streaming. With our PreToolUse hooks now returning {} (allow), the - // old practice of appending CLARIFICATION_HINT to permissionDecisionReason - // no longer reaches Claude — PostToolUse additionalContext is the new path. - function _maybeClarifyContext() { - if (!_queuedClarification) { return {}; } + // Read-and-clear for the live preview nudge. Returns the hint text when the + // model has piled up unverified live-preview edits, else null. Called from + // the PostToolBatch hook (primary) and the PostToolUse catch-all (fallback + // for older CLIs) — the body is synchronous, so whichever gets here first + // takes the hint and the other finds the counter already cleared. + // + // toolNames is what the model just called: seeing it inspect the preview + // itself means there is nothing to nag about. + function _takeLivePreviewHint(toolNames) { + if (toolNames && toolNames.some(function (name) { + return LP_INSPECT_TOOLS.indexOf(name) !== -1; + })) { + _lpPendingEdits = 0; + return null; + } + if (_lpNudgeCount >= LP_MAX_NUDGES_PER_REQUEST) { + return null; + } + const threshold = _lpNudgeCount === 0 ? 1 : LP_NUDGE_REPEAT_AFTER; + if (_lpPendingEdits < threshold) { + return null; + } + const text = _livePreviewHintText(_lpPendingEdits); + console.log("[Phoenix AI] live preview nudge:", _lpPendingEdits, "edit(s) unverified"); + _lpPendingEdits = 0; + _lpNudgeCount++; + return text; + } + + // Returns a PostToolUse SyncHookJSONOutput carrying whatever the model + // should see after a tool call: the clarification hint when the user has + // typed a follow-up while the AI is streaming, and/or the live preview + // nudge. With our PreToolUse hooks now returning {} (allow), the old + // practice of appending CLARIFICATION_HINT to permissionDecisionReason no + // longer reaches Claude — PostToolUse additionalContext is the new path. + // + // _queuedClarification is deliberately not cleared here; it clears only + // when the model calls getUserClarification. The live preview counter is + // cleared by _takeLivePreviewHint, so that half cannot repeat. + function _buildPostToolUseHint(input) { + const parts = []; + if (_queuedClarification) { + parts.push(CLARIFICATION_HINT); + } + const lpHint = _takeLivePreviewHint(input && input.tool_name ? [input.tool_name] : null); + if (lpHint) { + parts.push(lpHint); + } + if (!parts.length) { return {}; } return { hookSpecificOutput: { hookEventName: "PostToolUse", - additionalContext: CLARIFICATION_HINT + additionalContext: parts.join("\n\n") } }; } diff --git a/src-node/package-lock.json b/src-node/package-lock.json index 14ca2e2f48..9cbf71d1f0 100644 --- a/src-node/package-lock.json +++ b/src-node/package-lock.json @@ -1,12 +1,12 @@ { "name": "@phcode/node-core", - "version": "5.2.4-0", + "version": "5.3.0-0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@phcode/node-core", - "version": "5.2.4-0", + "version": "5.3.0-0", "hasInstallScript": true, "license": "GNU-AGPL3.0", "dependencies": { diff --git a/src-node/package.json b/src-node/package.json index 998a1914b9..b6b8ff9346 100644 --- a/src-node/package.json +++ b/src-node/package.json @@ -1,8 +1,8 @@ { "name": "@phcode/node-core", "description": "Phoenix Node Core", - "version": "5.2.6-0", - "apiVersion": "5.2.6", + "version": "5.3.0-0", + "apiVersion": "5.3.0", "keywords": [], "author": "arun@core.ai", "homepage": "https://github.com/phcode-dev/phoenix", diff --git a/src/LiveDevelopment/MultiBrowserImpl/documents/LiveHTMLDocument.js b/src/LiveDevelopment/MultiBrowserImpl/documents/LiveHTMLDocument.js index 9686ad405c..195e11c444 100644 --- a/src/LiveDevelopment/MultiBrowserImpl/documents/LiveHTMLDocument.js +++ b/src/LiveDevelopment/MultiBrowserImpl/documents/LiveHTMLDocument.js @@ -61,6 +61,9 @@ define(function (require, exports, module) { this._onChange = this._onChange.bind(this); this.doc.on("change", this._onChange); + this._onDeleted = this._onDeleted.bind(this); + this.doc.on("deleted", this._onDeleted); + this._onRelated = this._onRelated.bind(this); this.protocol.on("DocumentRelated", this._onRelated); @@ -152,6 +155,7 @@ define(function (require, exports, module) { LiveHTMLDocument.prototype.close = function () { this.doc.releaseRef(); this.doc.off("change", this._onChange); + this.doc.off("deleted", this._onDeleted); this.protocol.off("DocumentRelated", this._onRelated); this.protocol.off("StylesheetAdded", this._onStylesheetAdded); this.protocol.off("StylesheetRemoved", this._onStylesheetRemoved); @@ -161,6 +165,19 @@ define(function (require, exports, module) { this.parentClass.close.call(this); }; + /** + * @private + * Handles the backing Document's file being deleted from disk (eg. from the file tree). + * Without this, our extra addRef() on the doc (on top of any editor's own ref) would never + * be released - close() is otherwise only called by the live preview session lifecycle, not + * in response to the file disappearing. + * @param {$.Event} event + */ + LiveHTMLDocument.prototype._onDeleted = function (event) { + this.close(); + this.trigger("deleted", [this]); + }; + /** * @override * Update the highlights in the browser based on the cursor position. diff --git a/src/brackets.config.dist.json b/src/brackets.config.dist.json index 0e8dfcc466..1a8800fe19 100644 --- a/src/brackets.config.dist.json +++ b/src/brackets.config.dist.json @@ -1,9 +1,9 @@ { - "googleAnalyticsID" : "G-FBK9RP5YK2", + "googleAnalyticsID" : "G-G3DTS3ZR09", "googleAnalyticsIDDesktop": "G-M7MX9BYZZ3", "mixPanelID" : "8cb6814f733e37c05cc59b4adad26407", "coreAnalyticsID" : "phoenix", - "coreAnalyticsAppName" : "phoenix-prod", + "coreAnalyticsAppName" : "phoenix-web", "coreAnalyticsAppNameDesktop" : "desktop-prod", "environment" : "production", "buildtype" : "production", diff --git a/src/config.json b/src/config.json index f378d48bf7..40aac1d4aa 100644 --- a/src/config.json +++ b/src/config.json @@ -51,8 +51,8 @@ "bugsnagEnv": "development" }, "name": "Phoenix Code", - "version": "5.2.6-0", - "apiVersion": "5.2.6", + "version": "5.3.0-0", + "apiVersion": "5.3.0", "homepage": "https://core.ai", "issues": { "url": "https://github.com/phcode-dev/phoenix/issues" diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index fa50fdfeff..3a418a63e9 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -41,6 +41,7 @@ define(function (require, exports, module) { FileUtils = require("file/FileUtils"), FileViewController = require("project/FileViewController"), InMemoryFile = require("document/InMemoryFile"), + EncodingDetector = require("document/EncodingDetector"), StringUtils = require("utils/StringUtils"), Async = require("utils/Async"), Metrics = require("utils/Metrics"), @@ -509,22 +510,66 @@ define(function (require, exports, module) { }); var file = FileSystem.getFileForPath(fullPath); + + function _openFileInPane() { + MainViewManager._open(paneId, file, options) + .done(function () { + result.resolve(file); + }) + .fail(function (fileError) { + _showErrorAndCleanUp(fileError, fullPath); + result.reject(); + }); + } + + // File.read() caches _contents/_stat keyed together with whatever _encoding was in + // effect at the time of that read (see File.js). Bare-reassigning file._encoding + // without invalidating that cache would let the imminent real open - which reads with + // this same newly-assigned encoding - incorrectly cache-hit and hand back stale + // content cached under the OLD encoding (worse, raw bytes, if that prior read used a + // byte-array encoding, eg via the "Download" command or an image-attach feature) + // instead of doing a real re-read. Always route encoding changes through here so that + // can never happen. + function _setFileEncoding(newEncoding) { + if (file._encoding !== newEncoding) { + file._clearCachedData(); + file._encoding = newEncoding; + } + } + if (options && options.encoding) { - file._encoding = options.encoding; + _setFileEncoding(options.encoding); + _openFileInPane(); } else { const encoding = PreferencesManager.getViewState("encoding", PreferencesManager.STATE_PROJECT_CONTEXT); if (encoding && encoding[fullPath]) { - file._encoding = encoding[fullPath]; + _setFileEncoding(encoding[fullPath]); + _openFileInPane(); + } else if (EncodingDetector.isKnownTextEncoding(file._encoding)) { + // File instances are cached/reused per path for the session (FileSystem._index), + // so a known-text _encoding here means we've already read (and so already + // detected or defaulted) this exact file once before as text - eg it's being + // reopened after a close. Re-running detection would mean re-reading the whole + // file from disk a second time for no new information, so just reuse what we + // already know. (isKnownTextEncoding - rather than a plain truthiness check - + // matters here: other code paths read this same File instance for non-text + // reasons, eg downloading it or attaching it as a chat image, and can leave a + // non-text sentinel encoding cached on it even though it was never opened as a + // document - we must not mistake that for "already detected".) + _openFileInPane(); + } else { + // No explicit, previously chosen, or already-known encoding for this file - see if + // it self-declares a non-UTF-8 charset (eg a legacy HTML file with + // ) so we don't silently and irreversibly corrupt it + // by force-decoding as UTF-8. See EncodingDetector. + EncodingDetector.detectFileEncoding(file).then(function (detectedEncoding) { + // Always land on a definite, known-text value - never leave file._encoding as + // whatever a prior non-text read (see above) may have left it as. + _setFileEncoding(detectedEncoding || "utf8"); + _openFileInPane(); + }); } } - MainViewManager._open(paneId, file, options) - .done(function () { - result.resolve(file); - }) - .fail(function (fileError) { - _showErrorAndCleanUp(fileError, fullPath); - result.reject(); - }); } return result.promise(); diff --git a/src/document/EncodingDetector.js b/src/document/EncodingDetector.js new file mode 100644 index 0000000000..6761e4ae7d --- /dev/null +++ b/src/document/EncodingDetector.js @@ -0,0 +1,268 @@ +/* + * GNU AGPL-3.0 License + * + * Copyright (c) 2021 - present core.ai . All rights reserved. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License + * for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://opensource.org/licenses/AGPL-3.0. + * + */ + +/*global fs*/ + +/** + * Phoenix always decodes newly opened files as UTF-8 by default (see File.js/AppshellFileSystem.js). + * That's correct for the vast majority of files, but some files - most commonly HTML/XML documents + * authored a long time ago, or exported by legacy tools (old FrontPage/Dreamweaver, Windows editors, + * etc) - are actually encoded in a legacy 8-bit charset like `windows-1252`, and self-declare that + * fact via a ``/`Content-Type` tag. Force-decoding such a file as UTF-8 doesn't just + * look wrong - the browser's `TextDecoder("utf8")` is non-fatal, so every undecodable byte is + * silently and *irreversibly* replaced with U+FFFD ("<27>") the moment the file is read. Once that + * happens the original bytes are gone; if the user then saves, the corruption is baked into the file + * on disk too. + * + * This module lets us catch that case before the first (lossy) read ever happens, so newly opened + * files get decoded with the encoding they actually declare - the same behavior a web browser + * exhibits when honoring a page's own declared charset. + * + * Two independent signals are checked, from strongest to weakest: + * 1. A byte-order-mark (BOM) - an unambiguous, extension-independent signal, so it's checked for + * any non-binary file (see detectFileEncoding's use of LanguageManager.isBinary()). This also + * means files like plain .txt with a genuine UTF-16/UTF-32 BOM now get decoded correctly on + * first open too, which - surprisingly - nothing did automatically before this module existed; + * previously that required manually picking the encoding from the status bar dropdown. + * 2. A self-declared charset (``/`Content-Type`) - only meaningful for markup file + * extensions (see SNIFFABLE_EXTENSIONS), and only trusted when the raw bytes are NOT already + * valid UTF-8. If they are, we trust that over any declaration - this avoids second-guessing + * modern UTF-8 files that simply have a stale/incorrect meta tag left over from a copy-paste. + * There is no BOM equivalent for single-byte legacy charsets like windows-1252 - a document + * declaring itself is the only signal there is. + * + * Either way, this is only ever used for a fresh, first-time open. Once a user has explicitly + * picked an encoding for a path (via the status bar dropdown), that choice always wins - see + * DocumentCommandHandlers. + */ +define(function (require, exports, module) { + + + const FileUtils = require("file/FileUtils"), + LanguageManager = require("language/LanguageManager"); + + /** + * File extensions for which we attempt to sniff a self-declared charset. + * @type {Array.} + */ + const SNIFFABLE_EXTENSIONS = ["html", "htm", "xhtml", "shtml", "php", "xml"]; + + // The HTML5 spec only requires user agents to scan the first 1024 bytes of a document for a + // charset declaration before starting to parse it; we use the same limit here. + const SNIFF_BYTE_LIMIT = 1024; + + // Matches both `` and + // `` style declarations. + const META_CHARSET_RE = /]+charset\s*=\s*["']?\s*([a-zA-Z0-9_\-:.]+)/i; + + // Charset aliases that browsers commonly treat as equivalent to a related, better-supported + // name (keyed/valued by the normalized form - see _normalizeEncodingName). Per the WHATWG + // encoding spec, content labeled iso-8859-1 is treated as windows-1252 in practice, since + // windows-1252 is a strict superset (it just assigns printable characters to the 0x80-0x9F + // range that iso-8859-1 leaves as C1 control codes, which real-world content essentially + // never intentionally uses). + const CHARSET_ALIASES = { + "latin1": "windows1252", + "iso88591": "windows1252" + }; + + /** + * @private + * Normalizes a charset name the same way Phoenix's underlying iconv-lite based fs layer does + * when looking up a codec (lower-cased, non-alphanumeric characters stripped) - eg + * "windows-1252" and "Windows_1252" both become "windows1252". `fs.SUPPORTED_ENCODINGS` is + * itself a list of already-normalized names, so declared charsets must be normalized the same + * way before being compared against it or handed back as the encoding to decode with. + * @param {string} name + * @return {string} + */ + function _normalizeEncodingName(name) { + return name.toLowerCase().replace(/[^0-9a-z]/g, ""); + } + + const BOM_SIGNATURES = [ + {bytes: [0xEF, 0xBB, 0xBF], encoding: "utf8"}, + {bytes: [0xFF, 0xFE, 0x00, 0x00], encoding: "utf32le"}, + {bytes: [0x00, 0x00, 0xFE, 0xFF], encoding: "utf32be"}, + {bytes: [0xFF, 0xFE], encoding: "utf16le"}, + {bytes: [0xFE, 0xFF], encoding: "utf16be"} + ]; + + /** + * @private + * Returns the encoding named by a recognized byte-order-mark at the start of `bytes`, or null. + * @param {Uint8Array} bytes + * @return {?string} + */ + function _detectBOM(bytes) { + for (const sig of BOM_SIGNATURES) { + if (bytes.length >= sig.bytes.length && sig.bytes.every(function (b, i) { + return bytes[i] === b; + })) { + return sig.encoding; + } + } + return null; + } + + /** + * @private + * Extracts a declared charset name from a `` tag, if present in the given bytes. The + * declaration itself is always plain ASCII per spec, so it's safe to scan for it by treating + * the raw bytes as Latin-1/ASCII regardless of the file's real encoding. + * @param {Uint8Array} bytes + * @return {?string} lower-cased charset name, or null if none found + */ + function _extractDeclaredCharset(bytes) { + const prefix = bytes.subarray(0, Math.min(bytes.length, SNIFF_BYTE_LIMIT)); + let asciiText = ""; + for (let i = 0; i < prefix.length; i++) { + asciiText += String.fromCharCode(prefix[i]); + } + const match = META_CHARSET_RE.exec(asciiText); + return match ? match[1].toLowerCase() : null; + } + + /** + * @private + * @param {Uint8Array} bytes + * @return {boolean} true if `bytes` is well-formed UTF-8 + */ + function _isValidUTF8(bytes) { + try { + new TextDecoder("utf8", {fatal: true}).decode(bytes); + return true; + } catch (e) { + return false; + } + } + + /** + * Given the raw bytes of a file and its extension, determine whether a non-default encoding + * should be used to decode it. Pure/synchronous - does no I/O. + * + * @param {string} extension lower-case file extension, no leading dot + * @param {Uint8Array} bytes raw file content + * @param {Array.=} supportedEncodings encoding names Phoenix's fs layer can decode with; + * defaults to `fs.SUPPORTED_ENCODINGS` when running in the app. + * @return {?string} the encoding name to use, or null to keep the default (utf8) + */ + function detectEncodingFromBytes(extension, bytes, supportedEncodings) { + if (!bytes || !bytes.length) { + return null; + } + + const bom = _detectBOM(bytes); + if (bom) { + // An explicit BOM is unambiguous. A utf-8 BOM just means "definitely utf-8", which is + // already our default, so nothing to override there. + return bom === "utf8" ? null : bom; + } + + if (SNIFFABLE_EXTENSIONS.indexOf(extension) === -1) { + return null; + } + + if (_isValidUTF8(bytes)) { + return null; + } + + const rawDeclared = _extractDeclaredCharset(bytes); + if (!rawDeclared) { + return null; + } + + let declared = _normalizeEncodingName(rawDeclared); + declared = CHARSET_ALIASES[declared] || declared; + + if (declared === "utf8") { + // Declared utf-8 but isn't valid utf-8 bytes - nothing sane we can substitute, so we + // just keep decoding as utf-8 (matching today's behavior) rather than guessing further. + return null; + } + + supportedEncodings = supportedEncodings || (typeof fs !== "undefined" && fs.SUPPORTED_ENCODINGS); + if (supportedEncodings && supportedEncodings.indexOf(declared) === -1) { + return null; + } + + return declared; + } + + /** + * Attempts to detect a non-default encoding for `file` before it's opened for the first time, + * by reading its raw bytes and looking for a BOM or (for markup files) a self-declared charset + * (see detectEncodingFromBytes). Never rejects - resolves with null if detection isn't + * applicable to this file, or nothing conclusive was found, in which case the caller should + * just fall back to the normal default (utf8) decode. + * + * We only skip reading the file at all when it's a known binary type (image, font, zip, etc) - + * same check the rest of the app uses (LanguageManager's isBinary()) to decide whether a file + * should ever be treated as text. Anything else is fair game for a BOM, even if its extension + * isn't one we scan for a `` declaration (see SNIFFABLE_EXTENSIONS) - a BOM is a + * cheap, unambiguous signal that doesn't depend on file type the way a meta tag scan does. + * + * @param {File} file + * @return {$.Promise} resolved with the detected encoding name, or null + */ + function detectFileEncoding(file) { + const result = new $.Deferred(); + const language = LanguageManager.getLanguageForPath(file.fullPath); + + if (language.isBinary()) { + result.resolve(null); + return result.promise(); + } + + const extension = FileUtils.getFileExtension(file.fullPath).toLowerCase(); + file.read({encoding: window.fs.BYTE_ARRAY_ENCODING, doNotCache: true}, function (err, content) { + if (err || !content) { + result.resolve(null); + return; + } + const bytes = new Uint8Array(content); + result.resolve(detectEncodingFromBytes(extension, bytes, window.fs.SUPPORTED_ENCODINGS)); + }); + + return result.promise(); + } + + /** + * True if `encoding` is a real text codec name, as opposed to the non-text sentinel value + * (`fs.BYTE_ARRAY_ENCODING`, i.e. "byte_array" - notably still present in + * `fs.SUPPORTED_ENCODINGS`, so that list alone can't be used to tell them apart) that plenty of + * *other* call sites across the codebase pass to `File.read()` for legitimate non-text reasons + * (downloading a file, attaching an image, exporting a zip, etc) - and, unless they also pass + * `doNotCache: true`, leave cached in `file._encoding` as a side effect of File.read()'s + * caching (see File.js). A File instance touched that way before ever being opened as a + * document would otherwise look "already known" to a naive truthiness check, silently + * defeating both detection and the re-open-skip optimization in DocumentCommandHandlers. + * @param {?string} encoding + * @return {boolean} + */ + function isKnownTextEncoding(encoding) { + return !!encoding && encoding !== window.fs.BYTE_ARRAY_ENCODING; + } + + exports.SNIFFABLE_EXTENSIONS = SNIFFABLE_EXTENSIONS; + exports.detectEncodingFromBytes = detectEncodingFromBytes; + exports.detectFileEncoding = detectFileEncoding; + exports.isKnownTextEncoding = isKnownTextEncoding; +}); diff --git a/src/editor/Editor.js b/src/editor/Editor.js index b105553689..8e0b9b2b84 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -1396,6 +1396,37 @@ define(function (require, exports, module) { }; } + /** + * Mark option for a subdued outline box, used to show every remaining stop of an active + * snippet/tab-stop session (see editor/TabstopManager.js) so the user can see at a glance how + * many fields are left and where, even for the ones they haven't tabbed to yet. + */ + function getMarkOptionTabstopOutline() { + return { + className: "editor-text-tabstop-outline", + startStyle: "editor-text-tabstop-outline-left", + endStyle: "editor-text-tabstop-outline-right", + clearWhenEmpty: false, + inclusiveLeft: true, + inclusiveRight: true + }; + } + + /** + * Mark option for the bold/active variant of the above, layered on top of it for whichever stop + * is currently selected in an active snippet/tab-stop session. + */ + function getMarkOptionTabstopOutlineActive() { + return { + className: "editor-text-tabstop-outline-active", + startStyle: "editor-text-tabstop-outline-active-left", + endStyle: "editor-text-tabstop-outline-active-right", + clearWhenEmpty: false, + inclusiveLeft: true, + inclusiveRight: true + }; + } + /** * Mark option to underline errors. */ @@ -1430,6 +1461,8 @@ define(function (require, exports, module) { * Mark option for renaming outlines. */ Editor.getMarkOptionRenameOutline = getMarkOptionRenameOutline; + Editor.getMarkOptionTabstopOutline = getMarkOptionTabstopOutline; + Editor.getMarkOptionTabstopOutlineActive = getMarkOptionTabstopOutlineActive; /** * Can be used to mark a range of text with a specific CSS class name. cursorFrom and cursorTo should be {line, ch} diff --git a/src/editor/TabstopManager.js b/src/editor/TabstopManager.js index ee4907d5fa..5fd62842ad 100644 --- a/src/editor/TabstopManager.js +++ b/src/editor/TabstopManager.js @@ -35,11 +35,19 @@ * stop, and (when there is more than one stop) starts a Tab-navigable session backed by markers * so the stops follow any later edits (e.g. an auto-import line inserted above). * - * NOTE: this is currently wired only into the LSP completion path (languageTools/DefaultProviders). - * The Emmet expander (HTMLCodeHints) and the custom-snippets feature have their own stable cursor - * handling and were intentionally left untouched; they can migrate onto this manager in future. + * Used by the LSP completion path (languageTools/DefaultProviders), DocCommentHints, and Custom + * Snippets (extensionsIntegrated/CustomSnippets/snippetCursorManager.js). The Emmet expander + * (HTMLCodeHints) still has its own separate cursor handling. + * + * While a Tab-navigable session is active, every remaining stop gets a subdued outline box (see + * Editor.getMarkOptionTabstopOutline) so the user can see at a glance how many fields are left and + * where, with the currently-selected one getting a bolder "active" outline layered on top (see + * Editor.getMarkOptionTabstopOutlineActive) - matches the visual language RenameIdentifier.js already + * uses for its own outline box. Zero-width stops (a bare `$N`/`$0` with no default text - just a + * caret position, no marker range) don't get an outline, since there's no span to box. */ define(function (require, exports, module) { + const Editor = require("editor/Editor").Editor; /** * Expand an LSP snippet into plain text plus the list of tab-stops. @@ -174,7 +182,7 @@ define(function (require, exports, module) { // ---- Tab-navigation session ---------------------------------------------------------------- - var _session = null; // { editor, markers: [marker], index, keymap } + var _session = null; // { editor, markers: [marker], index, keymap, activeOutlineMarker } function _clearSession() { if (!_session) { @@ -185,10 +193,49 @@ define(function (require, exports, module) { session.markers.forEach(function (m) { m.clear(); }); + if (session.activeOutlineMarker) { + session.activeOutlineMarker.clear(); + } session.editor._codeMirror.removeKeyMap(session.keymap); session.editor.off(".tabstop"); } + /** + * @param {{line: number, ch: number}} pos - a document position + * @return {boolean} true if `pos` falls within the line span currently covered by the active + * session's markers (i.e. the snippet the user is still tabbing through) + */ + function _isWithinSessionBounds(pos) { + var minLine = Infinity, + maxLine = -Infinity; + _session.markers.forEach(function (m) { + var r = _markerRange(m); + if (r) { + minLine = Math.min(minLine, r.from.line); + maxLine = Math.max(maxLine, r.to.line); + } + }); + if (minLine === Infinity) { + return false; // no markers left resolve-able + } + return pos.line >= minLine && pos.line <= maxLine; + } + + /** + * Ends the session as soon as the user's cursor leaves the snippet's lines (e.g. clicks + * elsewhere to fix something unrelated) or a multi-cursor selection is made - matches standard + * editor behavior (VS Code et al.) and avoids a stray later Tab press unexpectedly jumping the + * cursor back into a snippet the user has moved on from. + */ + function _handleCursorActivity(event, editor) { + if (!_session || _session.editor !== editor) { + return; + } + if (editor.getSelections().length > 1 || !_isWithinSessionBounds(editor.getCursorPos())) { + _clearSession(); + } + } + /** * Resolve a marker (markText range or bookmark) to a {from, to} document range, or null if the * marker no longer exists in the document. @@ -215,10 +262,28 @@ define(function (require, exports, module) { } _session.index = index; _session.editor.setSelection(range.from, range.to); + + // swap the bold "active" outline onto whichever stop we just landed on - only meaningful for + // a real span (a bare $N/$0 with no default text is a zero-width caret, nothing to box) + if (_session.activeOutlineMarker) { + _session.activeOutlineMarker.clear(); + _session.activeOutlineMarker = null; + } + if (range.from.line !== range.to.line || range.from.ch !== range.to.ch) { + _session.activeOutlineMarker = _session.editor.markText( + "tabstop-active", range.from, range.to, Editor.getMarkOptionTabstopOutlineActive()); + } return true; } function _gotoNext() { + if (!_session) { + // no-op: goToNextStop/goToPreviousStop are exported as public API (see bottom of file) + // for callers like Custom Snippets' snippetCursorManager.js to drive navigation directly, + // not only via the CodeMirror keymap installed below (which only exists while a session is + // active, so it could never reach this function with no session) - a direct caller could. + return; + } // Move forward through the stops; leaving the last one ends the session (caret stays put). for (var i = _session.index + 1; i < _session.markers.length; i++) { if (_selectStop(i)) { @@ -233,6 +298,9 @@ define(function (require, exports, module) { } function _gotoPrev() { + if (!_session) { + return; // see _gotoNext's no-op comment - same reasoning applies here + } for (var i = _session.index - 1; i >= 0; i--) { if (_selectStop(i)) { return; @@ -286,6 +354,11 @@ define(function (require, exports, module) { } // Multiple stops: lay down markers and start a Tab-navigable session. + // the subdued outline (visual only) is layered onto the SAME functional tracking options + // below by className/startStyle/endStyle alone - deliberately not spreading the whole helper + // object in, since its own inclusiveLeft/clearWhenEmpty differ from what marker TRACKING here + // actually needs (inclusiveLeft: false is what makes typing at a stop's start not stick to it). + var outlineOption = Editor.getMarkOptionTabstopOutline(); var markers = parsed.stops.map(function (stop) { var ms = posFromOffset(stop.start), me = posFromOffset(stop.end); @@ -295,7 +368,10 @@ define(function (require, exports, module) { return editor.markText("tabstop", ms, me, { clearWhenEmpty: false, inclusiveLeft: false, - inclusiveRight: true + inclusiveRight: true, + className: outlineOption.className, + startStyle: outlineOption.startStyle, + endStyle: outlineOption.endStyle }); }); @@ -312,11 +388,13 @@ define(function (require, exports, module) { } }; - _session = { editor: editor, markers: markers, index: -1, keymap: keymap }; + _session = { editor: editor, markers: markers, index: -1, keymap: keymap, activeOutlineMarker: null }; editor._codeMirror.addKeyMap(keymap); - // End the session if the editor it belongs to is destroyed (file closed). Namespaced so - // _clearSession can remove it with a single off(".tabstop"). + // End the session if the editor it belongs to is destroyed (file closed), or the user moves + // on (cursor leaves the snippet's lines, or a multi-cursor selection is made). Namespaced so + // _clearSession can remove both with a single off(".tabstop"). editor.on("beforeDestroy.tabstop", _clearSession); + editor.on("cursorActivity.tabstop", _handleCursorActivity); _selectStop(0); return parsed; @@ -338,4 +416,8 @@ define(function (require, exports, module) { exports.insertSnippet = insertSnippet; exports.hasActiveSession = hasActiveSession; exports.endSession = endSession; + // exposed so other features with their own stable session lifecycle (e.g. custom snippets) + // can drive Tab / Shift-Tab navigation without duplicating this logic + exports.goToNextStop = _gotoNext; + exports.goToPreviousStop = _gotoPrev; }); diff --git a/src/extensionsIntegrated/CustomSnippets/codeHintIntegration.js b/src/extensionsIntegrated/CustomSnippets/codeHintIntegration.js index 2159fe8d4b..cdd422f8ee 100644 --- a/src/extensionsIntegrated/CustomSnippets/codeHintIntegration.js +++ b/src/extensionsIntegrated/CustomSnippets/codeHintIntegration.js @@ -23,7 +23,6 @@ define(function (require, exports, module) { const EditorManager = require("editor/EditorManager"); const Metrics = require("utils/Metrics"); - const Global = require("./global"); const Driver = require("./driver"); const Helper = require("./helper"); const SnippetCursorManager = require("./snippetCursorManager"); @@ -84,7 +83,8 @@ define(function (require, exports, module) { if (matchingSnippets.length > 0) { const customSnippetHints = matchingSnippets.map((snippet) => { - return Helper.createHintItem(snippet.abbreviation, needle.word, snippet.description); + return Helper.createHintItem( + snippet.abbreviation, needle.word, snippet.description, snippet.insertionKey); }); return { @@ -108,34 +108,33 @@ define(function (require, exports, module) { insertHint: function (hint) { // check if the hint is a custom snippet if (hint && hint.jquery && hint.attr("data-isCustomSnippet")) { - // handle custom snippet insertion - const abbreviation = hint.attr("data-val"); - if (Global.SnippetHintsList) { - const matchedSnippet = Global.SnippetHintsList.find( - (snippet) => snippet.abbreviation === abbreviation - ); + // handle custom snippet insertion. The hint list was already built from the correctly + // language-scoped candidates (see getHints above), and each hint element carries the + // exact resolved snippet's insertionKey - so accepting it is a direct O(1) lookup, not + // a re-search by abbreviation + the (possibly since-changed) current language context + const insertionKey = hint.attr("data-insertion-key"); + + // Get current editor from EditorManager since it's not passed + const editor = EditorManager.getActiveEditor(); + if (editor) { + const matchedSnippet = Helper.getSnippetByInsertionKey(insertionKey); if (matchedSnippet) { - // Get current editor from EditorManager since it's not passed - const editor = EditorManager.getActiveEditor(); - - if (editor) { - // to track the usage metrics - const fileCategory = Helper.categorizeFileExtensionForMetrics(matchedSnippet.fileExtension); - Metrics.countEvent(Metrics.EVENT_TYPE.EDITOR, "snipt", `use.${fileCategory}`); - - // replace the typed abbreviation with the template text using cursor manager - const wordInfo = Driver.getWordBeforeCursor(); - const start = { line: wordInfo.line, ch: wordInfo.ch + 1 }; - const end = editor.getCursorPos(); - - SnippetCursorManager.insertSnippetWithTabStops( - editor, - matchedSnippet.templateText, - start, - end - ); - return true; // handled - } + // to track the usage metrics + const fileCategory = Helper.categorizeFileExtensionForMetrics(matchedSnippet.fileExtension); + Metrics.countEvent(Metrics.EVENT_TYPE.EDITOR, "snipt", `use.${fileCategory}`); + + // replace the typed abbreviation with the template text using cursor manager + const wordInfo = Driver.getWordBeforeCursor(); + const start = { line: wordInfo.line, ch: wordInfo.ch + 1 }; + const end = editor.getCursorPos(); + + SnippetCursorManager.insertSnippetWithTabStops( + editor, + matchedSnippet.templateText, + start, + end + ); + return true; // handled } } } diff --git a/src/extensionsIntegrated/CustomSnippets/defaultSnippets.js b/src/extensionsIntegrated/CustomSnippets/defaultSnippets.js new file mode 100644 index 0000000000..38e38ebdd6 --- /dev/null +++ b/src/extensionsIntegrated/CustomSnippets/defaultSnippets.js @@ -0,0 +1,114 @@ +/* + * GNU AGPL-3.0 License + * + * Copyright (c) 2021 - present core.ai . All rights reserved. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License + * for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://opensource.org/licenses/AGPL-3.0. + * + */ + +define(function (require, exports, module) { + // INDENT is templateText's own indent-unit marker (see snippetCursorManager.js INDENT_TOKEN) - + // resolved to this specific file's actual detected/configured indent (spaces or tabs, whatever + // width) at insertion time, instead of hardcoding a literal " " that would look wrong the + // moment a user's file uses 2-space indent, tabs, etc. + const INDENT = require("./snippetCursorManager").INDENT_TOKEN; + + // These are literal code syntax shown verbatim in the hint tooltip, not natural-language prose - + // there is nothing in them for a translator to translate, so per the i18n rule in CLAUDE.md they + // are local constants here rather than strings.js keys (only genuinely translatable strings belong + // in strings.js; content that must render identically in every locale - like code syntax - does + // not, and should never be sent through the automated AI translation pass). + const FUNCTION_DESC = "function name() {...}"; + const ARROW_DESC = "const name = () => {...}"; + const PYTHON_FUNCTION_DESC = "def name(): ..."; + + /** + * Built-in snippets shipped with Phoenix (see https://github.com/phcode-dev/phoenix/issues/618). + * + * These are NOT persisted to the user's customSnippets.json and NOT part of `Global.SnippetHintsList` + * at all - they're merged into the matching engine's optimized structures directly (see helper.js + * `rebuildOptimizedStructures`), which is also why they never appear in the Custom Snippets panel + * (snippetsList.js/driver.js only ever read/write `Global.SnippetHintsList`, so a built-in is simply + * invisible to add/edit/delete there) and can't be user-edited or deleted. Because they're always + * re-derived straight from this file on every boot, template/description improvements in a later + * Phoenix release reach every user immediately - there's nothing to keep in sync. + * + * `isDefault: true` marks an entry as one of these built-ins (as opposed to a user-created snippet) + * wherever the merged/optimized snippet objects are inspected. + * + * `prefixTrigger: true` lets the hint pop up as soon as the user has typed a leading prefix of + * `abbreviation` (2+ chars - see helper.js `hasExactMatchingSnippet`), not only once the whole word + * is typed - e.g. typing "fu"/"fun"/"func" all offer the "function" entry. Regular user-created + * snippets never get this flag, so their exact-match-only behavior is unaffected. + */ + const DEFAULT_SNIPPETS = [ + { + id: "default-function", + isDefault: true, + abbreviation: "function", + prefixTrigger: true, + description: FUNCTION_DESC, + templateText: + "function ${1:name}(${2}) {\n" + + INDENT + "${0}\n" + + "}", + fileExtension: ".js, .jsx, .ts, .tsx" + }, + { + id: "default-arrow-function", + isDefault: true, + abbreviation: "arrow", + prefixTrigger: true, + description: ARROW_DESC, + templateText: + "const ${1:name} = (${2}) => {\n" + + INDENT + "${0}\n" + + "};", + fileExtension: ".js, .jsx, .ts, .tsx" + }, + { + // PHP genuinely uses the same `function` keyword as JS - deliberately shares the same + // abbreviation, scoped to .php only. Requires hasExactMatchingSnippet to check ALL + // same-named candidates per language (see helper.js) rather than a single Map winner. + id: "default-function-php", + isDefault: true, + abbreviation: "function", + prefixTrigger: true, + description: FUNCTION_DESC, + templateText: + "function ${1:name}(${2}) {\n" + + INDENT + "${0}\n" + + "}", + fileExtension: ".php" + }, + { + // Python has no braces - the body is indentation-scoped under the colon-terminated + // `def` line, so this needs its own trigger word ("def") and template shape entirely. + // Unlike JS/PHP, an empty body here is a real syntax error (IndentationError), so the + // final stop defaults to "pass" (selected, ready to type over) instead of being empty. + id: "default-function-python", + isDefault: true, + abbreviation: "def", + prefixTrigger: true, + description: PYTHON_FUNCTION_DESC, + templateText: + "def ${1:name}(${2}):\n" + + INDENT + "${0:pass}", + fileExtension: ".py" + } + ]; + + exports.DEFAULT_SNIPPETS = DEFAULT_SNIPPETS; +}); diff --git a/src/extensionsIntegrated/CustomSnippets/helper.js b/src/extensionsIntegrated/CustomSnippets/helper.js index 43713dcda5..89fd7d71ab 100644 --- a/src/extensionsIntegrated/CustomSnippets/helper.js +++ b/src/extensionsIntegrated/CustomSnippets/helper.js @@ -23,6 +23,7 @@ define(function (require, exports, module) { const Global = require("./global"); const UIHelper = require("./UIHelper"); const Strings = require("strings"); + const DefaultSnippets = require("./defaultSnippets"); // list of all the navigation and function keys that are allowed inside the input fields const ALLOWED_NAVIGATION_KEYS = [ @@ -56,7 +57,13 @@ define(function (require, exports, module) { // Optimized data structures for fast snippet lookups let snippetsByLanguage = new Map(); let snippetsByAbbreviation = new Map(); + let snippetsByInsertionKey = new Map(); let allSnippetsOptimized = []; + // small dedicated subset of allSnippetsOptimized (only built-in defaults ever set prefixTrigger - + // see defaultSnippets.js), scanned on every keystroke by hasExactMatchingSnippet below. Kept + // separate from allSnippetsOptimized so that scan's cost never grows with the user's own + // (potentially much larger) custom snippet count. + let prefixTriggerSnippets = []; /** * Preprocesses a snippet to add optimized lookup properties @@ -69,6 +76,12 @@ define(function (require, exports, module) { // pre-compute lowercase abbreviation for faster matching optimizedSnippet.abbreviationLower = snippet.abbreviation.toLowerCase(); + // a stable key identifying this exact snippet for insertion (see findSnippetForInsertion / + // getSnippetByInsertionKey) - built-ins use their unique `id`; regular user snippets don't + // have one, but driver.js already enforces globally-unique abbreviations for those on add, so + // the abbreviation itself is a safe stable key for them + optimizedSnippet.insertionKey = snippet.id || snippet.abbreviation; + // parse and create a Set of supported extensions for O(1) lookup if (snippet.fileExtension.toLowerCase() === "all") { optimizedSnippet.supportedLangSet = new Set(["all"]); @@ -90,20 +103,44 @@ define(function (require, exports, module) { * Rebuilds optimized data structures from the current snippet list * we call this function whenever snippets are loaded, added, modified, or deleted * i.e. whenever the snippetList is updated + * + * This is also where built-in default snippets (see defaultSnippets.js) get merged into the + * matching engine - they are NOT part of Global.SnippetHintsList and are never persisted/shown + * in the panel, they only exist here, in this optimized/derived view. */ function rebuildOptimizedStructures() { // clear existing structures snippetsByLanguage.clear(); snippetsByAbbreviation.clear(); + snippetsByInsertionKey.clear(); allSnippetsOptimized.length = 0; + prefixTriggerSnippets.length = 0; - // Process each snippet - Global.SnippetHintsList.forEach(snippet => { + // Process each snippet - user snippets first, so a user snippet that happens to share an + // abbreviation with a default is index-order-first (relevant only for iteration order, since + // matching itself checks every candidate for language support regardless of order) + Global.SnippetHintsList.concat(DefaultSnippets.DEFAULT_SNIPPETS).forEach(snippet => { const optimizedSnippet = preprocessSnippet(snippet); allSnippetsOptimized.push(optimizedSnippet); - // Index by abbreviation (lowercase) for exact matches - snippetsByAbbreviation.set(optimizedSnippet.abbreviationLower, optimizedSnippet); + if (optimizedSnippet.prefixTrigger) { + prefixTriggerSnippets.push(optimizedSnippet); + } + + // O(1) lookup by stable identity for insertion - see findSnippetForInsertion. Collisions + // aren't expected (built-ins key by unique `id`; user snippets key by their own + // abbreviation, which driver.js already enforces is unique among user snippets on add) - + // if one somehow occurs, last one indexed wins, same as any other Map.set. + snippetsByInsertionKey.set(optimizedSnippet.insertionKey, optimizedSnippet); + + // Index by abbreviation (lowercase) for exact matches. Multiple snippets CAN share the + // same abbreviation (e.g. "function" for both JS and PHP) as long as they're scoped to + // different, non-overlapping languages - so this maps to an array of candidates, not a + // single winner, and hasExactMatchingSnippet below checks all of them. + if (!snippetsByAbbreviation.has(optimizedSnippet.abbreviationLower)) { + snippetsByAbbreviation.set(optimizedSnippet.abbreviationLower, []); + } + snippetsByAbbreviation.get(optimizedSnippet.abbreviationLower).push(optimizedSnippet); // Index by supported languages/extensions if (optimizedSnippet.supportsAllLanguages) { @@ -366,24 +403,66 @@ define(function (require, exports, module) { return false; } + // Minimum characters the user must type before a `prefixTrigger` snippet (see defaultSnippets.js) + // is allowed to fire on a partial/in-progress prefix of its abbreviation, instead of only once + // fully typed. Guards against 1-character noise (e.g. every word starting with "f"). + const MIN_PREFIX_TRIGGER_LENGTH = 2; + /** - * Checks if there's at least one exact match for the query + * Checks if there's at least one matching snippet for the query - either an exact abbreviation + * match, or, for snippets opted into `prefixTrigger` (see defaultSnippets.js), a leading prefix + * of their abbreviation once the user has typed at least MIN_PREFIX_TRIGGER_LENGTH characters. + * Regular user-created snippets never set `prefixTrigger`, so they keep requiring an exact match, + * unaffected by this. + * + * Multiple snippets can share the same abbreviation across different languages (e.g. "function" + * for both JS and PHP) - every candidate for a given abbreviation/prefix is checked against the + * current language context, so one language's entry never shadows another's. * @param {string} query - The search query * @param {Editor} editor - The editor instance - * @returns {boolean} - True if there's an exact match + * @returns {boolean} - True if there's a matching snippet */ function hasExactMatchingSnippet(query, editor) { const queryLower = query.toLowerCase(); const languageContext = getCurrentLanguageContext(editor); - const snippet = snippetsByAbbreviation.get(queryLower); - if (snippet) { - return isSnippetSupportedInLanguageContext(snippet, languageContext, editor); + const exactCandidates = snippetsByAbbreviation.get(queryLower); + if (exactCandidates && exactCandidates.some((snippet) => + isSnippetSupportedInLanguageContext(snippet, languageContext, editor))) { + return true; + } + + if (queryLower.length >= MIN_PREFIX_TRIGGER_LENGTH) { + // scoped to the small prefixTriggerSnippets subset (built-ins only), not the user's full + // (potentially much larger) snippet list - see its declaration for why + const hasPrefixMatch = prefixTriggerSnippets.some((snippet) => + snippet.abbreviationLower.startsWith(queryLower) && + isSnippetSupportedInLanguageContext(snippet, languageContext, editor) + ); + if (hasPrefixMatch) { + return true; + } } return false; } + /** + * Looks up the exact snippet to insert by its insertionKey (see preprocessSnippet) - an O(1) Map + * lookup, not a re-derivation of language context. The hint list shown to the user was already + * built from the correctly language-scoped candidates (getMatchingSnippets/hasExactMatchingSnippet + * run at hint-display time); each rendered hint element carries the exact resolved snippet's + * insertionKey (see createHintItem), so accepting a hint just needs to look that key up directly - + * whatever was shown is exactly what gets inserted, with no risk of re-resolving to a different + * snippet than the one actually displayed (e.g. if the cursor's language context could ever change + * between the hint being shown and being accepted). + * @param {string} insertionKey - the `data-insertion-key` carried by the accepted hint element + * @returns {Object|null} the matching snippet, or null if not found + */ + function getSnippetByInsertionKey(insertionKey) { + return snippetsByInsertionKey.get(insertionKey) || null; + } + /** * Gets all snippets that match the query (prefix matches) * @param {string} query - The search query @@ -450,14 +529,21 @@ define(function (require, exports, module) { * @param {String} abbr - the abbreviation text that is to be displayed in the code hint * @param {String} query - the query string typed by the user for highlighting matching characters * @param {String} description - the description of the snippet to be displayed + * @param {String} [insertionKey] - the exact snippet's insertionKey (see preprocessSnippet), + * carried on the element so insertHint can look it up directly (getSnippetByInsertionKey) + * instead of re-resolving by abbreviation + current language context at accept time * @returns {JQuery} - the jquery item that has the abbr text and the Snippet icon */ - function createHintItem(abbr, query, description) { + function createHintItem(abbr, query, description, insertionKey) { var $hint = $("") .addClass("brackets-css-hints brackets-hints custom-snippets-hint") .attr("data-val", abbr) .attr("data-isCustomSnippet", true); + if (insertionKey !== undefined && insertionKey !== null) { + $hint.attr("data-insertion-key", insertionKey); + } + // add the tooltip for the description shown when the hint is hovered if (description && description.trim() !== "") { $hint.attr("title", description.trim()); @@ -917,6 +1003,7 @@ define(function (require, exports, module) { exports.isSnippetSupportedInLanguageContext = isSnippetSupportedInLanguageContext; exports.isSnippetSupportedInFile = isSnippetSupportedInFile; exports.hasExactMatchingSnippet = hasExactMatchingSnippet; + exports.getSnippetByInsertionKey = getSnippetByInsertionKey; exports.getMatchingSnippets = getMatchingSnippets; exports.sanitizeFileExtensionInput = sanitizeFileExtensionInput; exports.handleFileExtensionInput = handleFileExtensionInput; diff --git a/src/extensionsIntegrated/CustomSnippets/main.js b/src/extensionsIntegrated/CustomSnippets/main.js index e88122f448..7b1b843cc3 100644 --- a/src/extensionsIntegrated/CustomSnippets/main.js +++ b/src/extensionsIntegrated/CustomSnippets/main.js @@ -290,7 +290,9 @@ define(function (require, exports, module) { _addToMenu(); CodeHintIntegration.init(); - // load snippets from file storage + // load snippets from file storage. Built-in default snippets (see defaultSnippets.js) are + // NOT part of this user data - they're merged directly into the matching engine's optimized + // structures (see helper.js rebuildOptimizedStructures), so they never touch this file. const _snippetsLoadedPromise = SnippetsState.loadSnippetsFromState() .then(function () { // track boot-time snippet count (only if user has snippets) @@ -304,8 +306,6 @@ define(function (require, exports, module) { logger.reportError(error, "Custom Snippets: didn't load on app init"); }); - SnippetCursorManager.registerHandlers(); - // Expose modules for integration testing if (brackets.test) { brackets.test.CustomSnippetsGlobal = Global; @@ -313,6 +313,7 @@ define(function (require, exports, module) { brackets.test.CustomSnippetsCursorManager = SnippetCursorManager; brackets.test.CustomSnippetsCodeHintHandler = CodeHintIntegration._CustomSnippetsHandler; brackets.test.CustomSnippetsDriver = Driver; + brackets.test.CustomSnippetsState = SnippetsState; brackets.test._customSnippetsLoadedPromise = _snippetsLoadedPromise; } }); diff --git a/src/extensionsIntegrated/CustomSnippets/snippetCursorManager.js b/src/extensionsIntegrated/CustomSnippets/snippetCursorManager.js index a7ce3de810..ee71c83a8d 100644 --- a/src/extensionsIntegrated/CustomSnippets/snippetCursorManager.js +++ b/src/extensionsIntegrated/CustomSnippets/snippetCursorManager.js @@ -20,124 +20,76 @@ define(function (require, exports, module) { const KeyEvent = require("utils/KeyEvent"); - const EditorManager = require("editor/EditorManager"); - - // tab stops regex to handle ${1}, ${2}.... etc. - const TAB_STOP_REGEX = /\$\{(\d+)\}/g; - - // this is to check whether an active snippet session is on or off - let activeSnippetSession = null; + const TabstopManager = require("editor/TabstopManager"); + const Editor = require("editor/Editor").Editor; /** - * this represents an active snippet session with tab stops + * Marker a snippet's templateText can use to mean "one indent level, in whatever this specific + * file/editor is actually configured/detected to use" (spaces vs tabs, and how many) - resolved by + * resolveIndentToken below, entirely within this module's own preprocessing, before the text ever + * reaches TabstopManager's shared LSP-grammar parser. That's deliberate: TabstopManager is also + * used directly by LSP completions and DocCommentHints (see editor/TabstopManager.js), and this + * marker is never taught to THAT shared parser at all - by the time TabstopManager sees the text, + * this token has already been fully replaced with literal characters, so there is nothing here for + * it to interpret, and real LSP-served snippet text (which never passes through this module) can + * never trigger this substitution either. Deliberately NOT `$`-prefixed, so it can never collide + * with real `${...}` tab-stop/placeholder syntax even if this substitution were ever skipped - it + * would just show up as this literal, obviously-wrong-looking text instead of silently misbehaving. */ - function SnippetSession(editor, tabStops, startLine, endLine) { - this.editor = editor; - this.tabStops = tabStops; // this is an array of {number, line} sorted by number - this.currentTabNumber = tabStops.length > 0 ? tabStops[0].number : 1; - this.startLine = startLine; - this.endLine = endLine; - this.isActive = true; - } + const INDENT_TOKEN = "@@INDENT@@"; /** - * this function is responsible to parse the template text and extract all the tab stops + * Resolves what "one indent level" literally looks like right now for the given editor's file - + * same auto-detection + project/language preference cascade Phoenix's own Tab-key handling uses + * (see Editor.getUseTabChar/getSpaceUnits), so it always matches what pressing Tab in that file + * would actually insert. * - * @param {string} templateText - the template text with tab stops - * @returns {Object} - Object containing the text and tab stop information + * @param {Editor} editor - the editor instance being inserted into + * @returns {string} - e.g. " " or "\t", scoped to this specific file/language/project */ - function parseTemplateText(templateText) { - const tabStops = []; - let match; - - // reset regex - TAB_STOP_REGEX.lastIndex = 0; - - // find all the tab stops - while ((match = TAB_STOP_REGEX.exec(templateText)) !== null) { - const tabNumber = parseInt(match[1], 10); - tabStops.push({ - number: tabNumber - }); - } - - // sort the tab stops by number. note: 0 should come at last - tabStops.sort((a, b) => { - if (a.number === 0) { - return 1; - } - if (b.number === 0) { - return -1; - } - return a.number - b.number; - }); - - return { - text: templateText, - tabStops: tabStops - }; + function getOneIndentUnit(editor) { + const fullPath = editor && editor.document && editor.document.file && editor.document.file.fullPath; + return Editor.getUseTabChar(fullPath) ? "\t" : " ".repeat(Editor.getSpaceUnits(fullPath)); } /** - * Find tab stops in the snippet lines and return their positions - * this is called after snippet insertion to find actual positions in the editor + * Replaces every INDENT_TOKEN in templateText with the current editor's actual one-indent-level + * string. See INDENT_TOKEN's own doc comment for why this is a plain string substitution done here + * rather than new tab-stop syntax taught to the shared TabstopManager parser. * - * @param {Editor} editor - editor instance - * @param {number} startLine - Start line of snippet - * @param {number} endLine - End line of snippet - * @returns {Array} - array of {number, line, start, end} sorted by number + * @param {string} templateText - the raw template text, may contain zero or more INDENT_TOKENs + * @param {Editor} editor - the editor instance being inserted into + * @returns {string} - templateText with every INDENT_TOKEN replaced */ - function findTabStops(editor, startLine, endLine) { - const tabStops = []; - const document = editor.document; - - for (let line = startLine; line <= endLine; line++) { - const lineText = document.getLine(line); - let match; - - TAB_STOP_REGEX.lastIndex = 0; - while ((match = TAB_STOP_REGEX.exec(lineText)) !== null) { - const tabNumber = parseInt(match[1], 10); - tabStops.push({ - number: tabNumber, - line: line, - start: { line: line, ch: match.index }, - end: { line: line, ch: match.index + match[0].length } - }); - } + function resolveIndentToken(templateText, editor) { + if (templateText.indexOf(INDENT_TOKEN) === -1) { + return templateText; // fast path - most snippets (all user-authored ones, today) skip this } - - tabStops.sort((a, b) => { - if (a.number === 0) { - return 1; - } - if (b.number === 0) { - return -1; - } - return a.number - b.number; - }); - - return tabStops; + const unit = getOneIndentUnit(editor); + return templateText.split(INDENT_TOKEN).join(unit); } /** - * responsible to check if session should continue (tab stops still exist in template area) - * we need this because users can delete tab stops while typing + * Custom snippet templateText historically only ever recognized the braced form `${1}` as a + * tab stop (regex `/\$\{(\d+)\}/g`) - a bare `$1`, `$scope`, `$5`, etc. was always just literal + * text. TabstopManager understands the fuller LSP snippet grammar (bare `$1` tab stops, `${VAR}` + * variables that get silently dropped if unresolved, `${1:default}` placeholders, `\$`/`\}`/`\\` + * escapes). To keep every already-saved snippet behaving exactly as before after this migration, + * we escape every '$' that isn't immediately starting a `${...}` group before handing the text to + * TabstopManager - this way only the braced forms are ever treated as snippet syntax, exactly + * matching the old engine's behavior, while additively allowing `${1:default text}` and + * `${1|a,b,c|}` for anyone (including our own default snippets) who wants richer placeholders. * - * @returns {boolean} + * @param {string} text - the raw template text + * @returns {string} - text with any bare (non-`${`) '$' escaped as '\$' */ - function shouldContinueSession() { - if (!activeSnippetSession || !activeSnippetSession.isActive) { - return false; - } - - const session = activeSnippetSession; - const tabStops = findTabStops(session.editor, session.startLine, session.endLine); - - // update the session with current tab stops - session.tabStops = tabStops; - - return tabStops.length > 0; + function escapeBareDollarSigns(text) { + // escape pre-existing literal backslashes first, so they aren't misread as introducing a + // \$, \}, \\ escape sequence once the next step injects backslashes next to '$' characters + let escaped = text.replace(/\\/g, "\\\\"); + // escape every '$' not immediately followed by '{' + escaped = escaped.replace(/\$(?!\{)/g, "\\$"); + return escaped; } /** @@ -202,333 +154,102 @@ define(function (require, exports, module) { * @param {Object} endPos - End position for insertion */ function insertSnippetWithTabStops(editor, templateText, startPos, endPos) { - const parsed = parseTemplateText(templateText); + // Resolve any INDENT_TOKEN to this file's actual indent unit first, so everything downstream + // just sees plain literal characters - see resolveIndentToken's doc comment for why this must + // happen before escaping/parsing, not as new syntax taught to the shared TabstopManager parser. + const withIndentResolved = resolveIndentToken(templateText, editor); + + const escapedText = escapeBareDollarSigns(withIndentResolved); // Get the current line's indentation to apply to all subsequent lines const baseIndent = getLineIndentation(editor, startPos); // Apply proper indentation to the snippet text for multi-line snippets - const indentedText = addIndentationToSnippet(parsed.text, baseIndent); + const indentedText = addIndentationToSnippet(escapedText, baseIndent); - editor.document.replaceRange(indentedText, startPos, endPos); - - // calculate snippet bounds - const lines = indentedText.split("\n"); - const startLine = startPos.line; - const endLine = startPos.line + lines.length - 1; - - // find tab stops in the inserted snippet - const tabStops = findTabStops(editor, startLine, endLine); - - if (tabStops.length > 0) { - activeSnippetSession = new SnippetSession(editor, tabStops, startLine, endLine); - - // move to first tab stop. this is the default behaviour - navigateToTabStop(activeSnippetSession.currentTabNumber); - } else { - // when no tab stops, we just place cursor at end - const finalPos = { - line: endLine, - ch: lines.length === 1 ? startPos.ch + lines[0].length : lines[lines.length - 1].length - }; - editor.setCursorPos(finalPos); - } + return TabstopManager.insertSnippet(editor, indentedText, startPos, endPos); } /** - * Navigate to a specific tab stop by number - * @param {number} tabNumber - Tab stop number to navigate to + * Check if we're currently in a snippet session + * @returns {boolean} */ - function navigateToTabStop(tabNumber) { - if (!shouldContinueSession()) { - endSnippetSession(); - return; - } - - const session = activeSnippetSession; - - // find the tab stop with the specified number - const tabStop = session.tabStops.find((t) => t.number === tabNumber); - - if (tabStop) { - session.currentTabNumber = tabNumber; + function isInSnippetSession() { + return TabstopManager.hasActiveSession(); + } - // select the entire tab stop placeholder - session.editor.setSelection(tabStop.start, tabStop.end); - session.editor.focus(); - } else { - endSnippetSession(); - } + /** + * End the current snippet session + */ + function endSnippetSession() { + TabstopManager.endSession(); } /** * Navigate to the next tab stop - * this handles the logic for finding the next available tab stop in sequence + * @returns {boolean} true if a session was active and navigation happened */ function navigateToNextTabStop() { - if (!shouldContinueSession()) { - endSnippetSession(); + if (!TabstopManager.hasActiveSession()) { return false; } - - const session = activeSnippetSession; - const currentNumber = session.currentTabNumber; - - let nextTabStop = null; - - // If we're currently at ${0}, there's no next tab stop so we need to end the session - if (currentNumber === 0) { - endSnippetSession(); - return false; - } - - // at first, look for the next numbered tab stop (greater than current) - for (let i = 0; i < session.tabStops.length; i++) { - if (session.tabStops[i].number > currentNumber && session.tabStops[i].number !== 0) { - nextTabStop = session.tabStops[i]; - break; - } - } - - // If no numbered tab stop found, look for ${0} as the final stop - if (!nextTabStop) { - nextTabStop = session.tabStops.find((t) => t.number === 0); - } - - if (nextTabStop) { - navigateToTabStop(nextTabStop.number); - return true; - } - endSnippetSession(); - return false; + TabstopManager.goToNextStop(); + return true; } /** * Navigate to the previous tab stop - * this handles shift+tab navigation to go backwards + * @returns {boolean} true if a session was active and navigation happened */ function navigateToPreviousTabStop() { - if (!shouldContinueSession()) { - endSnippetSession(); - return false; - } - - const session = activeSnippetSession; - const currentNumber = session.currentTabNumber; - - // Find the previous tab stop number in the sorted array - let prevTabStop = null; - - // If we're currently at ${0}, find the highest numbered tab stop - if (currentNumber === 0) { - let maxNumber = -1; - for (let i = 0; i < session.tabStops.length; i++) { - if (session.tabStops[i].number !== 0 && session.tabStops[i].number > maxNumber) { - maxNumber = session.tabStops[i].number; - prevTabStop = session.tabStops[i]; - } - } - } else { - // Find the previous numbered tab stop (less than current, but not 0) - for (let i = session.tabStops.length - 1; i >= 0; i--) { - if (session.tabStops[i].number < currentNumber && session.tabStops[i].number !== 0) { - prevTabStop = session.tabStops[i]; - break; - } - } - } - - if (prevTabStop) { - navigateToTabStop(prevTabStop.number); - return true; - } - return false; - } - - /** - * End the current snippet session - * this cleans up all remaining tab stop placeholders and resets the session - */ - function endSnippetSession() { - if (activeSnippetSession) { - const session = activeSnippetSession; - - // Remove any remaining tab stop placeholders - const tabStops = findTabStops(session.editor, session.startLine, session.endLine); - tabStops.reverse().forEach((tabStop) => { - session.editor.document.replaceRange("", tabStop.start, tabStop.end); - }); - - activeSnippetSession.isActive = false; - activeSnippetSession = null; - } - } - - /** - * Check if we're currently in a snippet session - * @returns {boolean} - */ - function isInSnippetSession() { - return activeSnippetSession && activeSnippetSession.isActive; - } - - /** - * Check if cursor is within snippet lines - * we need this to end the session if user moves cursor outside the snippet area - * - * @param {Object} cursorPos - Current cursor position - * @returns {boolean} - */ - function isCursorInSnippetLines(cursorPos) { - if (!activeSnippetSession) { + if (!TabstopManager.hasActiveSession()) { return false; } - - return cursorPos.line >= activeSnippetSession.startLine && cursorPos.line <= activeSnippetSession.endLine; + TabstopManager.goToPreviousStop(); + return true; } /** - * Handle key events for tab navigation - * this is where all the tab/shift+tab/escape key handling happens + * Handle key events for tab navigation. + * NOTE: real Tab/Shift-Tab/Esc handling during an active session is now owned by + * TabstopManager's own CodeMirror keymap (installed per-session in insertSnippet). This + * function is kept only as a thin compatibility shim for callers/tests that dispatch a + * synthesized key event directly instead of going through the real DOM/CodeMirror path. * - * @param {Event} jqEvent - jQuery event + * @param {Event} jqEvent - jQuery event (unused, kept for signature compatibility) * @param {Editor} editor - Editor instance * @param {KeyboardEvent} event - Keyboard event */ function handleKeyEvent(jqEvent, editor, event) { - if (!isInSnippetSession() || activeSnippetSession.editor !== editor) { - return false; - } - - // make sure that the cursor is still within snippet lines - const cursorPos = editor.getCursorPos(); - if (!isCursorInSnippetLines(cursorPos)) { - endSnippetSession(); + if (!TabstopManager.hasActiveSession()) { return false; } - // Tab key handling if (event.keyCode === KeyEvent.DOM_VK_TAB) { - if (event.shiftKey) { - // Shift+Tab: go to previous tab stop - if (navigateToPreviousTabStop()) { - event.preventDefault(); - return true; - } - } else { - // Tab: go to next tab stop - if (navigateToNextTabStop()) { - event.preventDefault(); - return true; - } + const moved = event.shiftKey ? navigateToPreviousTabStop() : navigateToNextTabStop(); + if (moved) { + event.preventDefault(); + return true; } } - // 'Esc' key to end snippet session if (event.keyCode === KeyEvent.DOM_VK_ESCAPE) { endSnippetSession(); event.preventDefault(); return true; } - // handle Delete/Backspace - check if session should continue - // we need this because users might delete the template text from the editor - if (event.keyCode === KeyEvent.DOM_VK_DELETE || event.keyCode === KeyEvent.DOM_VK_BACK_SPACE) { - // just to let the delete/backspace complete - setTimeout(() => { - if (!shouldContinueSession()) { - endSnippetSession(); - } - }, 10); - } - return false; } - /** - * Handle cursor position changes - * this ends the session if user moves cursor outside snippet bounds or creates multiple selections - * @param {Event} event - Cursor activity event - * @param {Editor} editor - Editor instance - */ - function handleCursorActivity(event, editor) { - if (!isInSnippetSession() || activeSnippetSession.editor !== editor) { - return; - } - - // end session if user creates multiple selections - if (editor.getSelections().length > 1) { - endSnippetSession(); - return; - } - - const cursorPos = editor.getCursorPos(); - if (!isCursorInSnippetLines(cursorPos)) { - endSnippetSession(); - } - } - - /** - * This function is responsible to register all the required handers - * we need this to set up all the event listeners for cursor navigation - */ - function registerHandlers() { - // register the event handler for snippet cursor navigation - const editorHolder = $("#editor-holder")[0]; - if (editorHolder) { - editorHolder.addEventListener( - "keydown", - function (event) { - const editor = EditorManager.getActiveEditor(); - if (editor) { - handleKeyEvent(null, editor, event); - } - }, - true - ); - } - - // Listen for editor changes to end snippet sessions - EditorManager.on("activeEditorChange", function (event, current, previous) { - if (isInSnippetSession()) { - endSnippetSession(); - } - }); - - // Register cursor activity handler for current and future editors - function registerCursorActivityForEditor(editor) { - if (editor) { - editor.on("cursorActivity", handleCursorActivity); - } - } - - // Register for current editor - const currentEditor = EditorManager.getActiveEditor(); - if (currentEditor) { - registerCursorActivityForEditor(currentEditor); - } - - // Register for editor changes - EditorManager.on("activeEditorChange", function (event, current, previous) { - if (previous) { - previous.off("cursorActivity", handleCursorActivity); - } - if (current) { - registerCursorActivityForEditor(current); - } - if (isInSnippetSession()) { - endSnippetSession(); - } - }); - } - - exports.parseTemplateText = parseTemplateText; + exports.escapeBareDollarSigns = escapeBareDollarSigns; exports.insertSnippetWithTabStops = insertSnippetWithTabStops; exports.isInSnippetSession = isInSnippetSession; exports.handleKeyEvent = handleKeyEvent; - exports.handleCursorActivity = handleCursorActivity; exports.endSnippetSession = endSnippetSession; - exports.registerHandlers = registerHandlers; exports.navigateToNextTabStop = navigateToNextTabStop; // exposed for integration testing exports.navigateToPreviousTabStop = navigateToPreviousTabStop; // exposed for integration testing + exports.INDENT_TOKEN = INDENT_TOKEN; // referenced by defaultSnippets.js templateText + exports.resolveIndentToken = resolveIndentToken; // exposed for unit testing + exports.getOneIndentUnit = getOneIndentUnit; // exposed for unit testing }); diff --git a/src/extensionsIntegrated/Phoenix-live-preview/images/sprites.svg b/src/extensionsIntegrated/Phoenix-live-preview/images/sprites.svg index 721b454884..2d50de3e0c 100644 --- a/src/extensionsIntegrated/Phoenix-live-preview/images/sprites.svg +++ b/src/extensionsIntegrated/Phoenix-live-preview/images/sprites.svg @@ -7,20 +7,20 @@ ]]> - + - + - + - + diff --git a/src/extensionsIntegrated/Phoenix-live-preview/live-preview.css b/src/extensionsIntegrated/Phoenix-live-preview/live-preview.css index 1f6c64d549..40fcf5e973 100644 --- a/src/extensionsIntegrated/Phoenix-live-preview/live-preview.css +++ b/src/extensionsIntegrated/Phoenix-live-preview/live-preview.css @@ -1,11 +1,23 @@ :root { --toolbar-height: 30px; + --lp-toolbar-btn: 24px; + --lp-toolbar-chevron: 14px; + --lp-toolbar-gap: 6px; + --lp-toolbar-inset: 6px; + --lp-toolbar-fg: #a0a0a0; + --lp-toolbar-border: rgba(255, 255, 255, 0.1); } .live-preview-browser-btn { opacity: 0; visibility: hidden; - transition: opacity 1s, visibility 0s linear 1s; + transition: opacity 1s, visibility 0s linear 1s; +} + +.live-preview-browser-btn img { + display: block; + width: 14px; + height: 14px; } #live-preview-plugin-toolbar { @@ -204,17 +216,21 @@ .plugin-toolbar { height: var(--toolbar-height); - color: #a0a0a0; - display: flex; - justify-content: center; - align-content: center; - flex-direction: column; + color: var(--lp-toolbar-fg); } .toolbar-button { + width: var(--lp-toolbar-btn); + height: var(--lp-toolbar-btn); + padding: 0; + margin: 0; + display: flex; + align-items: center; + justify-content: center; + flex-shrink: 0; + box-sizing: border-box; background-color: transparent; - width: 28px; - height: 22px; + color: var(--lp-toolbar-fg); } #live-preview-plugin-toolbar .btn-alt-quiet:hover, @@ -238,23 +254,19 @@ } .open-icon { - background: url("./images/sprites.svg#open-icon") no-repeat 72.5%; - width: 30px; - margin-left: 4px; + background: url("./images/sprites.svg#open-icon") center / 20px no-repeat; } .pin-icon { - background: url("./images/sprites.svg#pinned-icon") no-repeat 72.5%; - width: 30px; + background: url("./images/sprites.svg#pinned-icon") center / 20px no-repeat; } .unpin-icon { - background: url("./images/sprites.svg#unpinned-icon") no-repeat 72.5%; - width: 30px; + background: url("./images/sprites.svg#unpinned-icon") center / 20px no-repeat; } .reload-icon { - background: url("./images/sprites.svg#reload-icon") center no-repeat; + background: url("./images/sprites.svg#reload-icon") center / 20px no-repeat; } #live-preview-plugin-toolbar:hover .lp-settings-icon { @@ -271,42 +283,8 @@ .lp-settings-icon { opacity: 0; - color: #a0a0a0; visibility: hidden; - transition: opacity 1s, visibility 0s linear 1s; - width: 30px; - height: 22px; - padding: 1px 6px; - flex-shrink: 0; - margin-top: 0; -} - -.lp-device-size-btn-group { - display: flex; - align-items: center; - flex-shrink: 0; - margin: 0 4px 0 3px; - border: 1px solid transparent; - border-radius: 3px; - box-sizing: border-box; -} - -.lp-device-size-btn-group:hover { - border-color: rgba(255, 255, 255, 0.1); -} - -.lp-device-size-icon { - display: flex; - align-items: center; - justify-content: center; - width: 20px; - cursor: pointer; - background: transparent; - box-shadow: none !important; - border: none; - color: #a0a0a0; - padding: 0; - margin: 0; + transition: opacity 1s, visibility 0s linear 1s; } .lp-device-size-icon:hover, @@ -317,25 +295,8 @@ box-shadow: none !important; } -.lp-device-size-dropdown-chevron { - display: flex; - align-items: center; - justify-content: center; - cursor: pointer; - background: transparent; - box-shadow: none !important; - border: none; - border-left: 1px solid transparent; - border-radius: 0 3px 3px 0; - color: #a0a0a0; - padding: 0 4px; - margin: 0; - height: 22px; - font-size: 10px; -} - .lp-device-size-btn-group:hover .lp-device-size-dropdown-chevron { - border-left-color: rgba(255, 255, 255, 0.1); + border-left-color: var(--lp-toolbar-border); } .lp-device-size-dropdown-chevron:hover, @@ -381,46 +342,37 @@ flex-direction: row; } -.lp-toolbar-left { +.lp-toolbar-left, +.lp-toolbar-centre, +.lp-toolbar-right { display: flex; align-items: center; - gap: 6px; - padding-left: 6px; + gap: var(--lp-toolbar-gap); box-sizing: border-box; - flex-shrink: 0; +} + +.lp-toolbar-left { + flex: 1 1 0; + padding-left: var(--lp-toolbar-inset); } .lp-toolbar-centre { - flex: 1; + flex: 0 1 auto; min-width: 0; - display: flex; justify-content: center; - align-items: center; overflow: hidden; } .lp-toolbar-right { - display: flex; - align-items: center; + flex: 1 1 0; + padding-right: var(--lp-toolbar-inset); justify-content: flex-end; - flex-shrink: 0; } #reloadLivePreviewButton, #designModeToggleLivePreviewButton { - width: 28px; - height: 24px; - margin: 0; - padding: 0; - display: flex; - align-items: center; - justify-content: center; - background: transparent; - color: #a0a0a0; border: 1px solid transparent; border-radius: 3px; - box-sizing: border-box; - flex-shrink: 0; } #live-preview-plugin-toolbar #reloadLivePreviewButton:hover, @@ -430,10 +382,11 @@ box-shadow: none !important; } -.lp-mode-btn-group { +.lp-mode-btn-group, +.lp-device-size-btn-group { display: flex; align-items: stretch; - height: 24px; + height: var(--lp-toolbar-btn); margin: 0; border: 1px solid transparent; border-radius: 3px; @@ -441,23 +394,26 @@ flex-shrink: 0; } -.lp-mode-btn-group:hover { - border-color: rgba(255, 255, 255, 0.1); +.lp-mode-btn-group:hover, +.lp-device-size-btn-group:hover { + border-color: var(--lp-toolbar-border); } -.lp-mode-icon { +.lp-mode-icon, +.lp-device-size-icon { display: flex; align-items: center; justify-content: center; - width: 24px; + width: var(--lp-toolbar-btn); height: 100%; + padding: 0; + margin: 0; + flex-shrink: 0; cursor: pointer; background: transparent; box-shadow: none !important; border: none; - color: #a0a0a0; - padding: 0; - margin: 0; + color: var(--lp-toolbar-fg); } #live-preview-plugin-toolbar .lp-mode-icon:hover, @@ -472,26 +428,28 @@ color: #FBB03B; } -.lp-mode-dropdown-chevron { +.lp-mode-dropdown-chevron, +.lp-device-size-dropdown-chevron { display: flex; align-items: center; justify-content: center; - width: 14px; + width: var(--lp-toolbar-chevron); height: 100%; + padding: 0; + margin: 0; + flex-shrink: 0; cursor: pointer; background: transparent; box-shadow: none !important; border: none; border-left: 1px solid transparent; border-radius: 0; - color: #a0a0a0; - padding: 0; - margin: 0; + color: var(--lp-toolbar-fg); font-size: 10px; } .lp-mode-btn-group:hover .lp-mode-dropdown-chevron { - border-left-color: rgba(255, 255, 255, 0.1); + border-left-color: var(--lp-toolbar-border); } #live-preview-plugin-toolbar .lp-mode-dropdown-chevron:hover, diff --git a/src/extensionsIntegrated/Terminal/TerminalInstance.js b/src/extensionsIntegrated/Terminal/TerminalInstance.js index e0afb59b98..2d6539b765 100644 --- a/src/extensionsIntegrated/Terminal/TerminalInstance.js +++ b/src/extensionsIntegrated/Terminal/TerminalInstance.js @@ -41,11 +41,19 @@ define(function (require, exports, module) { /** - * Read terminal theme colors from CSS variables + * Read terminal theme colors from CSS variables. + * @param {Element} [baseEl] - Element to resolve --terminal-* custom + * properties from (CSS custom properties inherit down the DOM tree, + * so this instance picks up whichever ancestor defines them). Pass + * the instance's own container so each embedding context controls + * its own theme — e.g. the bottom Terminal panel's light/dark-aware + * .terminal-panel-container vs. the always-dark AI sidebar's fixed + * palette (see .ai-chat-body-cli in Extn-AIChatPanel.less). Falls + * back to the old global lookup when omitted. * @returns {Object} xterm.js theme object */ - function _getThemeFromCSS() { - const panelEl = document.querySelector('.terminal-panel-container') || document.documentElement; + function _getThemeFromCSS(baseEl) { + const panelEl = baseEl || document.querySelector('.terminal-panel-container') || document.documentElement; const style = getComputedStyle(panelEl); function v(name) { return style.getPropertyValue(name).trim() || undefined; @@ -122,7 +130,7 @@ define(function (require, exports, module) { // Create xterm.js instance this.terminal = new Terminal({ - theme: _getThemeFromCSS(), + theme: _getThemeFromCSS(this.$container[0]), fontFamily: "'Menlo', 'DejaVu Sans Mono', 'Consolas', 'Lucida Console', monospace", fontSize: 13, lineHeight: 1.2, @@ -189,8 +197,11 @@ define(function (require, exports, module) { /** * Spawn the PTY process on the Node side + * @param {Object} [env] - Extra environment variables to merge into the + * PTY's process env (e.g. custom API endpoint overrides). Optional — + * existing callers that omit it are unaffected. */ - TerminalInstance.prototype.spawn = async function () { + TerminalInstance.prototype.spawn = async function (env) { const dims = this.fitAddon.proposeDimensions(); try { const result = await this.nodeConnector.execPeer("createTerminal", { @@ -199,7 +210,8 @@ define(function (require, exports, module) { args: this.shellProfile.args || [], cwd: this.cwd, cols: dims ? dims.cols : 80, - rows: dims ? dims.rows : 24 + rows: dims ? dims.rows : 24, + env: env || undefined }); this.pid = result.pid; this.isAlive = true; @@ -406,7 +418,7 @@ define(function (require, exports, module) { */ TerminalInstance.prototype.updateTheme = function () { if (this.terminal) { - this.terminal.options.theme = _getThemeFromCSS(); + this.terminal.options.theme = _getThemeFromCSS(this.$container ? this.$container[0] : null); } }; diff --git a/src/extensionsIntegrated/Terminal/main.js b/src/extensionsIntegrated/Terminal/main.js index a9e10ae4e1..060136b47f 100644 --- a/src/extensionsIntegrated/Terminal/main.js +++ b/src/extensionsIntegrated/Terminal/main.js @@ -1008,6 +1008,20 @@ define(function (require, exports, module) { exports.CMD_VIEW_TERMINAL = CMD_VIEW_TERMINAL; exports.CMD_NEW_TERMINAL = CMD_NEW_TERMINAL; + /** + * Get the shared "phoenix_terminal" NodeConnector so other extensions + * (e.g. the AI chat panel's embedded CLI terminal) can spawn their own + * independent TerminalInstance without registering a second connector + * on the same id, which throws. Safe to call any time after boot — + * _initNodeConnector() runs unconditionally on AppInit.appReady, before + * any user interaction. + * @return {Object|null} The terminal NodeConnector, or null if this is + * not a native app build (terminal is unavailable there). + */ + exports.getNodeConnector = function () { + return nodeConnector; + }; + if (Phoenix.isTestWindow) { exports._getActiveTerminal = _getActiveTerminal; exports._refreshAllProcesses = _refreshAllProcesses; diff --git a/src/features/BeautificationManager.js b/src/features/BeautificationManager.js index 9198879038..4908e458b8 100644 --- a/src/features/BeautificationManager.js +++ b/src/features/BeautificationManager.js @@ -323,6 +323,10 @@ define(function (require, exports, module) { if(!_isBeautifyOnSaveEnabled() || !editor || editor.document.file.fullPath !== doc.file.fullPath){ return; } + if(!_getEnabledProviders(doc.file.fullPath).length){ + // no beautify provider registered for this file type, silently skip instead of showing an error. + return; + } editor.clearSelection(); _beautifyCommand(); } diff --git a/src/features/QuickViewManager.js b/src/features/QuickViewManager.js index 17a47f7adf..2970f9fd48 100644 --- a/src/features/QuickViewManager.js +++ b/src/features/QuickViewManager.js @@ -241,6 +241,23 @@ define(function (require, exports, module) { animationRequest, quickViewLocked = false; + // True while some other menu-like UI is open - the top menu bar, a context menu (including the + // editor's right-click menu), the autocomplete Code Hints list, or an InlineMenu picker like + // jump-to-definition's multi-target picker (see languageTools/DefaultProviders.js) or Extract + // to Variable/Function. QuickView shouldn't pop up while any of those has the user's attention. + // All of them share the same underlying convention - a `

` root that + // gets an "open" class added while shown (see command/Menus.js openMenu()/registerContextMenu() + // and the codehint-menu/inlinemenu-menu markup in CodeHintList.js/widgets/InlineMenu.js) - so + // one generic selector covers all of them, including any future dropdown-based popup, without + // needing to name each one. Checked live at draw time, deliberately not via an imperative + // suppress/unsuppress pairing: those callers can close their popup through several different + // paths (Esc, selecting an item, clicking elsewhere - InlineMenu in particular doesn't notify + // on a plain click-away), and a state flag that isn't reliably unset on every one of those + // paths gets stuck "suppressed" forever. + function _isOtherEditorPopupOpen() { + return $(".dropdown.open").length > 0; + } + // Constants const CMD_ENABLE_QUICK_VIEW = "view.enableQuickView", QUICK_VIEW_EDITOR_MARKER = 'quickViewMark', @@ -599,7 +616,7 @@ define(function (require, exports, module) { clientY: event.clientY }; - if (!enabled || quickViewLocked + if (!enabled || quickViewLocked || _isOtherEditorPopupOpen() || $previewContainer[0].contains(window.document.activeElement)) { // activeElement check as, if the popup has an active element, say a text input, user may // move the mouse outside popup to type in the input, in which case we should not close popup. diff --git a/src/index.html b/src/index.html index 3ea50a46ac..481abdc295 100644 --- a/src/index.html +++ b/src/index.html @@ -47,14 +47,14 @@ - + - + @@ -68,15 +68,15 @@ - +