Skip to content

Conversation

NoahGorny
Copy link
Member

Correctly restore old pwd in all cases of bash-it update

Description

When we update, we usually call bash-it restart, which re-execs bash. It means that the code of restoring
old pwd at the end is not being executed. I fixed this and another problem by correctly cding into to old pwd in time.

Motivation and Context

Closes #1920

How Has This Been Tested?

Locally, sadly it is very hard to automate testing of bash-it update, as it uses actual git repo and tries to pull information.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • If my change requires a change to the documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
  • I have added tests to cover my changes, and all the new and existing tests pass.

Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

I would document that _bash-it_pull_and_update_inner doesn't return when a restart happens.

Feels bit more awkward than using pushd/popd but since we're documenting the parameter and the function is only meant to be called from _bash-it_update-, this should work.

/nitipick : _bash-it_pull_and_update_inner might could be named better - It doesn't give much hint that a restart is the end action of the function and control won't return to the caller.

Renamed the function to _bash-it_update_migrate_and_restart
Use pushd/popd instead of passing another parameter
Document the function so it will be clear that it does not return
@NoahGorny NoahGorny force-pushed the fix-update-not-popping-back-cwd branch from 10df8f8 to 9566a3e Compare August 17, 2021 21:30
@NoahGorny
Copy link
Member Author

Hi @davidpfarrell
I figured that while we are at it, we not improve the code just like you suggested 😄
You are welcome to take another look!

@NoahGorny NoahGorny merged commit 226454b into Bash-it:master Aug 18, 2021
@NoahGorny NoahGorny deleted the fix-update-not-popping-back-cwd branch August 18, 2021 18:17
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.

Post bashit update command to switch actual current directory not working
2 participants