Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(options): implement noclobber option (a.k.a. -C) #291

Merged
merged 1 commit into from
Dec 15, 2024
Merged

Conversation

reubeno
Copy link
Owner

@reubeno reubeno commented Dec 15, 2024

  • Adds support for -C on command line, via set, etc.
  • Adds basic compat tests to check.

@reubeno reubeno requested a review from Copilot December 15, 2024 06:52

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • brush-core/src/interp.rs: Evaluated as low risk
Comments suppressed due to low confidence (2)

brush-shell/src/args.rs:48

  • The comment correctly describes the option. No spelling or grammar mistakes found.
/// Enable noclobber shell option.

brush-core/src/shell.rs:127

  • [nitpick] The variable name disallow_overwriting_regular_files_via_output_redirection is too lengthy. Consider renaming it to noclobber for better readability.
pub disallow_overwriting_regular_files_via_output_redirection: bool,
Copy link

Test Results

    2 files      9 suites   1m 24s ⏱️
  556 tests   556 ✅ 0 💤 0 ❌
1 098 runs  1 098 ✅ 0 💤 0 ❌

Results for commit 591d200.

Copy link

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
expand_one_string 3.32 μs 3.48 μs 0.16 μs 🟠 +4.66%
instantiate_shell 59.40 μs 58.69 μs -0.71 μs ⚪ Unchanged
instantiate_shell_with_init_scripts 29140.74 μs 29478.29 μs 337.55 μs ⚪ Unchanged
parse_bash_completion 2700.18 μs 2676.96 μs -23.22 μs ⚪ Unchanged
parse_sample_script 4.14 μs 4.09 μs -0.05 μs ⚪ Unchanged
run_echo_builtin_command 89.34 μs 89.28 μs -0.05 μs ⚪ Unchanged
run_one_builtin_command 105.38 μs 106.69 μs 1.32 μs ⚪ Unchanged
run_one_external_command 1886.92 μs 1867.32 μs -19.60 μs ⚪ Unchanged
run_one_external_command_directly 960.30 μs 971.88 μs 11.57 μs ⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-core/src/builtins/set.rs 🟠 57.23% 🟠 60.69% 🟢 3.46%
brush-core/src/interp.rs 🟢 91.27% 🟢 92.35% 🟢 1.08%
brush-core/src/jobs.rs 🔴 37.23% 🔴 42.42% 🟢 5.19%
brush-core/src/options.rs 🟢 89.8% 🟢 90.2% 🟢 0.4%
brush-core/src/shell.rs 🟢 82.29% 🟢 82.42% 🟢 0.13%
brush-shell/src/args.rs 🟠 62.22% 🟠 60.87% 🔴 -1.35%
brush-shell/src/main.rs 🟢 90.38% 🟢 90.51% 🟢 0.13%
Overall Coverage 🟢 76.95% 🟢 77.16% 🟢 0.21%

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

@reubeno reubeno merged commit d695bdc into main Dec 15, 2024
17 checks passed
@reubeno reubeno deleted the no-clobber branch December 15, 2024 07:33
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.

1 participant