Typeset built-in for functions

[magicant]

Summary by CodeRabbit

• New Features
  • Introduced enhanced function attributes and error handling capabilities.
  • Added ability to print shell functions.
  • Implemented functionality to modify function attributes.
• Tests
  • Added tests for new functionalities including printing and modifying function attributes.
• Documentation
  • Updated license information.

[coderabbitai]

Walkthrough

The changes introduce new functionality to the yash-builtin package, enhancing the handling of shell functions and their attributes. The updates include the addition of new modules, methods, and tests, focusing on printing and setting function attributes, and improved error handling.

Changes

File Path	Change Summary
yash-builtin/src/typeset.rs	Introduced a new module print_functions, added a new Function import, defined a new FunctionAttr enum with a test method, and expanded error handling with a new ModifyUnsetFunction variant.
yash-builtin/src/typeset/print_functions.rs	Added a new PrintFunctions struct with an execute method for printing shell functions, helper functions for printing individual functions, and tests for the new functionality. Updated the license information.
yash-builtin/src/typeset/set_functions.rs	Updated the SetFunctions struct with an execute method for modifying function attributes in a FunctionSet. Included tests for handling function attributes and non-existing functions.

Poem

🍂 As autumn leaves fall, code changes enthrall, 🍁

🐇 With functions set and printed, no errors permitted. 🎉

On this day of remembrance, we celebrate code's elegance, 🌟

And hop towards progress, with joy and effervescence! 🚀

---

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: 1

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between b50e851 and c4c2b0a.

Files selected for processing (3)

• yash-builtin/src/typeset.rs (6 hunks)
• yash-builtin/src/typeset/print_functions.rs (1 hunks)
• yash-builtin/src/typeset/set_functions.rs (1 hunks)

Additional comments: 11

yash-builtin/src/typeset/set_functions.rs (2)

• 23-69: The execute method is well-structured and handles different cases for function attributes. It collects errors in a vector and returns them if any exist, otherwise it returns an empty string. This is a good approach for error handling.

• 161-174: The tests are comprehensive and cover all the scenarios for the execute method. They test making existing functions read-only, unsetting read-only attributes, and handling non-existing functions. The use of assertions ensures the expected behavior.

yash-builtin/src/typeset/print_functions.rs (4)

• 1-15: Ensure that the license information is updated correctly and is consistent with the rest of the project.

• 22-49: The execute method in the PrintFunctions struct is well implemented. It handles both the cases where specific functions are to be printed and where all functions are to be printed. It also correctly handles errors when a function to be printed does not exist.

• 52-72: The helper function print_one correctly filters functions based on attributes and formats the function for printing. It also handles the case where the function is read-only.

• 74-260: The tests cover a wide range of scenarios and seem to be well implemented. They test the printing of one function, multiple functions, all functions, quoting of function names, printing of read-only functions, selecting of read-only and non-read-only functions, and handling of non-existing functions.

yash-builtin/src/typeset.rs (5)

• 256-270: The new print_functions module is imported and the Function import is added. Ensure that the print_functions module is correctly implemented and that the Function import is used appropriately in the code.

• 347-359: A new FunctionAttr enum is defined with a test method. This seems to be a good way to handle function attributes. Ensure that the test method is correctly implemented and used.

• 490-497: A new ModifyUnsetFunction variant is added to the ExecuteError enum. This is a good way to handle errors related to modifying unset functions. Ensure that this new variant is correctly handled in the error handling code.

• 504-510: The MessageBase implementation is updated to handle the new ModifyUnsetFunction variant. This is a good way to provide error messages for the new error variant. Ensure that the error message is informative and user-friendly.

• 525-531: A new match arm is added in the main_annotation method to handle the ModifyUnsetFunction variant. This is a good way to provide annotations for the new error variant. Ensure that the annotation is informative and user-friendly.

[coderabbitai]

The use of unwrap_or_else with unreachable is risky. If the define function ever fails, it will cause a panic. Consider handling this error gracefully.

- .unwrap_or_else(|e| unreachable!("{e:?}"));
+ .map_err(|e| errors.push(ExecuteError::FunctionDefineError(name.clone(), e)))?;

---

Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.

Suggested change

	.unwrap_or_else(|e| unreachable!("{e:?}"));
	.map_err(|e| errors.push(ExecuteError::FunctionDefineError(name.clone(), e)))?;

[magicant]

The use of unreachable is actually safe because it is logically impossible to reach the error case. I added a comment to clarify the situation.

[coderabbitai]

@magicant Thank you for the clarification. It's important to ensure that the code's behavior is well-documented, especially in cases where certain error conditions are considered unreachable. Your comment helps future developers understand the reasoning behind the use of unreachable.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between c4c2b0a and abfd64a.

Files selected for processing (1)

• yash-builtin/src/typeset/set_functions.rs (1 hunks)

Additional comments: 2

yash-builtin/src/typeset/set_functions.rs (2)

• 21-73: The execute method is well-structured and handles errors appropriately. It iterates over the functions and their attributes, modifying the FunctionSet accordingly. The error handling is comprehensive, covering cases where a function is unset or read-only. The logic seems correct and the code is readable and maintainable.

• 76-178: The tests cover all the main scenarios and edge cases, including making existing functions read-only, unsetting read-only attributes, and handling non-existing functions. The use of assert_matches and assert_eq ensures that the function behavior is as expected. The tests are well-structured and comprehensive.