Getopts built-in

[magicant]

Summary by CodeRabbit

• New Features

  • Implemented a new getopts built-in for enhanced option parsing in shell scripts.
  • Introduced new error handling and reporting mechanisms for option parsing.
  • Added state verification to ensure consistency across multiple calls to getopts.

• Bug Fixes

  • Improved logic in the next function for more accurate command-line option parsing.

• Documentation

  • Updated documentation to reflect new data structures and error types related to getopts.

• Tests

  • Expanded test suite with new test cases for getopts functionality.
  • Added a new scripted test file getopts-p.sh for comprehensive testing of getopts behavior.

• Chores

  • Added license information to new files.
  • Integrated getopts module into the Env struct for state management.

[coderabbitai]

Walkthrough

The yash shell has received a comprehensive update to its getopts built-in, with new modules for parsing, reporting, and verifying command-line options. The Env structure now includes state management for getopts, and the test suite has been expanded to ensure robustness. These changes modernize the shell's option handling and improve error reporting and consistency checks across invocations.

Changes

File Path	Change Summary
yash-builtin/src/getopts.rs, yash-builtin/src/getopts/...	Added new modules (model, report, verify), helper functions, and tests for getopts built-in. Modified main function to use async and handle option parsing and reporting.
yash-builtin/src/lib.rs, yash-env/src/builtin.rs	Added getopts module to the list of imported modules and to the BUILTINS constant.
yash-env/src/builtin/getopts.rs	Added license information, new data structures (Origin, GetoptsState), and documentation updates.
yash-env/src/lib.rs	Added getopts_state field to Env struct for state management.
yash/tests/scripted_test.rs, yash/tests/scripted_test/getopts-p.sh	Added new test function getopts_builtin and a series of tests for the getopts built-in.

🐇✨
In the land of code, where the shell scripts run,
A rabbit hopped in, with a task well done.
getopts refined, with a parser so bright,
Now scripts can dance, in the moon's soft light. 🌙
✨🐇

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• If you reply to a review comment from CodeRabbit, the bot will automatically respond.
• To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
• Note: Review comments are made on code diffs or files, not on the PR overview.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 75953b7 and 5f77db5.

Files ignored due to filter (2)

• Cargo.lock
• yash-builtin/Cargo.toml

Files selected for processing (10)

• yash-builtin/src/getopts.rs (1 hunks)
• yash-builtin/src/getopts/model.rs (1 hunks)
• yash-builtin/src/getopts/report.rs (1 hunks)
• yash-builtin/src/getopts/verify.rs (1 hunks)
• yash-builtin/src/lib.rs (2 hunks)
• yash-env/src/builtin.rs (1 hunks)
• yash-env/src/builtin/getopts.rs (1 hunks)
• yash-env/src/lib.rs (5 hunks)
• yash/tests/scripted_test.rs (1 hunks)
• yash/tests/scripted_test/getopts-p.sh (1 hunks)

Files skipped from review due to trivial changes (2)

• yash-env/src/builtin.rs
• yash/tests/scripted_test/getopts-p.sh

Additional comments: 29

yash-builtin/src/getopts.rs (4)

• 199-201: The addition of new modules (model, report, verify) aligns with the PR objectives and the AI-generated summaries. These modules are expected to handle parsing, reporting, and state verification of command-line options.

• 203-220: The helper functions indexes_from_optind and indexes_to_optind are implemented to manage the $OPTIND value. Ensure that these functions are thoroughly tested, especially for edge cases and abnormal values.

• 223-294: The main function has been refactored to use asynchronous programming patterns and includes logic for parsing command-line options and reporting results. It is crucial to ensure that the asynchronous behavior is consistent with the rest of the shell's environment handling and that it does not introduce any race conditions or unexpected behavior.

• 296-338: New test cases have been added to validate the functionality of the indexes_from_optind and indexes_to_optind functions. It is important to verify that these tests cover a wide range of inputs, including normal and abnormal values, to ensure the robustness of the getopts functionality.

yash-builtin/src/getopts/model.rs (5)

• 19-20: The use of thiserror crate for error handling is a good practice as it provides a convenient way to define and manage errors in Rust.

• 22-31: The OptionType enum is well-defined and covers all possible types of options (NoArgument, TakesArgument, and Unknown), which is essential for the correct parsing of command-line options.

• 33-37: The OptionSpec struct is minimal and focused, holding only the raw string representation of the option specification. This is a good example of keeping data structures simple and focused on their purpose.

• 136-211: The next function's logic is complex but seems to be correct. It handles parsing options from command-line arguments, including edge cases like double hyphens and options with or without arguments. The use of pattern matching and options is idiomatic in Rust and enhances readability and maintainability.

• 213-574: The tests are comprehensive and cover a wide range of scenarios. This thorough testing is crucial for ensuring the reliability of the getopts parsing logic.

yash-builtin/src/getopts/report.rs (3)

• 33-57: The Error enum in the report.rs file provides detailed error handling for reporting results to the environment, which is a crucial aspect of the getopts functionality. This implementation is not explicitly mentioned in the summary, which could be considered an oversight given the importance of error handling in command-line option parsing.

• 129-211: The report function within the report.rs file is a key component of the new getopts implementation, as it updates environment variables based on the parsing results. This function's implementation details are not mentioned in the summary, which is a significant omission given its role in the new getopts functionality.

• 213-641: The test module within the report.rs file includes a comprehensive set of test cases that validate various aspects of the getopts functionality. The presence and importance of these tests are not mentioned in the summary, which is an oversight given their role in ensuring the correctness and reliability of the getopts built-in.

yash-builtin/src/getopts/verify.rs (7)

• 17-22: The documentation provides a clear explanation of the purpose of the state verification for the getopts built-in. This is important for maintainability and understanding the intent of the code.

• 28-37: The Error enum is correctly defined and uses the thiserror crate for error handling, which is a good practice in Rust for creating and managing custom error types.

• 39-45: The GetoptsStateRef struct is well-defined and its fields are appropriate for the intended purpose of state verification.

• 47-54: The implementation of the From trait for GetoptsStateRef is idiomatic and correctly converts a reference to GetoptsState into a GetoptsStateRef.

• 57-70: The into_state method is correctly implemented, allowing for the conversion of a GetoptsStateRef back into a GetoptsState by cloning the referenced data.

• 72-103: The verify method's logic is correct. It ensures that the getopts built-in receives consistent arguments and $OPTIND values across calls, with special handling when $OPTIND is 1.

• 107-192: The tests are comprehensive and cover various scenarios, ensuring that the state verification logic works as expected.

yash-builtin/src/lib.rs (3)

• 59-59: The addition of the getopts module is consistent with the PR objectives and the summary provided.

• 161-164: The addition of the getopts entry to the BUILTINS constant with type Mandatory and execution function getopts::main is correct and aligns with the PR objectives and the summary.

• 157-167: The test to ensure that the BUILTINS array is sorted alphabetically is a good practice for maintainability.

yash-env/src/builtin/getopts.rs (2)

• 17-44: The summary mentions the addition of an Error enum, but it is not present in the provided hunk. Please verify if the Error enum is defined elsewhere in the code or if the summary is incorrect.

• 43-43: The optind field in the GetoptsState struct is of type String, which is unconventional as OPTIND is typically an integer. Please verify if this is intentional and consider using an integer type if appropriate.

yash-env/src/lib.rs (4)

• 34-37: The addition of the GetoptsState import and its use in the Env struct aligns with the PR's objective to enhance the getopts functionality.

• 84-127: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [84-138]

The Env struct has been successfully updated to include the getopts_state field, which is necessary for the new getopts functionality.

• 151-157: The default constructor for the Env struct now initializes the getopts_state field, which is necessary for the new getopts functionality.

• 183-189: The clone_with_system method of the Env struct has been updated to include the getopts_state field, ensuring that the state is preserved when cloning the environment with a new system.

yash/tests/scripted_test.rs (1)

• 134-137: The addition of the getopts_builtin test function is consistent with the PR's objective to enhance the getopts functionality and the summary provided. This new test will help ensure the robustness of the getopts built-in.