Skip to content

feat: implement {cd,pwd} -{L,P}#458

Merged
reubeno merged 1 commit into
mainfrom
cd-pwd-LP
May 4, 2025
Merged

feat: implement {cd,pwd} -{L,P}#458
reubeno merged 1 commit into
mainfrom
cd-pwd-LP

Conversation

@reubeno
Copy link
Copy Markdown
Owner

@reubeno reubeno commented May 4, 2025

This integrates the compat tests, clap annotation updates, and some of the core builtin updates from #219.

There are 2 notable differences with this PR's approach:

  • In place of a custom implementation of path normalization, this PR instead opts for adopting the normalize-path crate with the acknowledged trade-off that this latter crate likely does not necessarily handle Windows paths.

  • This version does not persistently maintain a logical and physical representation of the current working dir. Rather, it maintains the existing single representation, and only computes physical paths on-demand when required. We still may very well need to go with maintaining both, but for now this keeps the change more minimal and seems to be more in line with handling the edge case of how, for example, pwd -P should behave on a directory that has been renamed/moved/altered since being cd'd into.

We will leave the older PR open for now, and re-evaluate how best to preserve/use its Windows-specific logic in subsequent changes.

(Note: @39555 -- the commit tags you as a contributor because you had done the heavy lifting of identifying the work needed and also because this PR directly incorporates some of your changes from the above-linked PR.)

Resolves: #202.

Co-authored-by: Igor <ig_@tutamail.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2025

Test Results

    3 files     14 suites   3m 44s ⏱️
  723 tests   723 ✅ 0 💤 0 ❌
2 155 runs  2 155 ✅ 0 💤 0 ❌

Results for commit d6ffb8e.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2025

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
clone_shell_object 18.10 μs 18.81 μs 0.71 μs 🟠 +3.94%
eval_arithmetic 0.17 μs 0.17 μs -0.00 μs ⚪ Unchanged
expand_one_string 1.87 μs 1.83 μs -0.05 μs ⚪ Unchanged
for_loop 22.97 μs 22.04 μs -0.93 μs 🟢 -4.04%
function_call 2.47 μs 2.40 μs -0.08 μs ⚪ Unchanged
instantiate_shell 57.10 μs 56.84 μs -0.25 μs ⚪ Unchanged
instantiate_shell_with_init_scripts 24232.32 μs 23975.26 μs -257.06 μs ⚪ Unchanged
parse_bash_completion 1665.57 μs 1667.42 μs 1.85 μs ⚪ Unchanged
parse_sample_script 1.76 μs 1.73 μs -0.02 μs ⚪ Unchanged
run_echo_builtin_command 15.32 μs 15.19 μs -0.12 μs ⚪ Unchanged
run_one_external_command 2338.50 μs 2397.61 μs 59.11 μs ⚪ Unchanged
tokenize_sample_script 2.89 μs 2.84 μs -0.05 μs 🟢 -1.76%

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-core/src/builtins/cd.rs 🟢 81.82% 🟢 82.35% 🟢 0.53%
brush-core/src/builtins/pwd.rs 🟢 80.95% 🟢 88.89% 🟢 7.94%
brush-core/src/shell.rs 🟢 82.99% 🟢 83.01% 🟢 0.02%
Overall Coverage 🟢 76.39% 🟢 76.41% 🟢 0.02%

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

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1289 61.12
❗️ Error 226 10.72
❌ Fail 200 9.48
⏩ Skip 379 17.97
❎ Expected Fail 14 0.66
✔️ Unexpected Pass 1 0.05
📊 Total 2109 100.00

@reubeno reubeno merged commit afa18b7 into main May 4, 2025
24 checks passed
@reubeno reubeno deleted the cd-pwd-LP branch May 4, 2025 08:07
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.

correct implementation of the physical -P and logical -L directories in cd, pwd and dirs

1 participant