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

BASH_IT_AUTOMATIC_RELOAD_AFTER_CONFIG_CHANGE causes undesired behaviour for reloading Bash-it #1665

Closed
ahmadassaf opened this issue Sep 25, 2020 · 9 comments · Fixed by #1671

Comments

@ahmadassaf
Copy link
Contributor

Having the BASH_IT_AUTOMATIC_RELOAD_AFTER_CONFIG_CHANGE flag should automatically reload Bash-it after activating or deactivating plugins, aliases, or completions. However, the behaviour that I am experiencing is completely different as it seems that the shell is being "reset/exit" and not reloaded.

I have found that the code in

exec ${0/-/}
and
exec ${0/-/}
if replaced with a simple call to the _bash-it-reload works as expected.

I am just wondering about what is the expected behaviour from the exec ${0/-/}, but happy to PR if this seems to be a valid issue.

@ahmadassaf
Copy link
Contributor Author

Found a reference to the addition of this line #764 (comment) but I don't think this is working properly

@nwinkler
Copy link
Member

You are right, this does not seem to work as expected...

  master ↓1  ~/Vagrant/vagrant-bash-it             12:48:15  ⚡ 100%  foo.bar 
❯ bash-it enable plugin tmux
bash-5.0$ 

This is on macOS with Bash v5.0.18 - it clearly does not what's expected.

The issue seems to be that Bash is not invoked as a login shell, which means that the user's profile is not loaded.

It looks like the -l options needs to be added to run Bash as a login shell.

Which OS are you seeing this on? Can you see if adding -l at the end of the two lines you referenced fixes this for you?

@ahmadassaf
Copy link
Contributor Author

macOS and yes the -l did indeed fix it 🙏

@nwinkler
Copy link
Member

nwinkler commented Oct 2, 2020

OK - it would be interesting to see whether a Linux Bash works in the same way. I think I have a VM somewhere...

@nwinkler
Copy link
Member

nwinkler commented Oct 2, 2020

Just gave this a quick test in a Vagrant box running Ubuntu: It looks like both bash and bash -l do the same thing in terms of Bash-it initialization:

image

It looks like adding the -l parameter to the code will work for both Linux and macOS. In the worst case, we could wrap it in an if statement...

Would you be willing to open a PR for this?

@ahmadassaf
Copy link
Contributor Author

ahmadassaf commented Oct 2, 2020

ok I discovered something that makes this line has more side effects.

    if [ -n "$BASH_IT_AUTOMATIC_RELOAD_AFTER_CONFIG_CHANGE" ]; then
        exec ${0/-/}
    fi

    printf '%s\n' "$file_entity enabled with priority $use_load_priority."

Now if we want to add the -l to run Bash as a login shell means that any subsequent operation will not be run! for the above it is easy to move the print before the load. However, things become interesting when trying to enable all and have the flagBASH_IT_AUTOMATIC_RELOAD_AFTER_CONFIG_CHANGE set to true. This means that only the first item of the component (alias, completion, or plugin) will be enabled, the shell will reload and that's it.

Again, reverting this back to bash-it reload/_bash-it-reload makes it work like a charm.

@cornfeedhobo
Copy link
Member

This thread reminded me that #1533 exists. Could this be related?

@nwinkler
Copy link
Member

nwinkler commented Oct 5, 2020

@ahmadassaf The call to _bash-it-reload works for some use cases, for example when enabling a new plugin. It does note undefine any functions when you disable a plugin and it's not removing alias definitions from the current shell when you're disabling an alias component.

I understand the issue with the looped reload, that's definitely something. Can we move the exec call to outside of the loop in this case, so that we only call exec once after everything has been enabled/disabled?

@NoahGorny
Copy link
Member

Honestly I prefer _bash-it-reload in these cases, as restarting the shell like this is unpleasent and probably not what the user wanted (maybe add a specific command to reset the shell and cd back to the directory?)
In any case, we actually do not need to reload after every enable/disable, so I filed a PR to change the behavior to reload and only do it after enabling/disabling all components (see #1671)

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 a pull request may close this issue.

4 participants