Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds parsing for labeled/default/custom VALUE tokens and labeled MACRO tokens, threads label/placeholder/description through formatters, prompts, and macro engines, introduces per-MultiChoice placeholder plus a settings modal, refactors token parsing into utilities, and updates ChoiceSuggester to accept options. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas to review closely:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts (1)
90-96: Consider extracting inline styles to CSS.The description rendering logic uses inline styles (
marginBottom = "0.75rem"). For consistency and maintainability, consider whether this should use a CSS class instead, especially if this pattern is shared withGenericInputPrompt.ts.src/gui/suggesters/choiceSuggester.ts (1)
67-87: Well-structured placeholder stack management for nested navigation.The implementation correctly:
- Tracks placeholder context as users navigate into/out of nested MultiChoices
- Uses stack operations (push on enter, pop on back) to restore previous placeholder state
- Prevents adding duplicate "Back" options when already at Back
One minor consideration: the
isBackdetection on line 69 relies on an exact name match ("← Back"). If the Back choice name were ever localized or changed, this would break.🔎 Optional: Consider a more robust back detection
+const BACK_CHOICE_NAME = "← Back"; + private onChooseMultiType(multi: IMultiChoice) { const choices = [...multi.choices]; - const isBack = multi.name === "← Back"; + const isBack = multi.name === BACK_CHOICE_NAME; if (!isBack) { - choices.push(new MultiChoice("← Back").addChoices(this.choices)); + choices.push(new MultiChoice(BACK_CHOICE_NAME).addChoices(this.choices)); }This extracts the magic string to a constant, making future changes easier and reducing duplication.
src/gui/GenericInputPrompt/GenericInputPrompt.ts (1)
93-99: Description rendering implementation is clean.The description displays correctly above the input using Obsidian's
setting-item-descriptionclass for consistent styling.Minor: The inline
marginBottomstyle on line 98 could be moved to a CSS class for consistency with the project's styling approach, but this is acceptable for a simple spacing adjustment.src/utils/valueSyntax.ts (1)
31-39: Clarify legacy#shorthand edge cases.The condition on line 32 (
!trimmed.includes(",") && !trimmed.startsWith("#")) excludes option lists and tag-like inputs, but the logic may be unclear for inputs likevalue#label,option2. Consider adding an inline comment explaining when the#shorthand applies versus when::should be used.🔎 Suggested documentation improvement
// Support legacy shorthand for single-value labels, but avoid tags/options lists. + // The `#` shorthand only applies to single-value tokens (no commas) that don't + // start with `#` (to avoid confusion with tag syntax). For multi-option tokens, + // use the `::` delimiter instead. if (!trimmed.includes(",") && !trimmed.startsWith("#")) {src/formatters/completeFormatter.ts (1)
218-225: Minor: Placeholder fallback todefaultValuemay cause UX confusion.When
placeholderis undefined butdefaultValueexists, the placeholder text will show the default value. This could confuse users who expect placeholder text to be descriptive (e.g., "Enter a name") rather than showing the actual default. Consider whether this is the intended behavior.context?.placeholder ?? (context?.defaultValue ? context.defaultValue : undefined),src/formatters/formatter.ts (1)
404-414: Verify behavior whenparseMacroTokenreturns null.When parsing fails, the macro token is silently stripped (replaced with empty string). This is reasonable for malformed tokens, but consider whether a warning log would help users debug issues.
🔎 Optional: Add debug logging for malformed macro tokens
const parsed = parseMacroToken(exec[1]); if (!parsed) { + log.logMessage(`Skipping malformed MACRO token: ${exec[1]}`); output = this.replacer(output, MACRO_REGEX, ""); continue; }src/preflight/RequirementCollector.ts (1)
238-256: Potential duplication:hasOptionsdetection differs fromparseValueToken.Line 240 checks
variableName.includes(",")to detect options, butparseValueTokenuses a more robust check (suggestedValues.length > 1). This could cause inconsistencies ifvariableNamecontains a trailing comma or edge cases.However, this path is only reached when
scanVariableTokenshasn't already recorded the requirement, which suggests this is a fallback for directpromptForVariablecalls. Consider using consistent parsing logic.🔎 Suggested improvement for consistency
// Generic named variables if (!this.requirements.has(key)) { - // Detect simple comma-separated option lists - const hasOptions = variableName.includes(","); + // Detect simple comma-separated option lists (split and filter like parseValueToken) + const options = variableName.split(",").map(s => s.trim()).filter(Boolean); + const hasOptions = options.length > 1; const req: FieldRequirement = { id: key, label: variableName, type: hasOptions ? "dropdown" : "text", description: context?.description, source: "collected", }; if (hasOptions) { - req.options = variableName - .split(",") - .map((s) => s.trim()) - .filter(Boolean); + req.options = options; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
docs/docs/Choices/MultiChoice.md(1 hunks)docs/docs/FormatSyntax.md(1 hunks)src/choiceExecutor.ts(2 hunks)src/constants.ts(3 hunks)src/engine/MacroChoiceEngine.ts(3 hunks)src/engine/SingleMacroEngine.ts(2 hunks)src/formatters/completeFormatter.ts(5 hunks)src/formatters/fileNameDisplayFormatter.ts(3 hunks)src/formatters/formatDisplayFormatter.ts(4 hunks)src/formatters/formatter-field-title-regression.test.ts(1 hunks)src/formatters/formatter-filenamecurrent.test.ts(1 hunks)src/formatters/formatter-issue920.test.ts(1 hunks)src/formatters/formatter-issue929-integration.test.ts(1 hunks)src/formatters/formatter-issue929.test.ts(1 hunks)src/formatters/formatter-linkcurrent.test.ts(1 hunks)src/formatters/formatter-template-property-types.test.ts(1 hunks)src/formatters/formatter-title.test.ts(1 hunks)src/formatters/formatter.ts(6 hunks)src/formatters/vdate-default.test.ts(3 hunks)src/gui/GenericInputPrompt/GenericInputPrompt.ts(4 hunks)src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts(5 hunks)src/gui/MultiChoiceSettingsModal.ts(1 hunks)src/gui/suggesters/choiceSuggester.ts(2 hunks)src/gui/suggesters/formatSyntaxSuggester.ts(2 hunks)src/preflight/RequirementCollector.test.ts(1 hunks)src/preflight/RequirementCollector.ts(6 hunks)src/services/choiceService.ts(3 hunks)src/types/choices/IMultiChoice.ts(1 hunks)src/types/choices/MultiChoice.ts(1 hunks)src/utils/macroSyntax.ts(1 hunks)src/utils/valueSyntax.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
docs/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Keep user-facing docs in
docs/directory
Files:
docs/docs/Choices/MultiChoice.mddocs/docs/FormatSyntax.md
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/engine/SingleMacroEngine.tssrc/types/choices/MultiChoice.tssrc/utils/macroSyntax.tssrc/preflight/RequirementCollector.test.tssrc/types/choices/IMultiChoice.tssrc/formatters/formatter-issue920.test.tssrc/gui/suggesters/formatSyntaxSuggester.tssrc/constants.tssrc/formatters/formatter-issue929.test.tssrc/formatters/formatter-field-title-regression.test.tssrc/formatters/formatter-filenamecurrent.test.tssrc/services/choiceService.tssrc/utils/valueSyntax.tssrc/formatters/formatter-title.test.tssrc/formatters/vdate-default.test.tssrc/choiceExecutor.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/engine/MacroChoiceEngine.tssrc/gui/suggesters/choiceSuggester.tssrc/formatters/completeFormatter.tssrc/formatters/formatDisplayFormatter.tssrc/preflight/RequirementCollector.tssrc/gui/MultiChoiceSettingsModal.tssrc/formatters/formatter-issue929-integration.test.tssrc/formatters/formatter-linkcurrent.test.tssrc/gui/GenericWideInputPrompt/GenericWideInputPrompt.tssrc/formatters/formatter-template-property-types.test.tssrc/formatters/formatter.tssrc/formatters/fileNameDisplayFormatter.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/engine/SingleMacroEngine.tssrc/types/choices/MultiChoice.tssrc/utils/macroSyntax.tssrc/preflight/RequirementCollector.test.tssrc/types/choices/IMultiChoice.tssrc/formatters/formatter-issue920.test.tssrc/gui/suggesters/formatSyntaxSuggester.tssrc/constants.tssrc/formatters/formatter-issue929.test.tssrc/formatters/formatter-field-title-regression.test.tssrc/formatters/formatter-filenamecurrent.test.tssrc/services/choiceService.tssrc/utils/valueSyntax.tssrc/formatters/formatter-title.test.tssrc/formatters/vdate-default.test.tssrc/choiceExecutor.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/engine/MacroChoiceEngine.tssrc/gui/suggesters/choiceSuggester.tssrc/formatters/completeFormatter.tssrc/formatters/formatDisplayFormatter.tssrc/preflight/RequirementCollector.tssrc/gui/MultiChoiceSettingsModal.tssrc/formatters/formatter-issue929-integration.test.tssrc/formatters/formatter-linkcurrent.test.tssrc/gui/GenericWideInputPrompt/GenericWideInputPrompt.tssrc/formatters/formatter-template-property-types.test.tssrc/formatters/formatter.tssrc/formatters/fileNameDisplayFormatter.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/engine/SingleMacroEngine.tssrc/types/choices/MultiChoice.tssrc/utils/macroSyntax.tssrc/preflight/RequirementCollector.test.tssrc/types/choices/IMultiChoice.tssrc/formatters/formatter-issue920.test.tssrc/gui/suggesters/formatSyntaxSuggester.tssrc/constants.tssrc/formatters/formatter-issue929.test.tssrc/formatters/formatter-field-title-regression.test.tssrc/formatters/formatter-filenamecurrent.test.tssrc/services/choiceService.tssrc/utils/valueSyntax.tssrc/formatters/formatter-title.test.tssrc/formatters/vdate-default.test.tssrc/choiceExecutor.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/engine/MacroChoiceEngine.tssrc/gui/suggesters/choiceSuggester.tssrc/formatters/completeFormatter.tssrc/formatters/formatDisplayFormatter.tssrc/preflight/RequirementCollector.tssrc/gui/MultiChoiceSettingsModal.tssrc/formatters/formatter-issue929-integration.test.tssrc/formatters/formatter-linkcurrent.test.tssrc/gui/GenericWideInputPrompt/GenericWideInputPrompt.tssrc/formatters/formatter-template-property-types.test.tssrc/formatters/formatter.tssrc/formatters/fileNameDisplayFormatter.ts
🧠 Learnings (1)
📚 Learning: 2025-12-09T21:20:52.425Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.425Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/preflight/RequirementCollector.test.tssrc/formatters/formatter-issue920.test.tssrc/formatters/formatter-field-title-regression.test.tssrc/formatters/formatter-filenamecurrent.test.tssrc/formatters/formatter-title.test.tssrc/formatters/formatter-issue929-integration.test.tssrc/formatters/formatter-linkcurrent.test.ts
🧬 Code graph analysis (12)
src/engine/SingleMacroEngine.ts (1)
esbuild.config.mjs (1)
context(79-79)
src/preflight/RequirementCollector.test.ts (1)
src/preflight/RequirementCollector.ts (1)
RequirementCollector(43-330)
src/services/choiceService.ts (2)
src/types/choices/IMultiChoice.ts (1)
IMultiChoice(3-7)src/gui/MultiChoiceSettingsModal.ts (1)
MultiChoiceSettingsModal(5-94)
src/gui/GenericInputPrompt/GenericInputPrompt.ts (1)
docs/static/scripts/userScriptExample.js (1)
description(141-145)
src/gui/suggesters/choiceSuggester.ts (6)
src/IChoiceExecutor.ts (1)
IChoiceExecutor(4-18)tests/obsidian-stub.ts (1)
FuzzySuggestModal(366-441)src/types/choices/IChoice.ts (1)
IChoice(3-10)src/choiceExecutor.ts (1)
ChoiceExecutor(18-130)src/main.ts (1)
QuickAdd(35-325)src/types/choices/MultiChoice.ts (1)
MultiChoice(5-23)
src/formatters/completeFormatter.ts (3)
src/gui/VDateInputPrompt/VDateInputPrompt.ts (1)
VDateInputPrompt(6-171)src/gui/InputSuggester/inputSuggester.ts (1)
InputSuggester(22-195)src/gui/GenericSuggester/genericSuggester.ts (1)
GenericSuggester(8-142)
src/formatters/formatDisplayFormatter.ts (2)
src/utils/valueSyntax.ts (1)
VALUE_LABEL_DELIMITER(1-1)src/formatters/helpers/previewHelpers.ts (2)
getVariableExample(44-53)getSuggestionPreview(79-84)
src/preflight/RequirementCollector.ts (1)
src/utils/valueSyntax.ts (1)
parseValueToken(44-79)
src/gui/MultiChoiceSettingsModal.ts (2)
tests/obsidian-stub.ts (4)
Modal(443-447)App(269-297)Setting(151-233)ButtonComponent(33-67)src/types/choices/IMultiChoice.ts (1)
IMultiChoice(3-7)
src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts (1)
docs/static/scripts/userScriptExample.js (1)
description(141-145)
src/formatters/formatter.ts (3)
src/utils/valueSyntax.ts (1)
parseValueToken(44-79)src/utils/macroSyntax.ts (1)
parseMacroToken(6-22)src/constants.ts (1)
MACRO_REGEX(90-90)
src/formatters/fileNameDisplayFormatter.ts (2)
src/utils/valueSyntax.ts (1)
VALUE_LABEL_DELIMITER(1-1)src/formatters/helpers/previewHelpers.ts (3)
getVariableExample(44-53)getSuggestionPreview(79-84)getMacroPreview(58-61)
🔇 Additional comments (48)
src/engine/SingleMacroEngine.ts (1)
45-48: LGTM! Clean API extension.The optional
contextparameter with label support is properly propagated toMacroChoiceEngine. The use of optional chaining ensures backward compatibility.Also applies to: 100-100
src/gui/suggesters/formatSyntaxSuggester.ts (1)
243-243: LGTM! Clear example suggestions.The new suggestion examples effectively demonstrate the label syntax for both MACRO and VALUE tokens. The hardcoded "MyMacro" example is appropriate for illustrating the syntax pattern.
Also applies to: 261-263
src/types/choices/IMultiChoice.ts (1)
6-6: LGTM! Clean interface extension.The optional
placeholderproperty extends the interface without breaking existing implementations.src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts (1)
19-35: LGTM! Consistent API extension.The description parameter is properly threaded through both static factory methods and the constructor, maintaining backward compatibility with optional parameters.
Also applies to: 37-54, 56-66
src/gui/MultiChoiceSettingsModal.ts (3)
14-29: LGTM! Proper modal initialization.The constructor correctly initializes the promise, copies initial values from the choice, and sets up the modal UI. The definite assignment assertions on
resolvePromiseandrejectPromiseare safe since they're immediately assigned in the promise constructor.
31-67: LGTM! Clean UI construction.The modal layout uses Obsidian's
SettingAPI consistently and provides clear labels and placeholders. The button styling with flexbox is appropriate.
79-93: LGTM! Proper state handling on close.The
onCloselogic correctly handles cancellation vs. submission, trims input values, and provides sensible defaults (falling back to original name if empty, setting placeholder toundefinedif empty).src/types/choices/MultiChoice.ts (1)
8-8: LGTM! Implements interface property.The optional
placeholderproperty correctly implements theIMultiChoiceinterface extension.docs/docs/Choices/MultiChoice.md (1)
13-15: LGTM! Clear documentation.The placeholder text documentation is concise and explains the feature's purpose and behavior clearly.
src/constants.ts (1)
8-9: No action required. The parsing logic for::and|delimiters is fully implemented and tested. TheparseValueToken()function insrc/utils/valueSyntax.tsexplicitly handles the::delimiter via thesplitLabel()helper, andparseMacroToken()insrc/utils/macroSyntax.tshandles the|delimiter. Both functions are integrated into the formatter and covered by tests inRequirementCollector.test.ts.src/preflight/RequirementCollector.test.ts (1)
25-40: LGTM! Good regression coverage for VALUE label parsing.The test validates the new VALUE label syntax (
::) for both single-value and multi-value inputs, ensuring labels and descriptions are correctly extracted and scoped.docs/docs/FormatSyntax.md (2)
13-13: LGTM! Clear documentation for VALUE helper text syntax.The documentation accurately describes the new
::delimiter for helper text, explains the difference between single and multi-value usage, and correctly notes the#shorthand. The guidance about ordering (helper text before|defaultor|custom) is helpful.
19-19: LGTM! Clear documentation for MACRO label syntax.The documentation clearly explains how the label parameter helps disambiguate multiple similar macro prompts.
src/formatters/formatter-filenamecurrent.test.ts (1)
33-73: LGTM! Test stub signatures aligned with expanded Formatter API.The signature updates correctly mirror the context-aware API expansion across the formatter hierarchy. The unused parameters follow the standard test stub pattern.
src/formatters/formatter-issue920.test.ts (1)
46-86: LGTM! Test stub signatures aligned with expanded Formatter API.The signature updates correctly mirror the context-aware API expansion, maintaining the existing regression test coverage for issue #920.
src/utils/macroSyntax.ts (1)
1-22: LGTM! Clean and straightforward MACRO token parser.The parsing logic correctly handles the pipe delimiter, trims whitespace, and returns appropriate null/undefined values for invalid or empty inputs. The API is well-structured for downstream consumers.
src/engine/MacroChoiceEngine.ts (3)
82-82: LGTM! Field addition for prompt labeling.The new optional field enables contextual labels for macro export prompts.
149-164: LGTM! Constructor properly initializes promptLabel.The optional parameter is correctly typed and initialized, maintaining backward compatibility.
373-380: GenericSuggester.Suggest signature is correct.The method accepts
placeholder?: stringas the fourth parameter, matching the code's usage ofthis.promptLabel.src/services/choiceService.ts (3)
7-7: LGTM! Import updated to support modal-based configuration.The import change reflects the new modal-based flow for Multi choice configuration.
43-48: LGTM! Improved Multi choice duplication with explicit property copying.The new implementation explicitly constructs the result with deep-cloned choices and preserved placeholder/collapsed fields, improving clarity and correctness.
134-141: LGTM! Modal-based configuration improves user experience.The switch from inline prompts to
MultiChoiceSettingsModalprovides a better UX for editing Multi choices. The error handling appropriately returnsundefinedon failure or cancellation.src/formatters/formatter-template-property-types.test.ts (1)
31-70: LGTM! Test stub signatures aligned with expanded Formatter API.The signature updates correctly mirror the context-aware API expansion, maintaining the existing template property type inference test coverage.
src/formatters/fileNameDisplayFormatter.ts (2)
74-79: LGTM! Clean implementation of label-aware variable resolution.The logic correctly checks for a stored variable first, then strips the label delimiter to get the base name for generating an example. The nullish coalescing fallback on line 77 handles edge cases well.
91-97: Extended method signatures for context propagation look good.The underscore-prefixed parameters (
_context) appropriately signal that they're unused in this display formatter while maintaining API compatibility with the baseFormatterclass. This allows concrete formatters to utilize the context when needed.Also applies to: 103-108, 110-123
src/choiceExecutor.ts (1)
124-129: Correct adoption of the new options-based API.The change properly passes the
choiceExecutorandplaceholderthrough an options object, aligning with the updatedChoiceSuggester.Opensignature. This enables per-MultiChoice placeholder text as intended by the PR objectives.src/gui/suggesters/choiceSuggester.ts (1)
38-41: Verify placeholder is applied correctly.The initialization looks correct—
setPlaceholderis only called when a non-empty placeholder exists. However, ensure the parent classFuzzySuggestModal.setPlaceholderis called aftersuper(plugin.app)completes, which it is here.src/formatters/formatter-field-title-regression.test.ts (1)
54-90: Test stub signatures correctly updated to match base class API.The extended method signatures with context parameters properly align with the
Formatterbase class changes. The underscore-prefixed parameters and empty-string returns are appropriate for this focused regression test that specifically tests FIELD/TITLE namespace handling.Based on learnings, this file appropriately adds regression coverage for the namespace separation bug fix.
src/gui/GenericInputPrompt/GenericInputPrompt.ts (1)
19-35: API extension is backward compatible.The
descriptionparameter is correctly added as the last optional parameter in bothPromptandPromptWithContext, maintaining backward compatibility with existing callers (e.g., the userScriptExample.js in docs that uses 3 parameters).Also applies to: 37-54
src/formatters/formatter-issue929.test.ts (1)
33-73: LGTM! Method signatures align with the expanded Formatter API.The updated signatures for
suggestForValue,suggestForField,getMacroValue,promptForVariable, andgetTemplateContentcorrectly match the base class contract changes. Parameters are appropriately prefixed with underscores to indicate they're unused in this test implementation.src/utils/valueSyntax.ts (1)
44-78: Well-structured token parsing logic.The
parseValueTokenfunction correctly handles:
- Pipe delimiter for default values
- Label extraction via
splitLabel- Custom input detection via case-insensitive "custom" check
- Variable key scoping with labels only for multi-option tokens
The implementation aligns with the PR objectives for VALUE labels and scoped variable storage.
src/formatters/vdate-default.test.ts (1)
39-79: LGTM! Test formatter signatures updated correctly.The expanded context parameters for
getMacroValue,promptForVariable, andsuggestForValuealign with the base class API changes. ThepromptForVariableimplementation correctly respectscontext.defaultValueas a fallback when no mock value is set.src/formatters/formatter-title.test.ts (1)
32-72: LGTM! Consistent signature updates across test formatters.The method signatures match the expanded Formatter API. The test suite provides good regression coverage for title handling, particularly the tests at lines 147-169 that verify script-provided titles are preserved over file basename titles (issue #929 fix).
src/formatters/formatDisplayFormatter.ts (2)
77-82: Good handling of labeled variable keys in preview.The logic correctly extracts the base variable name by splitting on
VALUE_LABEL_DELIMITERbefore generating the example. This ensures preview output remains readable even when variable keys include labels (e.g.,options::Label1→ usesoptionsfor example lookup).
94-126: LGTM! Extended method signatures for preview formatter.The
suggestForValue,getMacroValue, andpromptForVariablemethods correctly accept the expanded context parameters while maintaining their preview-focused implementations. The unused_contextparameters appropriately signal that preview formatting doesn't require the additional metadata.src/formatters/formatter-issue929-integration.test.ts (2)
42-82: LGTM! Integration test signatures aligned with API.The signature updates are consistent with other test formatters. The
CaptureFormatterTestclass provides a good abstraction for testing the full macro flow scenario from issue #929.
163-187: Excellent edge case coverage.The tests for empty string (line 163-170) and null/undefined (lines 172-187) title values correctly verify the expected behavior:
- Empty string from script is preserved (not overwritten by fallback)
- Null/undefined correctly falls back to file basename
This provides strong regression coverage for the fix. Based on learnings, this aligns with the guidance to add regression coverage for bug fixes.
src/formatters/completeFormatter.ts (3)
193-232: LGTM! Extended context handling for prompts.The
promptForVariablemethod properly handles the expanded context object:
- VDateInputPrompt receives appropriate header fallback chain
- Regular prompts pass through placeholder, defaultValue, and description
- Cancellation error handling is preserved
331-352: LGTM! Macro value retrieval with label context.The
getMacroValuemethod now accepts an optionalcontextwithlabeland correctly forwards it tomacroEngine.runAndGetOutput. Variable propagation from macro execution remains intact.
263-268: No issues found. TheGenericSuggester.Suggestsignature correctly acceptsplaceholderas the 4th parameter (optional) andrenderItemas the 5th parameter (optional). The call at lines 263-268 properly passescontext?.placeholderas the placeholder argument and omitsrenderItem, which is valid since both are optional parameters.src/formatters/formatter.ts (4)
27-28: LGTM!Good separation of concerns by delegating token parsing to dedicated utility functions. This improves testability and reusability of the parsing logic.
263-276: LGTM!The destructuring of
parseValueTokenresult cleanly extracts all needed fields. The null check with error throw on parse failure maintains existing error behavior.
279-310: LGTM on the variable population flow.The logic correctly:
- Uses
variableKeyfor storage/lookup (enabling label-scoped variables per PR objectives)- Passes contextual metadata (
description,placeholder,variableKey) to prompts- Falls back to
defaultValuewhen no input provided
428-431: LGTM on the abstract method signature updates.The context objects are optional, ensuring backward compatibility with existing implementations. The expanded
promptForVariablecontext type consolidates all metadata needed for rich prompts.Also applies to: 555-571
src/formatters/formatter-linkcurrent.test.ts (1)
33-73: LGTM!The
StubFormattermethod signatures correctly match the updated abstract method contracts inFormatter. The underscore-prefixed parameters appropriately indicate unused arguments in this test context. Based on learnings, this maintains test compatibility while the base class evolves.src/preflight/RequirementCollector.ts (3)
139-186: LGTM on token parsing integration.The
scanVariableTokensmethod correctly:
- Uses
parseValueTokenfor consistent parsing- Derives
displayLabelfrom label (for options) or variableName (fallback)- Sets
descriptiononly for non-option variables with labels- Creates
suggesterConfigwhenallowCustomInputis true- Uses
variableKeyas the requirement ID for proper scoping
206-234: LGTM onpromptForVariablecontext handling.The method correctly:
- Uses
context?.variableKey ?? variableNameas the lookup key- Handles VDATE variables with proper dateFormat metadata
- Passes
context.descriptionthrough to requirements
287-296: LGTM on updated method signatures.The signatures align with the base
Formatterclass contracts. ThesuggestForValueno-op is appropriate sincescanVariableTokenshandles requirement recording upstream.Also applies to: 320-325
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/utils/valueSyntax.ts (1)
23-28: Consider documenting the default-true behavior.The function returns
truewhen the value is undefined or empty, which is unconventional but appropriate for flag syntax (e.g.,customwithout a value should enable the flag). Adding a brief JSDoc comment would help future readers understand this intentional design choice.🔎 Suggested JSDoc comment
+/** + * Parses a string value as a boolean. + * Returns true by default (when value is undefined/empty) to support flag syntax. + * @param value - The string value to parse + * @returns true unless value explicitly indicates false ("false", "no", "0", "off") + */ function parseBoolean(value?: string): boolean {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/docs/FormatSyntax.md(1 hunks)src/constants.ts(3 hunks)src/formatters/fileNameDisplayFormatter.ts(3 hunks)src/formatters/formatDisplayFormatter.ts(4 hunks)src/gui/suggesters/formatSyntaxSuggester.ts(2 hunks)src/preflight/RequirementCollector.test.ts(1 hunks)src/utils/valueSyntax.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/gui/suggesters/formatSyntaxSuggester.ts
- src/constants.ts
- src/formatters/fileNameDisplayFormatter.ts
- docs/docs/FormatSyntax.md
- src/preflight/RequirementCollector.test.ts
- src/formatters/formatDisplayFormatter.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/utils/valueSyntax.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/utils/valueSyntax.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/utils/valueSyntax.ts
🔇 Additional comments (4)
src/utils/valueSyntax.ts (4)
1-21: LGTM! Well-structured type definitions and constants.The type definitions are comprehensive and clear. Using a
SetforVALUE_OPTION_KEYSprovides efficient O(1) lookup, andParsedValueTokenproperly captures all parsed metadata fields for downstream consumption.
30-36: LGTM! Clean guard clauses and safe string parsing.The early returns and optional chaining make the function robust against edge cases.
38-103: LGTM! Robust handling of both legacy and explicit option syntax.The function correctly distinguishes between legacy syntax (where everything after the first
|is treated as a default value) and the new explicit option syntax withlabel:,default:, andcustom:keys. The logic gracefully handles both cases and the standalonecustomkeyword.
105-142: Label scoping applies only to multi-value VALUE prompts.The logic correctly limits label-based
variableKeymodification to scenarios with multiple suggested values:
{{VALUE:option1|label:My Label}}→variableKey = "option1"(no label scoping; label acts as helper text){{VALUE:option1,option2|label:My Label}}→variableKey = "option1,option2::My Label"(label scoped via::delimiter)This design allows multiple VALUE prompts with identical option lists but different labels to maintain separate variable storage and requirement tracking. The legacy
customshorthand (line 123) correctly requireshasOptionsto be true, since single-value prompts inherently accept custom input.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/preflight/RequirementCollector.ts (1)
229-248: PasshasOptionsin context to avoid redundant recalculation.The
promptForVariablemethod recalculateshasOptionsfrom the rawvariableNameusingvariableName.includes(","), butformatter.tsalready determines this viaparseValueTokenwith different logic:hasOptions = suggestedValues.length > 1(which requires at least 2 valid values after trimming and filtering). This creates potential for inconsistency in edge cases like"a,"or",a", where the presence of a comma differs from the count of valid items. Since formatter controls whether to callpromptForVariablebased on itshasOptionscalculation, pass this flag in the context object so RequirementCollector can reuse the determination instead of recalculating independently.
🧹 Nitpick comments (6)
src/gui/GenericInputPrompt/GenericInputPrompt.ts (1)
101-107: LGTM!The description rendering logic correctly conditionally displays the description when present, using Obsidian's
setting-item-descriptionCSS class for consistent styling.Consider extracting the inline margin style to a CSS class for maintainability, though this is a minor point given it's consistent with the pattern in
GenericWideInputPrompt.src/formatters/fileNameDisplayFormatter.ts (1)
163-189: Consider extracting magic numbers as constants.The values
6(preview length),100(max length), and'ABC123'are embedded inline. Consider extracting these as named constants for clarity.🔎 Suggested improvement
+const RANDOM_PREVIEW_LENGTH = 6; +const RANDOM_MAX_LENGTH = 100; +const RANDOM_PREVIEW_CHARS = 'ABC123'; + protected replaceRandomInString(input: string): string { let output = input; output = output.replace(/{{RANDOM:(\d+)}}/gi, (match, length) => { const len = parseInt(length); - if (len <= 0 || len > 100) { + if (len <= 0 || len > RANDOM_MAX_LENGTH) { return match; } - const exampleChars = 'ABC123'; let preview = ''; - for (let i = 0; i < Math.min(len, 6); i++) { - preview += exampleChars.charAt(i % exampleChars.length); + for (let i = 0; i < Math.min(len, RANDOM_PREVIEW_LENGTH); i++) { + preview += RANDOM_PREVIEW_CHARS.charAt(i % RANDOM_PREVIEW_CHARS.length); } - if (len > 6) { + if (len > RANDOM_PREVIEW_LENGTH) { preview += '...'; } return preview; }); return output; }src/gui/MultiChoiceSettingsModal.ts (2)
79-93: Rejecting the promise on cancel may cause unhandled rejection warnings.When the user cancels,
rejectPromise("No input given.")is called. If the caller useswaitForClosewithout a.catch()handler, this will produce an unhandled promise rejection warning. The service layer wraps this in try/catch (per the AI summary), but consider resolving withundefinedinstead for a more conventional "no result" pattern.🔎 Suggested improvement
onClose() { super.onClose(); if (!this.didSubmit) { - this.rejectPromise("No input given."); + this.resolvePromise(undefined); return; } const trimmedPlaceholder = this.placeholder.trim(); const updated: IMultiChoice = { ...this.choice, name: this.name.trim() || this.choice.name, placeholder: trimmedPlaceholder ? trimmedPlaceholder : undefined, }; this.resolvePromise(updated); }
55-66: Consider using CSS classes instead of inline styles.The button row uses inline styles for layout. This works but could be more maintainable with CSS classes, especially if similar patterns are used elsewhere in the codebase.
src/formatters/formatter-title.test.ts (1)
55-68: Consider using importedPromptContexttype instead of inline type definition.The inline context type duplicates the
PromptContextinterface structure. Importing and using the type would reduce duplication and ensure consistency if the interface changes.🔎 Suggested improvement
+import { Formatter, type PromptContext } from './formatter'; -import { Formatter } from './formatter'; // In TestFormatter class: protected promptForVariable( _variableName: string, - _context?: { - type?: string; - dateFormat?: string; - defaultValue?: string; - label?: string; - description?: string; - placeholder?: string; - variableKey?: string; - }, + _context?: PromptContext, ): Promise<string> { return Promise.resolve(""); }src/formatters/formatter-linkcurrent.test.ts (1)
33-73: LGTM! Stub signatures correctly updated to match base class API.The method signature updates appropriately extend the stub to align with the enhanced Formatter base class that now supports labels, placeholders, and richer prompt contexts. The underscore-prefixed parameters clearly indicate these are intentionally unused in the stub, which is appropriate since this test file focuses exclusively on link-to-current-file behavior.
Optional: Consider extracting context types to shared definitions.
The inline context object types (e.g.,
{ placeholder?: string; variableKey?: string },{ label?: string }) are acceptable for test stubs, but if the base Formatter or related code defines shared types likePromptContext(mentioned in the AI summary), importing and reusing those types would improve maintainability and ensure the stub stays in sync with future API changes.Example:
// If PromptContext is defined in src/formatters/formatter.ts or src/types import type { PromptContext, MacroContext } from "./formatter"; protected async promptForVariable( _variableName: string, _context?: PromptContext, ): Promise<string> { return ""; } protected async getMacroValue( _macroName: string, _context?: MacroContext, ): Promise<string> { return ""; }However, inline types are perfectly valid for test stubs, so this is purely a consistency consideration.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
docs/docs/Choices/MultiChoice.md(1 hunks)docs/docs/FormatSyntax.md(1 hunks)src/choiceExecutor.ts(2 hunks)src/constants.ts(3 hunks)src/engine/MacroChoiceEngine.ts(3 hunks)src/engine/SingleMacroEngine.ts(2 hunks)src/formatters/completeFormatter.ts(6 hunks)src/formatters/fileNameDisplayFormatter.ts(4 hunks)src/formatters/formatDisplayFormatter.ts(5 hunks)src/formatters/formatter-field-title-regression.test.ts(1 hunks)src/formatters/formatter-filenamecurrent.test.ts(1 hunks)src/formatters/formatter-issue920.test.ts(1 hunks)src/formatters/formatter-issue929-integration.test.ts(1 hunks)src/formatters/formatter-issue929.test.ts(1 hunks)src/formatters/formatter-linkcurrent.test.ts(1 hunks)src/formatters/formatter-template-property-types.test.ts(1 hunks)src/formatters/formatter-title.test.ts(1 hunks)src/formatters/formatter.ts(6 hunks)src/formatters/vdate-default.test.ts(3 hunks)src/gui/GenericInputPrompt/GenericInputPrompt.ts(4 hunks)src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts(5 hunks)src/gui/MultiChoiceSettingsModal.ts(1 hunks)src/gui/suggesters/choiceSuggester.ts(2 hunks)src/gui/suggesters/formatSyntaxSuggester.ts(2 hunks)src/preflight/RequirementCollector.test.ts(2 hunks)src/preflight/RequirementCollector.ts(6 hunks)src/services/choiceService.ts(3 hunks)src/types/choices/IMultiChoice.ts(1 hunks)src/types/choices/MultiChoice.ts(1 hunks)src/utils/macroSyntax.ts(1 hunks)src/utils/valueSyntax.test.ts(1 hunks)src/utils/valueSyntax.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- src/formatters/vdate-default.test.ts
- src/gui/suggesters/choiceSuggester.ts
- src/preflight/RequirementCollector.test.ts
- src/engine/MacroChoiceEngine.ts
- src/types/choices/IMultiChoice.ts
- src/engine/SingleMacroEngine.ts
- src/formatters/formatter-issue929.test.ts
- src/formatters/formatter-field-title-regression.test.ts
- src/formatters/formatter-issue920.test.ts
- src/formatters/formatter-issue929-integration.test.ts
- src/formatters/formatter-filenamecurrent.test.ts
- src/gui/suggesters/formatSyntaxSuggester.ts
- src/utils/valueSyntax.ts
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/utils/valueSyntax.test.tssrc/formatters/formatter-template-property-types.test.tssrc/types/choices/MultiChoice.tssrc/utils/macroSyntax.tssrc/gui/MultiChoiceSettingsModal.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/formatters/formatter-linkcurrent.test.tssrc/formatters/completeFormatter.tssrc/constants.tssrc/formatters/formatter-title.test.tssrc/formatters/formatter.tssrc/formatters/fileNameDisplayFormatter.tssrc/choiceExecutor.tssrc/services/choiceService.tssrc/preflight/RequirementCollector.tssrc/gui/GenericWideInputPrompt/GenericWideInputPrompt.tssrc/formatters/formatDisplayFormatter.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/utils/valueSyntax.test.tssrc/formatters/formatter-template-property-types.test.tssrc/types/choices/MultiChoice.tssrc/utils/macroSyntax.tssrc/gui/MultiChoiceSettingsModal.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/formatters/formatter-linkcurrent.test.tssrc/formatters/completeFormatter.tssrc/constants.tssrc/formatters/formatter-title.test.tssrc/formatters/formatter.tssrc/formatters/fileNameDisplayFormatter.tssrc/choiceExecutor.tssrc/services/choiceService.tssrc/preflight/RequirementCollector.tssrc/gui/GenericWideInputPrompt/GenericWideInputPrompt.tssrc/formatters/formatDisplayFormatter.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/utils/valueSyntax.test.tssrc/formatters/formatter-template-property-types.test.tssrc/types/choices/MultiChoice.tssrc/utils/macroSyntax.tssrc/gui/MultiChoiceSettingsModal.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/formatters/formatter-linkcurrent.test.tssrc/formatters/completeFormatter.tssrc/constants.tssrc/formatters/formatter-title.test.tssrc/formatters/formatter.tssrc/formatters/fileNameDisplayFormatter.tssrc/choiceExecutor.tssrc/services/choiceService.tssrc/preflight/RequirementCollector.tssrc/gui/GenericWideInputPrompt/GenericWideInputPrompt.tssrc/formatters/formatDisplayFormatter.ts
docs/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Keep user-facing docs in
docs/directory
Files:
docs/docs/FormatSyntax.mddocs/docs/Choices/MultiChoice.md
🧠 Learnings (1)
📚 Learning: 2025-12-09T21:20:52.425Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.425Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/utils/valueSyntax.test.tssrc/formatters/formatter-linkcurrent.test.tssrc/formatters/formatter-title.test.ts
🧬 Code graph analysis (11)
src/utils/valueSyntax.test.ts (1)
src/utils/valueSyntax.ts (2)
parseValueToken(122-156)buildValueVariableKey(17-25)
src/gui/MultiChoiceSettingsModal.ts (2)
tests/obsidian-stub.ts (3)
Modal(443-447)Setting(151-233)ButtonComponent(33-67)src/types/choices/IMultiChoice.ts (1)
IMultiChoice(3-7)
src/gui/GenericInputPrompt/GenericInputPrompt.ts (1)
docs/static/scripts/userScriptExample.js (1)
description(141-145)
src/formatters/completeFormatter.ts (3)
src/formatters/formatter.ts (1)
PromptContext(34-42)src/gui/InputSuggester/inputSuggester.ts (1)
InputSuggester(22-195)src/gui/GenericSuggester/genericSuggester.ts (1)
GenericSuggester(8-142)
src/formatters/formatter.ts (3)
src/utils/valueSyntax.ts (1)
parseValueToken(122-156)src/utils/macroSyntax.ts (1)
parseMacroToken(6-22)src/constants.ts (1)
MACRO_REGEX(94-94)
src/formatters/fileNameDisplayFormatter.ts (3)
src/utils/valueSyntax.ts (1)
getValueVariableBaseName(27-31)src/formatters/helpers/previewHelpers.ts (3)
getVariableExample(44-53)getSuggestionPreview(79-84)getMacroPreview(58-61)src/formatters/formatter.ts (1)
PromptContext(34-42)
src/choiceExecutor.ts (4)
src/settingsStore.ts (1)
settingsStore(8-21)src/preflight/runOnePagePreflight.ts (1)
runOnePagePreflight(138-268)src/main.ts (1)
QuickAdd(36-326)src/gui/suggesters/choiceSuggester.ts (1)
ChoiceSuggester(17-91)
src/services/choiceService.ts (2)
src/types/choices/IMultiChoice.ts (1)
IMultiChoice(3-7)src/gui/MultiChoiceSettingsModal.ts (1)
MultiChoiceSettingsModal(5-94)
src/preflight/RequirementCollector.ts (2)
src/utils/valueSyntax.ts (1)
parseValueToken(122-156)src/formatters/formatter.ts (1)
PromptContext(34-42)
src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts (1)
docs/static/scripts/userScriptExample.js (1)
description(141-145)
src/formatters/formatDisplayFormatter.ts (3)
src/utils/valueSyntax.ts (1)
getValueVariableBaseName(27-31)src/formatters/helpers/previewHelpers.ts (2)
getVariableExample(44-53)getSuggestionPreview(79-84)src/formatters/formatter.ts (1)
PromptContext(34-42)
🔇 Additional comments (37)
src/types/choices/MultiChoice.ts (1)
8-8: LGTM!The optional
placeholderproperty is correctly added to align with theIMultiChoiceinterface extension. This cleanly supports the new placeholder feature for Multi choices.src/choiceExecutor.ts (2)
36-60: LGTM!The preflight invocation is properly formatted for readability. The conditional logic correctly evaluates the one-page input override and global settings, and error handling appropriately catches cancellation errors.
124-129: LGTM!The
ChoiceSuggester.Opencall correctly uses the new options object API, passing thechoiceExecutorandplaceholderfrom the multi-choice. This enables the placeholder text feature in the choice picker.src/gui/GenericInputPrompt/GenericInputPrompt.ts (3)
17-17: LGTM!The
descriptionfield is correctly declared asprivate readonlywith optional typing.
21-56: LGTM!Both static factory methods correctly propagate the new
descriptionparameter to the constructor. The parameter ordering is consistent, withdescriptionappropriately placed as the final optional parameter.
58-68: LGTM!The constructor correctly accepts the new
descriptionparameter and initializes it with proper null-safe trimming. Empty or whitespace-only descriptions are normalized toundefined.src/gui/GenericWideInputPrompt/GenericWideInputPrompt.ts (1)
17-17: LGTM!The
descriptionfield implementation mirrorsGenericInputPromptexactly, maintaining consistency across both prompt variants:
- Field declaration with proper
private readonlytyping- Static factory methods correctly propagate the parameter
- Constructor initializes with null-safe trimming
- Rendering conditionally displays the description with appropriate styling
Also applies to: 21-37, 39-56, 58-68, 98-104
docs/docs/Choices/MultiChoice.md (1)
12-15: LGTM!The new documentation section clearly explains the placeholder text feature, its purpose, and behavior. The explanation is concise and helpful for users configuring complex multi-choice menus.
src/utils/valueSyntax.test.ts (1)
1-44: LGTM! Comprehensive test coverage for value token parsing.The test suite provides excellent coverage for the new
parseValueTokenfunctionality, including edge cases like empty labels, multiple labels, scoping behavior, legacy syntax compatibility, and combined option parsing.src/utils/macroSyntax.ts (1)
1-22: LGTM! Clean and robust macro token parsing.The implementation correctly handles all edge cases: empty input, missing delimiter, empty labels, and missing macro names. The logic is straightforward and maintainable.
src/constants.ts (2)
8-11: LGTM! New syntax constants follow existing conventions.The new
VARIABLE_DEFAULT_OPTION_SYNTAXandVARIABLE_LABEL_SYNTAXconstants are well-named and consistent with the existing syntax constant patterns.
32-33: LGTM! Syntax arrays correctly extended.The new syntax constants are properly added to both
FORMAT_SYNTAXandFILE_NAME_FORMAT_SYNTAXarrays, maintaining consistency across formatting contexts. The labeled macro syntax addition aligns with the new parsing capabilities.Also applies to: 41-41, 59-60
src/services/choiceService.ts (2)
43-48: LGTM! Improved immutability in duplicateChoice.The refactored approach constructs a fresh object instead of mutating the copy, which is cleaner and more functional. The explicit copying of
choices,placeholder, andcollapsedmakes the intent clear.
134-141: LGTM! Modal-based configuration improves UX.Replacing the in-line prompt with
MultiChoiceSettingsModalprovides a better user experience for editing multiple fields (name and placeholder). The try/catch properly handles cancellation by returningundefined.docs/docs/FormatSyntax.md (1)
13-16: LGTM! Clear and comprehensive documentation.The documentation effectively explains the new label, default, and custom syntax variants with helpful examples and important caveats (e.g., not mixing shorthand defaults with option keys, custom not combining with shorthand defaults). The macro label syntax is clearly documented.
Also applies to: 20-20
src/formatters/formatter-template-property-types.test.ts (1)
31-71: LGTM! Test fixture signatures correctly updated.The method signatures have been appropriately updated to match the new API surface with context parameters. The stub implementations remain suitable for testing purposes.
src/formatters/completeFormatter.ts (3)
193-224: LGTM! PromptContext properly threaded through variable prompts.The
promptForVariablemethod now correctly usesPromptContextto pass label, placeholder, defaultValue, and description to prompts. The fallback chainheader ?? context?.label ?? "Enter value"ensures a good default experience. Error handling is preserved.
237-267: LGTM! Context-aware suggestions enhance UX.The
suggestForValuemethod now accepts and conditionally passesplaceholderto bothInputSuggesterandGenericSuggester, improving the user experience when configured. The logic correctly handles theallowCustomInputbranching.
323-344: LGTM! Macro label context properly propagated.The
getMacroValuemethod now accepts alabelin the context and passes it through torunAndGetOutput. Variable propagation from macro execution is correctly preserved.src/formatters/fileNameDisplayFormatter.ts (4)
1-15: LGTM!The imports are correctly updated to include the new
PromptContexttype andgetValueVariableBaseNameutility, aligning with the broader API changes across the formatter system.
74-79: LGTM!Good use of
getValueVariableBaseNameto extract the base variable name before generating preview examples. This ensures consistent preview behavior when labels are attached to variable keys.
91-115: LGTM!The signature updates for
suggestForValue,getMacroValue, andpromptForVariablecorrectly extend the API to accept optional context objects while maintaining backward compatibility. The underscore-prefixed parameters appropriately indicate they are unused in this display formatter implementation.
117-133: LGTM!The new protected helper methods provide sensible preview values for template content, selected text, clipboard content, and field suggestions. These are appropriate for the filename display formatter's purpose of generating preview output.
src/gui/MultiChoiceSettingsModal.ts (1)
5-29: LGTM!The modal class structure follows the standard Obsidian modal pattern with promise-based closing. The constructor properly initializes state from the choice and sets up the promise for async completion.
src/formatters/formatter-title.test.ts (1)
32-53: LGTM!The test formatter stub correctly implements the updated method signatures with underscore-prefixed unused parameters, maintaining compatibility with the base class abstract methods.
src/formatters/formatter.ts (4)
29-42: LGTM!The new imports and
PromptContextinterface are well-structured. The interface provides a clean contract for passing rich context through the prompting flow, and the exports make it available to dependent formatters.
275-318: LGTM!The refactored variable handling correctly uses
parseValueTokenfor consistent parsing. The logic properly distinguishes between single-value prompts (usingdescription) and multi-option suggesters (usingplaceholder), and thevariableKeyensures proper scoping for labeled variables.
409-436: LGTM!The macro handling correctly uses
parseMacroTokenand gracefully handles parse failures by stripping the invalid macro and continuing. The optional label is properly passed through the context object.
569-577: LGTM!The updated abstract method signatures correctly define the context parameter contracts that subclasses must implement.
src/preflight/RequirementCollector.ts (3)
139-186: LGTM!The
scanVariableTokensmethod is properly refactored to useparseValueToken, ensuring consistent parsing across the codebase. The requirement creation logic correctly maps parsed token data to field requirements with appropriate types and metadata.
279-288: LGTM!The
suggestForValuesignature is correctly updated and the implementation appropriately defers toscanVariableTokensfor requirement collection, avoiding duplicate processing.
206-227: LGTM!The VDATE handling correctly uses the context's
variableKeyfor requirement keying and properly propagates the description metadata for richer UI display.src/formatters/formatDisplayFormatter.ts (5)
1-1: LGTM: Imports support context-rich prompting.The new imports enable threading of labels, placeholders, and descriptions through the formatter pipeline, aligning with the PR objectives for issues #513, #547, and #506.
Also applies to: 17-17
77-82: LGTM: Correctly handles labeled variable keys in preview mode.The updated logic extracts the base variable name for preview examples, which correctly supports the new labeled VALUE syntax while maintaining backward compatibility with unlabeled variables.
94-100: LGTM: Signature extends API surface for context support.The optional
_contextparameter enables consistent threading of placeholder and variableKey metadata through all formatters. The underscore prefix correctly indicates this preview formatter doesn't consume the context.
102-107: LGTM: Supports macro label syntax.The optional
_contextparameter with label field enables the new macro label syntax (issue #547) to flow through the formatter pipeline. Appropriate that the preview formatter doesn't consume it.
113-118: LGTM: Type upgrade improves API clarity.Switching from a plain object to the structured
PromptContexttype improves type safety and consistency across the formatter hierarchy while maintaining backward compatibility through the optional parameter.
|
🎉 This PR is included in version 2.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
|label:(with|default:/|customas options); list labels now scope variables per label{{MACRO:macroName|label:Label}}used as placeholder when choosing script exportsvalueSyntaxandmacroSyntaxTesting
Issues
Closes #513
Closes #547
Fixes #506
Refs #337
Refs #1052