Skip to content

Update fasd to use installed version instead of embedded #691

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 25, 2016
Merged

Update fasd to use installed version instead of embedded #691

merged 3 commits into from
Mar 25, 2016

Conversation

tswicegood
Copy link
Member

This removes the embedded version of fasd in favor of installed versions. I've included an error message when enabled and unable to find the fasd executable.

@tswicegood
Update to latest version of fasd.
c600d82 
This brings the fasd up to November 5, 2015.

Commit imported: clvv/fasd@d41c4b0
@tswicegood
Copy link
Member Author

@nwinkler would appreciate a second set of eyes on this before I merge. I was trying to fix fasd swallowing $? when I did the upgrade. I have a fix that I might still apply, but this seems to fix it now though I'm not 100% sure how. Love it when that happens. 😎

Couple of things to check:

  • Verify that echo $? outputs 0 if you run true and 1 if you run `false.
  • Test the output of echo $PROMPT_COMMAND. I'm seeing __powerline_prompt_command running the powerline-multiline theme. Before this commit it included an eval tacked on there by fasd. That's no longer there, but all of fasd seems to be working.

@tswicegood
Copy link
Member Author

FWIW, there was something up with my shell that caused this to succeed falsely. I'm in the process of removing this plugin entirely in favor of installing it and using a custom fasd --init to initialize it in bash.

@tswicegood
Replace embedded fasd with initialization
06b458c 
There are multiple ways to install fasd, but the initialization
still needs to happen. Currently, fasd is broken in bash in that
it doesn't maintain the exit code properly. This custom init removes
the call to `fasd --init bash-hook` and replaces it with the version
that would be generated once fasd PR Bash-it#72 is merged.

See: clvv/fasd#72
@tswicegood tswicegood changed the title Update to latest version of fasd. Update fasd to use installed version instead of embedded Mar 25, 2016
@tswicegood
Copy link
Member Author

Note, this PR includes a work around for clvv/fasd#72. Once that is merged we can simply replace the entire else block with eval "$(fasd --init posix-alias bash-hook bash-ccomp bash-ccomp-install)".

@tswicegood
Copy link
Member Author

Ok, with the latest commits I'm back to understanding what's going on. Some ghost shell variables or something was causing it to initialize properly earlier. Going to go ahead and merge, but would still be curious for any feedback y'all have.

@tswicegood tswicegood merged commit ae179df into Bash-it:master Mar 25, 2016
@tswicegood tswicegood deleted the upgrade-fasd branch March 25, 2016 22:28
@edubxb
Copy link
Member

edubxb commented Mar 26, 2016

Thanks @tswicegood for the fix, I stopped using fasd because this.

edubxb added a commit to edubxb/bash-it that referenced this pull request Mar 26, 2016
@edubxb
Make Bash-it#691 work on Linux
183c514
edubxb added a commit that referenced this pull request Mar 27, 2016
@edubxb
Merge pull request #694 from edubxb/fix-fasd-linux
3447cba 
Make #691 work on Linux
@nwinkler
Copy link
Member

Good stuff - I had stopped using fasd because of this issue as well. Time to enable it again! Thanks for working on this!

@knorth55 knorth55 mentioned this pull request Nov 4, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tswicegood @edubxb @nwinkler