-
-
Notifications
You must be signed in to change notification settings - Fork 683
feat(core/formatter): add slice, sort, and merge array modifiers #493
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
WalkthroughExtends the formatter modifier system in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 (1)
packages/core/src/formatters/base.ts (1)
754-768: Slice modifier implementation is correct.The logic correctly handles both
slice(start)andslice(start, end)patterns with appropriate fallback behaviour.Minor style inconsistency: Line 757 has
_mod.length -1whilst line 771 uses_mod.length - 1with proper spacing around the operator.Apply this diff for consistency:
- const args = _mod - .substring(6, _mod.length -1) - .split(',') - .map((arg) => parseInt(arg.trim(), 10)); + const args = _mod + .substring(6, _mod.length - 1) + .split(',') + .map((arg) => parseInt(arg.trim(), 10));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/formatters/base.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-14T16:46:18.507Z
Learnt from: DavidGracias
Repo: Viren070/AIOStreams PR: 375
File: packages/core/src/formatters/base.ts:0-0
Timestamp: 2025-09-14T16:46:18.507Z
Learning: In packages/core/src/formatters/base.ts, the modifier definitions intentionally include regex patterns (like "join('.*?')", "$.*?", "^.*?") that require pattern matching functionality. The buildModifierRegexPattern() method should not fully escape all regex metacharacters as this would break the intended regex matching behavior within modifier definitions.
Applied to files:
packages/core/src/formatters/base.ts
🔇 Additional comments (6)
packages/core/src/formatters/base.ts (6)
592-594: LGTM! Context parameter properly propagated.The addition of
parseValueas a parameter enables context-aware modifiers like merge to access other variables from the parse context.
636-637: LGTM! Return type appropriately extended.The extended return type
any[]allows array modifiers to preserve array types through chaining, which is essential for the new slice, sort, and merge functionality.
769-791: LGTM! Robust sort implementation with numeric awareness.The implementation correctly:
- Validates the direction parameter
- Uses spread operator to avoid mutation
- Provides numeric-aware sorting via
localeComparewithnumeric: trueoption- Handles both pure numeric and mixed-type arrays appropriately
797-820: LGTM! Merge implementation with sensible fallback.The implementation correctly:
- Validates the variable path format (requires
{...}wrapper)- Checks that the target property is an array before merging
- Gracefully falls back to the original array if the merge target is invalid
Note: The merge modifier doesn't support modifiers within the path (e.g.,
merge('{stream.audioChannels::slice(0, 2)}')), which is a reasonable design choice to maintain simplicity.
964-972: LGTM! Type signature appropriately broadened.The updated signature
any[]enables the sort modifier to handle arrays containing numbers, strings, or mixed types whilst maintaining numeric-aware comparison for consistency with the new parametrised sort modifier.
1025-1030: LGTM! Regex patterns correctly defined.The hardcoded modifier patterns appropriately:
- Match both single-argument
slice(n)and two-argumentslice(n, m)forms with flexible whitespace- Cover both ascending and descending sort directions
- Support both single and double quote delimiters for merge
|
These are very useful, hope this gets merged soon. |
|
hey @be-na17, sorry for the late response, but do you mind splitting the merge modifier into a separate PR? The slice and sort are more simple and I can merge those but I would like to explore an alternative approach with the merge modifier where syntax could be something like {stream.audioTags::merge::stream.audioChannels}. I'm not sure how I feel with the current method and I'd need to test it myself. |
|
Hey, unfortunately I won't be in front of a computer for the foreseeable future. |
|
Ah no worries. I'll do just that, thanks! |
…numerical sorting to `sort` Co-authored-by: be-na17 <[email protected]>
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.
Pull request overview
This PR adds new array manipulation modifiers (slice, sort, rsort, lsort, and reverse) to the formatter system, allowing users to extract subarrays, sort arrays in various ways, and reverse arrays before applying other transformations.
Changes:
- Added
slice(start, end)modifier for extracting array subarrays with numeric indices - Modified
sortto use numeric-aware sorting and addedrsort(reverse sort) andlsort(lexical sort) modifiers - Changed array modifier return type from strings to arrays, allowing chaining of array operations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Adds three new modifiers for arrays
Slice modifier
Usage: ::slice(start, end) or ::slice(start)
Example:
{stream.visualTags::slice(0, 2)}Input:
10bit, HDR, DV, 3D, IMAXOutput:
10bit, HDRSort modifier
Usage: ::sort(asc) or ::sort(desc)
Example (alphabetical):
{stream.audioTags::sort(asc)}Input:
Atmos, DTS-HD MA, TrueHD, OPUS, AACOutput:
AAC, Atmos, DTS-HD MA, OPUS, TrueHDExample (numerical):
{stream.seasons::sort(desc)}Input:
1, 2, 3, 4, 5, 6, 7, 8Output:
8, 7, 6, 5, 4, 3, 2, 1Note: Combined alphabetical/numerical sorting is possible
Merge modifier
Usage: ::merge('{example.Array}')
Example:
{stream.audioTags::merge('{stream.audioChannels}')}Input:
Atmos, TrueHD + 7.1Output:
Atmos, TrueHD, 7.1Combined usage
Example:
{stream.visualTags::merge('{stream.audioTags}')::merge('{stream.audioChannels}')::sort(asc)::slice(0, 4)::join(' - ')}Input:
HDR10, DV + Atmos, TrueHD + 7.1Output:
7.1 - Atmos - DV - HDR10Technical changes
Updated applySingleModifier: Added support for returning arrays, added
parseValueparameter to support context access for merging.Summary by CodeRabbit
New Features
Other
✏️ Tip: You can customize this high-level summary in your review settings.