Skip to content

refactor: Shell struct API improvements#692

Merged
reubeno merged 10 commits into
mainfrom
ref
Oct 3, 2025
Merged

refactor: Shell struct API improvements#692
reubeno merged 10 commits into
mainfrom
ref

Conversation

@reubeno
Copy link
Copy Markdown
Owner

@reubeno reubeno commented Oct 2, 2025

This PR takes a pass over the Shell type:

  • Makes more fields private, adding accessors when needed.
  • Drops get_ prefix from most getter names.
  • Consolidates more file-open operations through Shell::open_file() for future extension.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 2, 2025

Test Results

    3 files     23 suites   4m 50s ⏱️
  876 tests   876 ✅ 0 💤 0 ❌
2 608 runs  2 608 ✅ 0 💤 0 ❌

Results for commit 4aef654.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 2, 2025

Public API changes for crate: brush-core

Removed items

-pub struct brush_core::functions::FunctionRegistration
-impl brush_core::functions::FunctionRegistration
-pub fn brush_core::functions::FunctionRegistration::definition(&self) -> &brush_parser::ast::FunctionDefinition
-pub const fn brush_core::functions::FunctionRegistration::export(&mut self)
-pub const fn brush_core::functions::FunctionRegistration::is_exported(&self) -> bool
-pub const fn brush_core::functions::FunctionRegistration::unexport(&mut self)
-impl core::convert::From<brush_parser::ast::FunctionDefinition> for brush_core::functions::FunctionRegistration
-pub fn brush_core::functions::FunctionRegistration::from(definition: brush_parser::ast::FunctionDefinition) -> Self
-pub fn brush_core::jobs::Job::get_annotation(&self) -> brush_core::jobs::JobAnnotation
-pub fn brush_core::jobs::Job::get_command_name(&self) -> &str
-pub fn brush_core::jobs::Job::get_process_group_id(&self) -> core::option::Option<i32>
-pub fn brush_core::jobs::Job::get_representative_pid(&self) -> core::option::Option<i32>
-pub fn brush_core::options::RuntimeOptions::get_option_flags(&self) -> alloc::string::String
-pub fn brush_core::options::RuntimeOptions::get_set_o_optstr(&self) -> alloc::string::String
-pub fn brush_core::options::RuntimeOptions::get_shopt_optstr(&self) -> alloc::string::String
-pub fn brush_core::variables::ShellValue::get_element_keys(&self, shell: &brush_core::Shell) -> alloc::vec::Vec<alloc::string::String>
-pub fn brush_core::variables::ShellValue::get_element_keys(&self, shell: &brush_core::Shell) -> alloc::vec::Vec<alloc::string::String>
-pub fn brush_core::variables::ShellValue::get_element_values(&self, shell: &brush_core::Shell) -> alloc::vec::Vec<alloc::string::String>
-pub fn brush_core::variables::ShellValue::get_element_values(&self, shell: &brush_core::Shell) -> alloc::vec::Vec<alloc::string::String>
-pub fn brush_core::variables::ShellVariable::get_attribute_flags(&self, shell: &brush_core::Shell) -> alloc::string::String
-pub fn brush_core::variables::ShellVariable::get_attribute_flags(&self, shell: &brush_core::Shell) -> alloc::string::String
-pub brush_core::Shell::builtins: std::collections::hash::map::HashMap<alloc::string::String, brush_core::builtins::Registration>
-pub brush_core::Shell::funcs: brush_core::functions::FunctionEnv
-pub brush_core::Shell::key_bindings: core::option::Option<alloc::sync::Arc<tokio::sync::mutex::Mutex<dyn brush_core::interfaces::KeyBindings>>>
-pub brush_core::Shell::last_exit_status: u8
-pub brush_core::Shell::shell_product_display_str: core::option::Option<alloc::string::String>
-pub brush_core::Shell::shell_version: core::option::Option<alloc::string::String>
-pub brush_core::Shell::working_dir: std::path::PathBuf
-pub fn brush_core::Shell::get_absolute_path(&self, path: impl core::convert::AsRef<std::path::Path>) -> std::path::PathBuf
-pub async fn brush_core::Shell::get_completions(&mut self, input: &str, position: usize) -> core::result::Result<brush_core::completion::Completions, brush_core::error::Error>
-pub fn brush_core::Shell::get_env_str(&self, name: &str) -> core::option::Option<alloc::borrow::Cow<'_, str>>
-pub fn brush_core::Shell::get_env_var(&self, name: &str) -> core::option::Option<&brush_core::variables::ShellVariable>
-pub fn brush_core::Shell::get_history_file_path(&self) -> core::option::Option<std::path::PathBuf>
-pub fn brush_core::Shell::get_history_time_format(&self) -> core::option::Option<alloc::string::String>
-pub fn brush_core::Shell::get_ifs(&self) -> alloc::borrow::Cow<'_, str>

