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

fix: set the exit code to the return of cd for the warp function #143

Merged
merged 2 commits into from
Nov 17, 2024

Conversation

zimeg
Copy link
Contributor

@zimeg zimeg commented Nov 3, 2024

Summary

👋 🤠 This PR updates the exit code with the returned value from cd in the wd_warp function to fix #142 and break chained commands if nested directories don't exist.

Preview

With this change the exit code is set to 1 when a missing nested warp point is provided:

$ wd home unknown
wd_warp:cd:19: no such file or directory: /home/user/unknown
$ echo $?
1

Chained commands also break once the missing nested warp point is reached:

$ wd home unknown && ./setup.sh
wd_warp:cd:19: no such file or directory: /home/user/unknown

Reviewers

To test the changes with this branch checked out, the following commands might be useful ✨

  1. Attempt to warp to unknown nested warp points:
$ cd
$ wd add home
$ wd home unknown
wd_warp:cd:19: no such file or directory: /home/user/unknown
$ echo $?
1
$ pwd
/home/user

Notes

This change includes extra commits from #140 for some of the changes to testing. Please let me know if a rebase or other reverts are needed, or another change!

Copy link
Contributor Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

📝 Leaving a few comments on changes from commits in another PR, but needed for the test_config test to pass with the changed source in tests!

test/tests.sh Outdated Show resolved Hide resolved
wd.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@alpha-tango-kilo alpha-tango-kilo left a comment

Choose a reason for hiding this comment

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

This looks good. If you separate it from #140 I'll merge it now, or alternatively feel free to go through my feedback on #140 and then rebase this PR, whichever is easiest for you

Copy link
Contributor Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

@alpha-tango-kilo I just rebased it with the changes in #141 since these share a few testing changes needed for sourcing with WD_CONFIG 😅

Thanks once more for the review, and please let me know if more updates are needed! 🙏 ✨

wd.sh Show resolved Hide resolved
test/tests.sh Outdated
Comment on lines 536 to 542
# confirm $WD_CONFIG was unchanged
assertEquals "$wd_config_lines" "$(total_wps)"
assertEquals "$wd_config_lines" 0

# confirm --config was changed
assertEquals 1 "$(wcl < "$arg_config")"
assertEquals 1 "$(WD_CONFIG=$arg_config total_wps)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No changes to the asserts in this PR or the linked, but it's added for clarity as part of #141 🔬

Copy link
Collaborator

@alpha-tango-kilo alpha-tango-kilo left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@alpha-tango-kilo alpha-tango-kilo merged commit e3e65a5 into mfaerevaag:master Nov 17, 2024
1 check passed
@zimeg zimeg deleted the fix-cd-exit-code branch November 17, 2024 19:25
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.

Bug report: The exit code remains 0 if a nested directory does not exist
2 participants