Typeset built-in for variables #315
Conversation
WalkthroughThe changes introduce a new built-in utility called "typeset" to the Changes
Poem
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (7)
- yash-builtin/src/lib.rs (3 hunks)
- yash-builtin/src/typeset.rs (1 hunks)
- yash-builtin/src/typeset/print_variables.rs (1 hunks)
- yash-builtin/src/typeset/set_functions.rs (1 hunks)
- yash-builtin/src/typeset/set_variables.rs (1 hunks)
- yash-builtin/src/typeset/syntax.rs (1 hunks)
- yash-env/src/option.rs (1 hunks)
Files not reviewed due to errors (1)
- yash-builtin/src/typeset/syntax.rs (Error: diff too large)
Files skipped from review due to trivial changes (1)
- yash-env/src/option.rs
Additional comments: 16
yash-builtin/src/typeset/set_functions.rs (1)
- 20-26: The
executemethod is currently unimplemented. Ensure that this is part of the plan and that it will be implemented in the future. If this method is called before it's implemented, it will panic at runtime.pub fn execute(self, functions: &mut FunctionSet) -> Result<String, Vec<ExecuteError>> { _ = functions; todo!() }yash-builtin/src/lib.rs (3)
57-63: The new
typesetmodule is added to the list of modules. Ensure that the module is correctly implemented and that it doesn't conflict with existing modules.71-75: The
Electivetype is imported and used in theBUILTINSarray. Ensure that theElectivetype is correctly defined and used.170-176: The
typesetcommand is added to theBUILTINSarray. Ensure that thetypeset::mainfunction is correctly implemented and that it doesn't conflict with existing commands.yash-builtin/src/typeset/set_variables.rs (4)
1-15: The license and copyright notice is correctly placed at the top of the file.
17-26: The
Fromtrait is correctly implemented for convertingScopetoyash_env::variable::Scope.29-81: The
executemethod in theSetVariablesstruct is logically sound. It correctly handles variable assignment, attribute application, and error handling. The use of a vector to collect errors allows for multiple error reporting, which is a good practice.84-376: The unit tests are comprehensive and cover a wide range of scenarios, including setting local and global variables, setting variables to read-only, exporting variables, unsetting read-only attributes, overwriting read-only variables, hiding read-only variables, and combinations of read-only attributes. The tests use assertions to verify the expected outcomes, which is a good practice.
yash-builtin/src/typeset/print_variables.rs (6)
1-15: The license and copyright notice is correctly placed at the top of the file.
17-19: The necessary modules and types are correctly imported.
21-48: The
executefunction correctly handles both cases whereself.variablesis empty and when it's not. It also correctly returns either the output string or a vector ofExecuteErrorif there are any errors.52-90: The
print_onefunction correctly formats a variable for printing based on its attributes.92-109: The
AttributeOptionstruct and itsDisplayimplementation are correctly defined.111-508: The tests cover a wide range of scenarios and correctly assert the expected results. However, the test on line 237 is ignored with a note that it needs
VariableSet::get_scoped. Ensure that this is implemented and the test is enabled before merging.yash-builtin/src/typeset.rs (2)
1-541: The documentation is comprehensive and well-written. It provides a clear understanding of the
typesetcommand's behavior, options, and error handling. The code structure is modular, with separate modules for different functionalities (print_variables,set_functions,set_variables,syntax). The use of enums (VariableAttr,Scope,FunctionAttr,Command,ExecuteError) provides a clear representation of the different states and behaviors. Error handling is also well-implemented with the use of thethiserrorcrate and custom error types. Theexecutemethod in theCommandenum is a good example of using Rust's pattern matching feature for handling different command types. Overall, the code is well-structured, maintainable, and follows Rust's best practices.541-541: The
mainfunction is well-structured and handles different outcomes of thesyntax::parseandsyntax::interpretfunctions. It also handles the result of thecommand.execute(env)call, outputting the result or reporting errors as needed. This function provides a good example of Rust's error handling and pattern matching features.
| Self::SetVariables(command) => command.execute(env), | ||
| Self::PrintVariables(command) => command.execute(&env.variables), | ||
| Self::SetFunctions(command) => command.execute(&mut env.functions), | ||
| Self::PrintFunctions(command) => todo!("{command:?}"), // command.execute(env), |
There was a problem hiding this comment.
The PrintFunctions variant in the execute method is currently not implemented. Ensure to implement this before merging the code.
- Self::PrintFunctions(command) => todo!("{command:?}"), // command.execute(env),
+ Self::PrintFunctions(command) => command.execute(env),Commitable suggestion (Beta)
| Self::PrintFunctions(command) => todo!("{command:?}"), // command.execute(env), | |
| Self::PrintFunctions(command) => command.execute(env), |
There was a problem hiding this comment.
I'm going to implement it in another pull request. Thanks!
There was a problem hiding this comment.
@magicant Understood! Feel free to reach out if you need any further assistance.
This pull request implements the
typesetbuilt-in for modifying and showing variables.Operations on functions are not handled in this pull request.
Summary by CodeRabbit
typesetcommand.Stateenum to prevent type errors.