Added items

+pub struct brush_core::functions::Registration
+impl brush_core::functions::Registration
+pub fn brush_core::functions::Registration::definition(&self) -> &brush_parser::ast::FunctionDefinition
+pub const fn brush_core::functions::Registration::export(&mut self)
+pub const fn brush_core::functions::Registration::is_exported(&self) -> bool
+pub const fn brush_core::functions::Registration::unexport(&mut self)
+impl core::convert::From<brush_parser::ast::FunctionDefinition> for brush_core::functions::Registration
+pub fn brush_core::functions::Registration::from(definition: brush_parser::ast::FunctionDefinition) -> Self
+pub fn brush_core::jobs::Job::annotation(&self) -> brush_core::jobs::JobAnnotation
+pub fn brush_core::jobs::Job::command_name(&self) -> &str
+pub fn brush_core::jobs::Job::process_group_id(&self) -> core::option::Option<i32>
+pub fn brush_core::jobs::Job::representative_pid(&self) -> core::option::Option<i32>
+pub fn brush_core::options::RuntimeOptions::option_flags(&self) -> alloc::string::String
+pub fn brush_core::options::RuntimeOptions::seto_optstr(&self) -> alloc::string::String
+pub fn brush_core::options::RuntimeOptions::shopt_optstr(&self) -> alloc::string::String
+pub fn brush_core::variables::ShellValue::element_keys(&self, shell: &brush_core::Shell) -> alloc::vec::Vec<alloc::string::String>
+pub fn brush_core::variables::ShellValue::element_keys(&self, shell: &brush_core::Shell) -> alloc::vec::Vec<alloc::string::String>
+pub fn brush_core::variables::ShellValue::element_values(&self, shell: &brush_core::Shell) -> alloc::vec::Vec<alloc::string::String>
+pub fn brush_core::variables::ShellValue::element_values(&self, shell: &brush_core::Shell) -> alloc::vec::Vec<alloc::string::String>
+pub fn brush_core::variables::ShellVariable::attribute_flags(&self, shell: &brush_core::Shell) -> alloc::string::String
+pub fn brush_core::variables::ShellVariable::attribute_flags(&self, shell: &brush_core::Shell) -> alloc::string::String
+pub enum brush_core::ScriptCallType
+pub brush_core::ScriptCallType::Executed
+pub brush_core::ScriptCallType::Sourced
+pub struct brush_core::FunctionCall
+pub brush_core::FunctionCall::function_definition: alloc::sync::Arc<brush_parser::ast::FunctionDefinition>
+pub brush_core::FunctionCall::function_name: alloc::string::String
+pub fn brush_core::Shell::absolute_path(&self, path: impl core::convert::AsRef<std::path::Path>) -> std::path::PathBuf
+pub fn brush_core::Shell::builtin_mut(&mut self, name: &str) -> core::option::Option<&mut brush_core::builtins::Registration>
+pub const fn brush_core::Shell::builtins(&self) -> &std::collections::hash::map::HashMap<alloc::string::String, brush_core::builtins::Registration>
+pub async fn brush_core::Shell::complete(&mut self, input: &str, position: usize) -> core::result::Result<brush_core::completion::Completions, brush_core::error::Error>
+pub const fn brush_core::Shell::current_line_number(&self) -> u32
+pub fn brush_core::Shell::define_func(&mut self, name: impl core::convert::Into<alloc::string::String>, definition: brush_parser::ast::FunctionDefinition)
+pub fn brush_core::Shell::define_func_from_str(&mut self, name: impl core::convert::Into<alloc::string::String>, body_text: &str) -> core::result::Result<(), brush_core::error::Error>
+pub const fn brush_core::Shell::depth(&self) -> usize
+pub fn brush_core::Shell::env_str(&self, name: &str) -> core::option::Option<alloc::borrow::Cow<'_, str>>
+pub fn brush_core::Shell::env_var(&self, name: &str) -> core::option::Option<&brush_core::variables::ShellVariable>
+pub fn brush_core::Shell::func_mut(&mut self, name: &str) -> core::option::Option<&mut brush_core::functions::Registration>
+pub const fn brush_core::Shell::funcs(&self) -> &brush_core::functions::FunctionEnv
+pub const fn brush_core::Shell::function_call_stack(&self) -> &alloc::collections::vec_deque::VecDeque<brush_core::FunctionCall>
+pub fn brush_core::Shell::history_file_path(&self) -> core::option::Option<std::path::PathBuf>
+pub fn brush_core::Shell::history_time_format(&self) -> core::option::Option<alloc::string::String>
+pub fn brush_core::Shell::ifs(&self) -> alloc::borrow::Cow<'_, str>
+pub const fn brush_core::Shell::key_bindings(&self) -> &core::option::Option<alloc::sync::Arc<tokio::sync::mutex::Mutex<dyn brush_core::interfaces::KeyBindings>>>
+pub const fn brush_core::Shell::last_exit_status_mut(&mut self) -> &mut u8
+pub const fn brush_core::Shell::last_stopwatch_offset(&self) -> u32
+pub const fn brush_core::Shell::last_stopwatch_time(&self) -> std::time::SystemTime
+pub const fn brush_core::Shell::product_display_str(&self) -> &core::option::Option<alloc::string::String>
+pub const fn brush_core::Shell::script_call_stack(&self) -> &alloc::collections::vec_deque::VecDeque<(brush_core::ScriptCallType, alloc::string::String)>
+pub fn brush_core::Shell::undefine_func(&mut self, name: &str) -> bool
+pub const fn brush_core::Shell::version(&self) -> &core::option::Option<alloc::string::String>
+pub fn brush_core::Shell::working_dir(&self) -> &std::path::Path

