feat: per file custom delim overrides #261
Conversation
|
Warning Rate limit exceeded@hay-kot has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
WalkthroughThis pull request introduces enhancements for custom templating. It adds a new ignore rule for macOS’s Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RWFS as RenderRWFS
participant Engine
participant Template
User->>RWFS: Invoke rendering with project config (incl. delimiters)
RWFS->>Engine: Call Factory(reader, opfns...) with default opts
Engine->>Engine: Initialize opts with default delimiters (e.g., "{{", "}}")
Engine->>Engine: Apply any WithDelims options based on project config
Engine->>Template: Create template using the resolved custom delimiters
Template-->>Engine: Return parsed template
Engine-->>RWFS: Return template for execution
RWFS-->>User: Output rendered result
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
docs/docs/components/Featured.vue (1)
37-44:⚠️ Potential issueFix typo in type definition.
There's a typo in the Props type definition:
deaturesshould befeatures.type Props = { features: { href: string; name: string; description: string; - deatures: string[]; + features: string[]; }[]; };
🧹 Nitpick comments (5)
app/scaffold/project_scaffold_file.go (1)
22-26: Consider adding field validation for delimiters.While the struct is well-defined, consider adding validation to ensure that:
- The glob pattern is valid
- The delimiters are non-empty and distinct
Example validation in
ReadScaffoldFile:func ReadScaffoldFile(reader io.Reader) (*ProjectScaffoldFile, error) { var out ProjectScaffoldFile err := yaml.NewDecoder(reader).Decode(&out) if err != nil { return nil, err } + + // Validate delimiters + for _, d := range out.Delimiters { + if d.Left == "" || d.Right == "" { + return nil, fmt.Errorf("delimiters cannot be empty") + } + if d.Left == d.Right { + return nil, fmt.Errorf("left and right delimiters must be different") + } + if _, err := filepath.Match(d.Glob, "test"); err != nil { + return nil, fmt.Errorf("invalid glob pattern: %s", d.Glob) + } + } return &out, nil }docs/docs/components/Featured.vue (1)
58-64: Remove redundant cursor style.The
cursor: pointeron.feature-gridis redundant as the childaelements already have this style. This could cause confusion about which areas are clickable..feature-grid { - cursor: pointer; width: 100%; display: grid; margin: 2rem 0; gap: 1rem; }app/core/engine/engine.go (1)
91-117: Consider documenting the default delimiters.The Factory method initializes default delimiters but this isn't documented in the function's comment.
// Factory returns a new template from the provided reader. // if the reader is empty, an ErrTemplateIsEmpty is returned. +// By default, the template uses "{{" and "}}" as delimiters unless +// overridden using WithDelims. func (e *Engine) Factory(reader io.Reader, opfns ...func(*opts)) (*template.Template, error) {app/scaffold/render_funcs.go (1)
273-288: Consider caching delimiter lookups for performance.The current implementation looks up delimiters for each file. For better performance, consider:
- Pre-compiling glob patterns
- Caching delimiter matches for similar paths
+var defaultDelims = struct{ left, right string }{"{{", "}}"} + func RenderRWFS(eng *engine.Engine, args *RWFSArgs, vars engine.Vars) error { + // Pre-compile glob patterns + type delimCache struct { + pattern *doublestar.Pattern + left, right string + } + delims := make([]delimCache, 0, len(args.Project.Conf.Delimiters)) + for _, d := range args.Project.Conf.Delimiters { + pattern, err := doublestar.PathMatch(d.Glob, "") + if err != nil { + return fmt.Errorf("invalid glob pattern %q: %w", d.Glob, err) + } + delims = append(delims, delimCache{pattern, d.Left, d.Right}) + }docs/docs/templates/config-reference.md (1)
300-303: Fix grammar in documentation.The sentence should use "are" instead of "is" since "delimiters" is plural.
-delimiters is a list of delimiter overrides for files. +delimiters are a list of delimiter overrides for files.🧰 Tools
🪛 LanguageTool
[grammar] ~302-~302: Did you mean “are” or “were”?
Context: ...ation. ::: ##delimitersdelimiters is a list of delimiter overrides for files...(SENT_START_NNS_IS)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.gitignore(1 hunks).golangci.toml(0 hunks)app/core/engine/engine.go(5 hunks)app/scaffold/.snapshots/render_rwfs/Test_RenderRWFileSystem-custom_delims.snapshot(1 hunks)app/scaffold/main_test.go(2 hunks)app/scaffold/project_scaffold_file.go(1 hunks)app/scaffold/render_funcs.go(2 hunks)app/scaffold/snapshot_test.go(1 hunks)app/scaffold/testdata/projects/custom_delims/{{ .Project }}/custom.txt(1 hunks)app/scaffold/testdata/projects/custom_delims/{{ .Project }}/standard.txt(1 hunks)docs/docs/components/Featured.vue(1 hunks)docs/docs/templates/config-reference.md(1 hunks)docs/schema/schema.scaffold.ts(1 hunks)
💤 Files with no reviewable changes (1)
- .golangci.toml
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- app/scaffold/.snapshots/render_rwfs/Test_RenderRWFileSystem-custom_delims.snapshot
🧰 Additional context used
🪛 LanguageTool
docs/docs/templates/config-reference.md
[grammar] ~302-~302: Did you mean “are” or “were”?
Context: ...ation. ::: ## delimiters delimiters is a list of delimiter overrides for files...
(SENT_START_NNS_IS)
app/scaffold/testdata/projects/custom_delims/{{ .Project }}/custom.txt
[uncategorized] ~1-~1: You might be missing the article “the” here.
Context: This should be project name = [[ .Project ]] This should rende...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
app/scaffold/testdata/projects/custom_delims/{{ .Project }}/standard.txt
[uncategorized] ~1-~1: You might be missing the article “the” here.
Context: This should be project name = {{ .Project }} This should rende...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (12)
app/scaffold/project_scaffold_file.go (1)
9-20: LGTM! Clean addition of the Delimiters field.The placement of the new field at the end of the struct maintains backward compatibility, and the YAML tag is properly defined.
app/scaffold/snapshot_test.go (1)
49-53:✅ Verification successful
LGTM! Good test coverage for custom delimiters.
The test case follows the established pattern and tests the new functionality.
Verify that the test helper functions exist:
🏁 Script executed:
#!/bin/bash # Search for the helper functions used in the test rg -A 5 'func CustomDelimsFiles\(\)' . rg -A 5 'func CustomDelimsProject\(\)' .Length of output: 752
LGTM: Custom delimiters test case correctly exercises the new functionality.
- Verified that both
CustomDelimsFiles()andCustomDelimsProject()exist inapp/scaffold/main_test.go.- The test follows the established pattern and effectively tests the custom delimiters feature.
app/core/engine/engine.go (2)
21-24: LGTM! Clean struct design for delimiter options.The
optsstruct is well-focused and encapsulates the delimiter configuration.
53-58: LGTM! Clean method update for custom delimiters.The
parsemethod cleanly integrates the custom delimiters while maintaining the existing functionality.app/scaffold/main_test.go (2)
17-18: LGTM! Good test data organization.The embedded filesystem for custom delimiters test data follows the established pattern in the codebase.
143-146: LGTM! Well-structured test helper functions.The helper functions
CustomDelimsFiles()andCustomDelimsProject()are well-implemented:
- Follow consistent patterns with other test helpers
- Provide clear test configuration for custom delimiters
- Use meaningful glob patterns and delimiter values
Also applies to: 148-162
docs/schema/schema.scaffold.ts (2)
32-36: Fixed typo in property name.Good catch on fixing the typo from
presentstopresets.
38-42: LGTM! Well-documented schema changes.The new
delimitersproperty and type are:
- Well-documented with clear comments
- Have proper TypeScript types
- Follow the schema's documentation style
Also applies to: 44-54
app/scaffold/render_funcs.go (1)
289-302: LGTM! Good error handling.The error handling for template creation and delimiter matching is robust:
- Proper cleanup with
f.Close()- Clear error propagation
docs/docs/templates/config-reference.md (1)
304-311: LGTM! Clear documentation with practical example.The documentation:
- Provides a clear use case with
.goreleaser- Shows a complete configuration example
- Explains the benefits of custom delimiters
app/scaffold/testdata/projects/custom_delims/{{ .Project }}/custom.txt (1)
1-2: LGTM! Test data properly validates custom delimiter functionality.The test data correctly demonstrates:
- Custom delimiter processing with
[[ .Project ]]- Standard delimiter preservation with
{{ .Project }}This effectively validates the per-file custom delimiter override feature.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~1-~1: You might be missing the article “the” here.
Context: This should be project name = [[ .Project ]] This should rende...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
app/scaffold/testdata/projects/custom_delims/{{ .Project }}/standard.txt (1)
1-2: LGTM! Test data provides complementary coverage.The test data correctly demonstrates the inverse scenario:
- Standard delimiter processing with
{{ .Project }}- Custom delimiter preservation with
[[ .Project ]]This effectively complements the custom.txt test case and validates proper per-file delimiter scoping.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~1-~1: You might be missing the article “the” here.
Context: This should be project name = {{ .Project }} This should rende...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
| func WithDelims(left string, right string) func(*opts) { | ||
| return func(o *opts) { | ||
| o.delimLeft = left | ||
| o.delimRight = right | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation in WithDelims function.
The function should validate that the delimiters are non-empty and distinct.
func WithDelims(left string, right string) func(*opts) {
+ if left == "" || right == "" {
+ panic("delimiters cannot be empty")
+ }
+ if left == right {
+ panic("left and right delimiters must be different")
+ }
return func(o *opts) {
o.delimLeft = left
o.delimRight = right
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func WithDelims(left string, right string) func(*opts) { | |
| return func(o *opts) { | |
| o.delimLeft = left | |
| o.delimRight = right | |
| } | |
| } | |
| func WithDelims(left string, right string) func(*opts) { | |
| if left == "" || right == "" { | |
| panic("delimiters cannot be empty") | |
| } | |
| if left == right { | |
| panic("left and right delimiters must be different") | |
| } | |
| return func(o *opts) { | |
| o.delimLeft = left | |
| o.delimRight = right | |
| } | |
| } |
f4395b5 to
3126ce9
Compare
Purpose
When working with files that contain go templates, like
goreleaserfiles, it's annoying to have to escape those template values that should get rendered out. This feature allows a user to specify a glob pattern to substitute the opening and closing brackets for the templates.This let's the file stay mostly in tack and the user can use a different syntax to template the file.
Proposed Changes
Checklist
Summary by CodeRabbit
New Features
Documentation
Tests
Chores & Style