fix(bash): do not use "return" to cancel initialization#1928
Merged
Conversation
We have introduced initialization guards in atuinsh#1533 [1], where `return 0` was used to cancel the initialization. However, this cancels the processing of the caller (which is typically `~/.bashrc`) instead of just canceling Atuin's initialization. In this patch, we avoid using `return 0`. Instead, we enclose the main part of the initialization in a big if-statement. [1] atuinsh#1533
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #1533, I used
return 0for the "include guards" of Atuin's initialization. However, I noticed that it would unexpectedly cancel the entire processing of~/.bashrcbecause it is evaluated byeval. This is my fault. In this PR, I suggest enclosing the entire content of the integration code within anif-statement.Estimation on the impact
I expect the impact of
return 0would not be so big since the problem only happens when (i)~/.bashrcis sourced twice in an interactive session, (ii)eval "$(atuin init bash)"is performed outside the interactive sessions, or (iii) it is performed in a very old version of Bash for which Atuin's integration doesn't offer the support. For case (ii), with a typical.bashrcstarting withcase $- in *i*) ;; *) return ;; esac(or equivalent),eval "$(atuin init bash)"is usually performed only in interactive sessions. For case (iii), Atuin's initialization anyway produces an error message, so the setup is technically wrong. If the user uses mixed versions of Bash, the user is supposed to check the Bash version at the caller side in~/.bashrcbefore evaluatingeval "$(atuin init bash)".Checks