Changed items

-pub brush_core::error::Error::RedirectionFailure(alloc::string::String, std::io::error::Error)
+pub brush_core::error::Error::RedirectionFailure(alloc::string::String, alloc::string::String)
-pub fn brush_core::functions::FunctionEnv::get(&self, name: &str) -> core::option::Option<&brush_core::functions::FunctionRegistration>
+pub fn brush_core::functions::FunctionEnv::get(&self, name: &str) -> core::option::Option<&brush_core::functions::Registration>
-pub fn brush_core::functions::FunctionEnv::get_mut(&mut self, name: &str) -> core::option::Option<&mut brush_core::functions::FunctionRegistration>
+pub fn brush_core::functions::FunctionEnv::get_mut(&mut self, name: &str) -> core::option::Option<&mut brush_core::functions::Registration>
-pub fn brush_core::functions::FunctionEnv::iter(&self) -> impl core::iter::traits::iterator::Iterator<Item = (&alloc::string::String, &brush_core::functions::FunctionRegistration)>
+pub fn brush_core::functions::FunctionEnv::iter(&self) -> impl core::iter::traits::iterator::Iterator<Item = (&alloc::string::String, &brush_core::functions::Registration)>
-pub fn brush_core::functions::FunctionEnv::remove(&mut self, name: &str) -> core::option::Option<brush_core::functions::FunctionRegistration>
+pub fn brush_core::functions::FunctionEnv::remove(&mut self, name: &str) -> core::option::Option<brush_core::functions::Registration>
-pub fn brush_core::functions::FunctionEnv::update(&mut self, name: alloc::string::String, registration: brush_core::functions::FunctionRegistration)
+pub fn brush_core::functions::FunctionEnv::update(&mut self, name: alloc::string::String, registration: brush_core::functions::Registration)
-pub fn brush_core::history::History::import(path: impl core::convert::AsRef<std::path::Path>) -> core::result::Result<Self, brush_core::error::Error>
+pub fn brush_core::history::History::import(reader: impl std::io::Read) -> core::result::Result<Self, brush_core::error::Error>
-pub brush_core::Error::RedirectionFailure(alloc::string::String, std::io::error::Error)
+pub brush_core::Error::RedirectionFailure(alloc::string::String, alloc::string::String)

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
clone_shell_object 17.28 μs 17.39 μs 0.11 μs 🟠 +0.64%
eval_arithmetic 0.15 μs 0.15 μs 0.00 μs ⚪ Unchanged
expand_one_string 2.06 μs 2.14 μs 0.08 μs ⚪ Unchanged
for_loop 22.43 μs 23.04 μs 0.61 μs 🟠 +2.71%
function_call 2.73 μs 2.85 μs 0.12 μs ⚪ Unchanged
instantiate_shell 57.76 μs 57.94 μs 0.18 μs ⚪ Unchanged
instantiate_shell_with_init_scripts 24632.48 μs 23885.09 μs -747.39 μs 🟢 -3.03%
parse_bash_completion 1699.35 μs 1689.76 μs -9.60 μs ⚪ Unchanged
parse_sample_script 1.77 μs 1.73 μs -0.03 μs 🟢 -1.98%
run_echo_builtin_command 15.96 μs 16.09 μs 0.12 μs ⚪ Unchanged
run_one_external_command 2439.10 μs 2096.13 μs -342.97 μs 🟢 -14.06%
tokenize_sample_script 2.76 μs 2.77 μs 0.01 μs ⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-builtins/src/dirs.rs 🟢 96.67% 🟢 97.22% 🟢 0.55%
brush-core/src/arithmetic.rs 🟢 98.39% 🟢 98.38% 🔴 -0.01%
brush-core/src/history.rs 🟠 68.25% 🟠 68.1% 🔴 -0.15%
brush-core/src/interp.rs 🟢 91.61% 🟢 91.62% 🟢 0.01%
brush-core/src/jobs.rs 🔴 37.16% 🔴 43.58% 🟢 6.42%
brush-core/src/shell.rs 🟢 78.34% 🟢 76.8% 🔴 -1.54%
brush-core/src/wellknownvars.rs 🔴 0% 🟢 87.74% 🟢 87.74%
Overall Coverage 🟢 72.31% 🟢 72.53% 🟢 0.22%

Minimum allowed coverage is 70%, this run produced 72.53%

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1308 62.02
❗️ Error 237 11.24
❌ Fail 179 8.49
⏩ Skip 370 17.54
❎ Expected Fail 15 0.71
📊 Total 2109 100.00

Comment thread .github/workflows/ci.yaml Fixed
@reubeno reubeno marked this pull request as ready for review October 3, 2025 02:17
@reubeno reubeno changed the title refactor: ergonomics improvements + styling renames refactor: Shell struct API improvements Oct 3, 2025
@reubeno reubeno merged commit 57a5803 into main Oct 3, 2025
42 checks passed
@reubeno reubeno deleted the ref branch October 3, 2025 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants