rainerscript: add split() function#6384
Conversation
Summary of ChangesHello @20syldev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a highly anticipated Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new split() function to RainerScript, which is a useful addition for string manipulation. The implementation in grammar/rainerscript.c is generally sound and covers several edge cases. The new documentation and tests are also appreciated.
I have a couple of suggestions to improve the error handling within the doFunct_split function. These changes will make the function more robust, especially in out-of-memory scenarios, by ensuring it fails cleanly instead of returning partial results.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a useful split() function to RainerScript for parsing delimited strings. The implementation is well-structured, including proper error handling and memory management. The new function is also documented and accompanied by tests. My review focuses on improving the function's correctness for edge cases and enhancing test coverage to ensure its robustness.
There was a problem hiding this comment.
Code Review
This pull request introduces a new split() function to RainerScript, which is a great addition for parsing delimited strings. The implementation is solid, and the accompanying documentation and tests are comprehensive.
I have two main suggestions:
- In
grammar/rainerscript.c, the error handling forNULLinputs could be improved to be more consistent. Currently, it returns an empty array, which might hide an underlying memory allocation failure. ReturningNULLwould be a clearer way to signal an error. - In the new documentation file
doc/source/rainerscript/functions/rs-split.rst, a newline is missing at the end of the file. It's a minor point but good practice to add one.
Overall, this is a well-executed feature addition.
There was a problem hiding this comment.
Code Review
This pull request introduces a new split() function to RainerScript, which is a useful addition for parsing delimited strings. The implementation is robust and includes good test coverage for various edge cases. I have one suggestion regarding the error handling and structure of the doFunct_split function to improve its maintainability and clarity. Overall, this is a solid contribution.
rgerhards
left a comment
There was a problem hiding this comment.
- code style needs to be fixed, can be done by running
develtools/format-code.sh- if that does not work due to dependencies, let me know and I will do and ammend to your PR - the doc should contain a complete example also showing how to process after the split (for a sample use case)
Users need to parse delimited strings (CSV, tags, paths) into arrays for iteration or JSON output without external processing. Impact: New RainerScript function available to all users. Before: No native way to split strings into arrays in RainerScript. After: split(string, separator) returns a JSON array of substrings. Technical overview: Implements doFunct_split() in grammar/rainerscript.c Registers "split" in scriptFunct table with 2 required args Adds CNFFUNC_SPLIT enum in rainerscript.h Uses unified strstr-based iteration for all separator lengths Handles edge cases: empty input, leading/trailing/consecutive delimiters Includes error handling for json-c memory allocation failures Returns empty JSON array on null/empty input or separator Includes documentation (rs-split.rst) and test scripts
My bad, I did the code formatting, but I forgot to redo it just before pushing. |
Summary
Users need to parse delimited strings (CSV, tags, paths) into arrays for iteration or JSON output without external processing.
Notes
rscript_split.shandrscript_split-vg.shdoc/source/rainerscript/functions/rs-split.rstImpact: New RainerScript function available to all users.
split(string, separator)returns a JSON array of substrings.