add basic formatting functions #518
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors string_format to formatString for consistency and introduces several new basic formatting functions: formatNumber, formatCurrency, formatDate, and pluralize. The changes are mostly well-implemented, but there is a critical issue with a broken JSON schema reference in the new formatDate function. I've also included some feedback on API consistency and the implementation of the pluralize function for your consideration.
| "value": { | ||
| "$ref": "common_types.json#/$defs/DynamicDate", | ||
| "description": "The date to format." | ||
| }, |
There was a problem hiding this comment.
The schema reference common_types.json#/$defs/DynamicDate is broken, as DynamicDate is not defined in common_types.json. This will cause validation failures. Based on other components like DateTimeInput, dates are typically handled as strings (ISO 8601) or numbers (timestamps). You should change this to a valid type, such as DynamicValue to support both strings and numbers.
| "value": { | |
| "$ref": "common_types.json#/$defs/DynamicDate", | |
| "description": "The date to format." | |
| }, | |
| "value": { | |
| "$ref": "common_types.json#/$defs/DynamicValue", | |
| "description": "The date to format, typically as an ISO 8601 string or a numeric timestamp." | |
| }, |
| "parameters": { | ||
| "type": "object", | ||
| "required": [ "value" ], | ||
| "properties": { | ||
| "value": { | ||
| "$ref": "common_types.json#/$defs/DynamicNumber", | ||
| "description": "The number to format." | ||
| }, | ||
| "decimals": { | ||
| "$ref": "common_types.json#/$defs/DynamicNumber", | ||
| "description": "Optional. The number of decimal places to show. Defaults to 0 or 2 depending on locale." | ||
| }, | ||
| "useGrouping": { | ||
| "$ref": "common_types.json#/$defs/DynamicBoolean", | ||
| "description": "Optional. If true, uses locale-specific grouping separators (e.g. '1,000'). If false, returns raw digits (e.g. '1000'). Defaults to true." | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The new formatting functions (formatNumber, formatCurrency, etc.) use named parameters (an object with properties), while existing functions (required, regex, formatString) use positional parameters (an array of values). This introduces an inconsistency in the function catalog's API design. While named parameters are great for functions with optional arguments, this shift in pattern could be confusing for developers. It would be beneficial to document this as the new standard for functions or consider a plan to migrate older functions for better consistency.
| { | ||
| "name": "pluralize", | ||
| "description": "Returns the singular string if the count is 1, otherwise returns the plural string.", | ||
| "returnType": "string", | ||
| "parameters": { | ||
| "type": "object", | ||
| "required": [ "value", "singular", "plural" ], | ||
| "properties": { | ||
| "value": { | ||
| "$ref": "common_types.json#/$defs/DynamicNumber", | ||
| "description": "The numeric value to check." | ||
| }, | ||
| "singular": { | ||
| "$ref": "common_types.json#/$defs/DynamicString", | ||
| "description": "The string to return if count is exactly 1." | ||
| }, | ||
| "plural": { | ||
| "$ref": "common_types.json#/$defs/DynamicString", | ||
| "description": "The string to return if count is not 1." | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The pluralize function is described as returning the plural form if the count is not 1. This is a very basic approach to pluralization and doesn't align with the locale-aware nature of the other new formatting functions. It will not work correctly for many languages and even has edge cases in English (e.g., 0 items, -1 items).
For better internationalization support, I recommend adopting a more robust mechanism based on CLDR plural rules, which include categories like zero, one, two, few, many, and other. A more flexible function signature could accept a map of these categories to their corresponding strings.
1 participant
add basic formatting functions