Conversation
- type: invoke native HTMLInputElement/Textarea/Select prototype value setter before assignment so React/Vue/Svelte value trackers see the change; dispatch input + change events so framework listeners fire. - type: accept --selector/--text as named flag synonyms for the positional form. Conflicting positional+flag is rejected with a clear AppError. Unknown flags on `type` get a tailored hint pointing at both invocation forms. - eval: wrap user code in a strict-mode IIFE that runs `eval(<source>)` so const/let/var declarations stay scoped per call (Firefox's console actor otherwise shares scope across evaluations and rejects redeclarations). Single expressions still return their value via eval's completion semantics. - eval: add --no-isolate to opt out and share scope across calls. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis pull request extends the CLI API by adding IIFE-based isolation to the eval command with an opt-out flag, supporting dual positional and flag-based input forms for the type command, refactoring type to use native setters for framework compatibility, and introducing contextual error hints when users invoke the type subcommand. End-to-end tests validate all new behaviors, and the iteration is marked complete. ChangesInput & Eval Ergonomics Enhancement
Sequence DiagramssequenceDiagram
participant User
participant CLI as ff-rdp CLI
participant Eval as eval::run()
participant Builder as build_script()
participant JS as JS Evaluator
User->>CLI: eval command with default isolation
CLI->>Eval: run(..., no_isolate=false)
Eval->>Builder: build_script(script, isolate=true)
Builder->>Builder: Wrap in strict IIFE
Builder-->>Eval: Final script wrapped
Eval->>JS: evaluate_js_async(final_script)
JS-->>User: result returned
User->>CLI: eval with --no-isolate flag
CLI->>Eval: run(..., no_isolate=true)
Eval->>Builder: build_script(script, isolate=false)
Builder-->>Eval: Script unchanged
Eval->>JS: evaluate_js_async(script)
JS-->>User: result returned
sequenceDiagram
participant User
participant CLI as ff-rdp CLI
participant Validator as Type Validator
participant TypeRunner as type_text::run()
participant JSGen as JS Generator
participant DOM as Page DOM
User->>CLI: type with flag-based input
CLI->>Validator: Validate form exclusivity
Validator-->>CLI: Extract selector and text
CLI->>TypeRunner: run(selector, text, clear)
TypeRunner->>JSGen: Generate JS with setter
JSGen->>JSGen: JSON-encode text
JSGen-->>TypeRunner: Setter-based JS code
TypeRunner->>DOM: Evaluate on page
DOM->>DOM: Call native prototype setter
DOM->>DOM: Dispatch input and change events
DOM-->>User: Framework notified of change
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR improves CLI ergonomics and modern framework compatibility by enhancing the type command (React/Vue/Svelte-friendly value setting + better argument UX) and making eval isolated-by-default to avoid Firefox console global-scope redeclaration issues.
Changes:
- Update
typeto set values via native element prototype setters and dispatchinput/changeevents for framework-controlled inputs. - Allow
type --selector/--textflag form alongside positional args, with clear conflict errors and a tailored unknown-flag hint. - Wrap
evalinput in a strict-mode IIFE viaeval(<encoded_source>)by default; addeval --no-isolateopt-out and add unit/e2e coverage.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| kb/iterations/iteration-52-input-eval-ergonomics.md | Marks iteration 52 tasks/acceptance criteria complete. |
| crates/ff-rdp-cli/tests/e2e/type_text.rs | Adds e2e coverage for type flag-vs-positional behavior and JS payload markers. |
| crates/ff-rdp-cli/tests/e2e/eval.rs | Adds e2e coverage for eval --no-isolate parsing and default isolation acceptance. |
| crates/ff-rdp-cli/src/main.rs | Augments clap parse errors with a type-specific hint using an argv heuristic. |
| crates/ff-rdp-cli/src/dispatch.rs | Implements selector/text resolution logic for type (positional vs flags) and wires eval no_isolate. |
| crates/ff-rdp-cli/src/commands/type_text.rs | Updates JS payload to use native prototype setters + dispatch input/change. |
| crates/ff-rdp-cli/src/commands/eval.rs | Introduces build_script to compose isolation/stringify behavior; adds tests; wires --no-isolate. |
| crates/ff-rdp-cli/src/cli/args.rs | Updates CLI help/docs and adds new flags/positional options for type and eval. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if a.starts_with("--") { | ||
| // Skip global flags that take a value (--host, --port, --timeout, etc.). | ||
| // A trailing `=` form is self-contained. | ||
| if !a.contains('=') { | ||
| let _ = iter.next(); | ||
| } |
| // discarded. We invoke the native prototype setter (cached at | ||
| // module-eval time below) to invalidate the tracker, then dispatch the | ||
| // synthetic `input`/`change` events the framework listeners expect. |
- main.rs: restrict is_type_invocation's lookahead-skip to an allowlist of value-taking globals, so booleans like --no-daemon don't swallow the `type` token and suppress the tailored hint (Copilot) - main.rs: drop the dead `let _ = Cli::command();` (and its now-unused `CommandFactory` import) - type_text.rs: fix misleading "cached at module-eval time" comment — the prototype setter is looked up on each invocation (Copilot) - main.rs: add tests covering boolean and mixed-global flag prefixes Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
typeworks on React/Vue/Svelte inputs: invoke the nativeHTMLInputElement/HTMLTextAreaElement/HTMLSelectElementprototypevaluesetter so React's value tracker is invalidated; then dispatchinput+changeevents so framework listeners fire.type --selector S --text T: accept the flag form alongside the positional form. Mixing positional and flag for the same value errors clearly. Unknown flags ontypeget a tailored hint pointing at both invocation forms.evalIIFE isolation by default: wrap user code in a strict-mode IIFE that runseval(<source>), soconst x = ...no longer collides with a previous call (Firefox's console actor shares global scope otherwise). Single expressions like1 + 1still return their value viaevalcompletion semantics.eval --no-isolate: opt out of the IIFE wrap when you want to share scope across calls.Test plan
cargo fmt,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace -qall pass.typepositional and flag forms,--selector/--textconflict, unknown-flag hint.eval --no-isolateparses; default-isolatedeval 'const x = 1; x'succeeds against the mock.build_scriptcovers the four (isolate × stringify) modes including special-character handling.typeagainst a React app's controlled input updates bound state;eval 'const x = 1; x'twice in a row succeeds.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--no-isolateflag to the eval command for disabling default code isolation (IIFE wrapping)--selectorand--textflags in addition to positional argumentsDocumentation