-
-
Notifications
You must be signed in to change notification settings - Fork 167
feat: add per-token multiline VALUE inputs #1061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a per- Changes
Sequence Diagram(s)sequenceDiagram
participant Parser as ValueSyntax Parser
participant Formatter as Formatter
participant ReqCollector as RequirementCollector
participant GUI as InputPrompt Factory
participant Modal as OnePageInputModal
Parser->>Formatter: parse token (include inputTypeOverride, label, default)
Formatter->>ReqCollector: set valuePromptContext (inputTypeOverride, label, default)
ReqCollector->>GUI: request field requirement (type: textarea or text)
GUI-->>ReqCollector: return prompt component (wide/text)
ReqCollector->>Formatter: gather responses
Formatter->>Modal: compose output (textarea values escaped)
Modal-->>Formatter: final resolved output
note right of GUI `#DDEBF7`: inputTypeOverride short-circuits global setting\n(if "multiline" -> wide prompt)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx,mts,mjs,js,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,svelte}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/{engine,services,utils}/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-21T07:54:34.863ZApplied to files:
📚 Learning: 2025-12-21T07:54:34.863ZApplied to files:
🧬 Code graph analysis (3)src/formatters/completeFormatter.ts (2)
src/formatters/formatter.ts (1)
src/utils/valueSyntax.test.ts (1)
🪛 ast-grep (0.40.3)src/formatters/formatter.ts[warning] 177-177: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) 🔇 Additional comments (14)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/preflight/RequirementCollector.ts (1)
159-171: Consider extracting the repeated input-type resolution logic.The pattern for computing
baseInputTypeappears three times in this file (here, inpromptForValue, and inpromptForVariable). Extracting it to a private helper would improve maintainability.🔎 Proposed helper extraction
+ private resolveBaseInputType( + inputType: "multiline" | undefined, + ): "textarea" | "text" { + return inputType === "multiline" || + this.plugin.settings.inputPrompt === "multi-line" + ? "textarea" + : "text"; + } + private scanVariableTokens(input: string) { // ... existing code ... - const baseInputType = - parsed.inputType === "multiline" || - this.plugin.settings.inputPrompt === "multi-line" - ? "textarea" - : "text"; + const baseInputType = this.resolveBaseInputType(parsed.inputType);src/utils/valueSyntax.ts (1)
170-193: Use theloggerutilities instead ofconsole.warn.Per coding guidelines for
src/**/*.{ts,tsx}, logging should be routed through theloggerutilities for consistent output. Theconsole.warncalls here should use the project's logging infrastructure.🔎 Proposed fix
+import { log } from "../logger/logManager"; + function resolveInputType( rawType: string | undefined, { tokenDisplay, hasOptions, allowCustomInput, }: { tokenDisplay: string; hasOptions: boolean; allowCustomInput: boolean }, ): ValueInputType | undefined { if (!rawType) return undefined; const normalized = rawType.trim().toLowerCase(); if (normalized !== "multiline") { - console.warn( + log.logWarning( `QuickAdd: Unsupported VALUE type "${rawType}" in token "${tokenDisplay}". Supported types: multiline.`, ); return undefined; } if (hasOptions || allowCustomInput) { - console.warn( + log.logWarning( `QuickAdd: Ignoring type:multiline for option-list VALUE token "${tokenDisplay}".`, ); return undefined; } return "multiline"; }Based on coding guidelines, route logging through the
loggerutilities.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/docs/Advanced/onePageInputs.mddocs/docs/FormatSyntax.mdsrc/constants.tssrc/formatters/completeFormatter.tssrc/formatters/formatter.tssrc/gui/InputPrompt.test.tssrc/gui/InputPrompt.tssrc/preflight/OnePageInputModal.tssrc/preflight/RequirementCollector.test.tssrc/preflight/RequirementCollector.tssrc/utils/valueSyntax.test.tssrc/utils/valueSyntax.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,mts,mjs,js,json}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,mts,mjs,js,json}: Use tab indentation with width 2 in TypeScript and configuration files (enforced by Biome).
Follow an 80-character line guide (enforced by Biome).
Files:
src/preflight/OnePageInputModal.tssrc/gui/InputPrompt.test.tssrc/formatters/completeFormatter.tssrc/gui/InputPrompt.tssrc/constants.tssrc/utils/valueSyntax.test.tssrc/formatters/formatter.tssrc/utils/valueSyntax.tssrc/preflight/RequirementCollector.test.tssrc/preflight/RequirementCollector.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use camelCase for variables and functions in TypeScript.
Prefer type-only imports in TypeScript.
Files:
src/preflight/OnePageInputModal.tssrc/gui/InputPrompt.test.tssrc/formatters/completeFormatter.tssrc/gui/InputPrompt.tssrc/constants.tssrc/utils/valueSyntax.test.tssrc/formatters/formatter.tssrc/utils/valueSyntax.tssrc/preflight/RequirementCollector.test.tssrc/preflight/RequirementCollector.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/preflight/OnePageInputModal.tssrc/gui/InputPrompt.test.tssrc/formatters/completeFormatter.tssrc/gui/InputPrompt.tssrc/constants.tssrc/utils/valueSyntax.test.tssrc/formatters/formatter.tssrc/utils/valueSyntax.tssrc/preflight/RequirementCollector.test.tssrc/preflight/RequirementCollector.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/preflight/OnePageInputModal.tssrc/gui/InputPrompt.test.tssrc/formatters/completeFormatter.tssrc/gui/InputPrompt.tssrc/constants.tssrc/utils/valueSyntax.test.tssrc/formatters/formatter.tssrc/utils/valueSyntax.tssrc/preflight/RequirementCollector.test.tssrc/preflight/RequirementCollector.ts
src/{engine,services,utils}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Structure production code so Obsidian dependencies are injected behind interfaces to enable unit testing without loading real Obsidian modules.
Files:
src/utils/valueSyntax.test.tssrc/utils/valueSyntax.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T07:54:34.863Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T07:54:34.863Z
Learning: Applies to tests/**/*.{ts,tsx} : Unit tests should target pure logic and swap in adapters or use `tests/obsidian-stub.ts` for Obsidian dependencies.
Applied to files:
src/gui/InputPrompt.test.ts
🧬 Code graph analysis (6)
src/gui/InputPrompt.test.ts (3)
src/gui/InputPrompt.ts (1)
InputPrompt(6-17)src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts (1)
GenericWideInputPrompt(7-235)src/gui/GenericInputPrompt/GenericInputPrompt.ts (1)
GenericInputPrompt(7-236)
src/formatters/completeFormatter.ts (2)
src/gui/InputPrompt.ts (1)
InputPrompt(6-17)docs/static/scripts/userScriptExample.js (1)
description(141-145)
src/gui/InputPrompt.ts (2)
src/utils/valueSyntax.ts (1)
ValueInputType(4-4)src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts (1)
GenericWideInputPrompt(7-235)
src/utils/valueSyntax.test.ts (1)
src/utils/valueSyntax.ts (2)
parseValueToken(195-237)parseAnonymousValueOptions(239-276)
src/formatters/formatter.ts (2)
src/utils/valueSyntax.ts (2)
ValueInputType(4-4)parseAnonymousValueOptions(239-276)src/constants.ts (1)
NAME_VALUE_REGEX(80-82)
src/preflight/RequirementCollector.test.ts (1)
src/preflight/RequirementCollector.ts (1)
RequirementCollector(43-335)
🪛 ast-grep (0.40.3)
src/formatters/formatter.ts
[warning] 176-176: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(NAME_VALUE_REGEX.source, "gi")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (26)
docs/docs/FormatSyntax.md (2)
16-16: Documentation is clear and comprehensive.The new VALUE token option is well-documented with appropriate constraints and usage notes. The explanation about shorthand defaults being ignored when
|type:is present is particularly helpful for avoiding confusion.
31-38: Good practical example demonstrating mixed input types.The example clearly shows how to combine single-line and multi-line inputs in the same template, which is helpful for users.
docs/docs/Advanced/onePageInputs.md (1)
21-21: LGTM!The documentation addition is concise and accurately describes the textarea rendering behavior for multiline VALUE tokens in the one-page modal.
src/preflight/OnePageInputModal.ts (2)
407-416: LGTM!The type-aware value escaping logic is well-implemented. Creating a Map for requirement lookup is efficient, and the conditional escaping for textarea inputs is correctly applied.
421-423: LGTM!The backslash escaping implementation is straightforward and correct, consistent with the pattern used elsewhere in the codebase.
src/constants.ts (1)
80-82: LGTM!The regex update correctly supports optional inline filters while excluding colon-based variable forms. The negative lookahead
(?!:)and optional filter group(?:\|[^\n\r}]*)?are well-constructed.src/preflight/RequirementCollector.test.ts (4)
10-17: Good test helper improvement.The configurable
makePluginhelper enables cleaner testing of different settings configurations while ensuring consistent default initialization.
77-85: LGTM!The test correctly verifies that explicit
type:multilineoverrides the global single-line setting, which is core to the feature.
87-96: LGTM!The test provides good coverage for unnamed VALUE tokens with multiline type, including verification that labels become descriptions for unnamed tokens.
98-106: LGTM!The test confirms that global multiline settings continue to work when no explicit type is specified, ensuring backward compatibility.
src/gui/InputPrompt.test.ts (3)
1-13: LGTM!The test setup properly mocks dependencies to avoid Obsidian loading while maintaining the necessary interfaces for testing. This follows the project's testing patterns.
Based on learnings, unit tests should use stubs for Obsidian dependencies.
15-25: LGTM!The test correctly verifies that explicit
inputTypeparameter takes precedence over global settings, which is essential for per-token control.
27-39: LGTM!The tests provide complete coverage of the factory's decision logic: explicit override, global multiline fallback, and global single-line fallback.
src/utils/valueSyntax.test.ts (5)
1-1: Good test hygiene with afterEach cleanup.Adding
afterEachto restore mocks ensures test isolation and prevents mock leakage between test cases.Also applies to: 10-12
51-59: LGTM!The test verifies complete multiline parsing with all option types, ensuring the parser correctly extracts variable name, input type, label, and default value.
61-72: LGTM!These tests verify important edge cases: shorthand default handling when explicit type is present (consistent with documentation), and validation warnings for unknown types.
74-79: LGTM!The test correctly verifies that
type:multilineis rejected for option lists (comma-separated values), with appropriate warnings, as documented in the constraints.
82-101: LGTM!The tests provide appropriate coverage for anonymous VALUE token parsing, including happy path multiline handling and validation for unknown types.
src/formatters/completeFormatter.ts (2)
177-199: LGTM!The changes properly integrate inputType throughout the value prompting flow, and consistently pass defaultValue and description to both prompt variants. This enables the per-token multiline control while maintaining backward compatibility.
228-228: LGTM!The change enables inputType propagation for variable prompts, maintaining consistency with the value prompt implementation and completing the feature integration.
src/gui/InputPrompt.ts (1)
4-14: LGTM! Clean implementation of per-token multiline override.The type-only import follows coding guidelines, and the early-return pattern for
inputType === "multiline"correctly prioritizes explicit per-token configuration over the global plugin setting. The logic flow is clear and maintains backward compatibility.src/preflight/RequirementCollector.ts (1)
195-211: LGTM! Correct propagation of description and defaultValue.The
promptForValuemethod now correctly derives the input type fromvaluePromptContext?.inputTypeand propagates description and defaultValue fields. This enables the one-page input modal to display richer context for anonymous VALUE tokens.src/utils/valueSyntax.ts (1)
239-275: LGTM! Well-structured anonymous value options parser.The
parseAnonymousValueOptionsfunction correctly handles edge cases (empty input, leading pipe normalization) and properly delegates toparseOptionsandresolveInputType. The return type is appropriately narrow.src/formatters/formatter.ts (3)
66-72: LGTM! Correct handling of intentional empty string values.The updated
hasConcreteVariablelogic correctly distinguishes between "no value set" (undefined/null) and "intentionally empty value" (""). The docstring clarifies the intent well.
176-203: LGTM! The static analysis warning is a false positive.The
NAME_VALUE_REGEXis a compile-time constant fromsrc/constants.ts, not user-controlled input. Thenew RegExp(NAME_VALUE_REGEX.source, "gi")pattern is safe and commonly used to add flags to an existing regex. ThegetValuePromptContextmethod correctly extracts and merges context from anonymous VALUE tokens.
328-341: LGTM! Correct propagation of inputType to variable prompts.The
inputTypefrom parsed value tokens is now correctly passed through topromptForVariable, enabling subclasses likeCompleteFormatterto select the appropriate input prompt type.
Summary
|type:multilineparsing for VALUE tokens with warnings for unsupported/option-list usageFixes #339.
Testing
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.