Bundler fixes

[Kriechi]

This splits the bundler-commands into seperate lines, to allow easier adding, deleting and therefore merging of new PRs.

fixes #1817
fixes #1825
fixes #2779
fixes #2200
fixes #2561
fixes #2076
fixes #1818
fixes #2885

already fixed, but issue still open:
closes #2695

not needed:
closes #2074

spring is added.
pry is added.
sidekiq is added.
ruby is removed.
berks is removed.
foreman is removed.
spin is removed.

If a binstub in ./bin/ exists, it is called instead of bundle exec <gem>.

Added functionality to include additional executables with wrapper: BUNDLED_COMMANDS.

A README.md file is added to document this plugin. Added documentation on how BUNDLED_COMMANDS and UNBUNDLED_COMMANDS works.

[lorin]

👍

[KevinBongart]

I believe it would still work without the \ (and look cleaner?):

bundled_commands=(
  annotate
  berks
  cap
  capify
  cucumber
  foodcritic
  guard
  irb
  jekyll
  kitchen
  knife
  middleman
  nanoc
  puma
  rackup
  rainbows
  rake
  rspec
  ruby
  shotgun
  sidekiq
  spec
  spork
  strainer
  tailor
  taps
  thin
  thor
  unicorn
  unicorn_rails
)

[Kriechi]

thanks @KevinBongart - you are right.
I've updated everything accordingly.

[KevinBongart]

Awesome, thanks

[mcornella]

Hi @Kriechi, thanks for taking the initiative with the bundler plugin.

When I was triaging issues I saw a lot related to the bundler plugin, so I saved them in case I made a super-PR like the one you're doing now. Since this PR intends to fix the bundler plugin, it makes sense to try to fix these issues and test these PRs. So here you go:

Delete ruby from bundled_commands

This one is mentioned several times and it seems, the most prominent being in #1587 which is related to postmodern/chruby#80. They suggest using binstubs, as is done in #2885. I don't know if that could be a possible solution.

#1587: bundler plugin should probably not re-alias ruby

• ArgumentError: Illformed requirement, on every terminal launch #2200: ArgumentError: Illformed requirement, on every terminal launch
• using bundler as plugin breaks "which ruby" syntax #2561: using bundler as plugin breaks "which ruby" syntax

Several modifications to bundled_commands

All these seem rather silly, we have already an UNBUNDLED_COMMANDS variable to unalias those commands that clash with people's workflow.
Why not add a BUNDLED_COMMANDS too -- with some predefined commands that seem general and harmless enough for everybody, like rake or rspec -- and let people use that instead of submitting yet another PR.
Or better yet, we can have a custom precmd function that aliases those commands that finds as a binstub. I don't know if that made sense 😅

• Remove spin from bundled_commands in 'bundler' plugin. #1817: Remove spin from bundled_commands in 'bundler' plugin.
• Remove foreman from the bundler plugin #1825: Remove foreman from the bundler plugin
  Add wagon command in bundler plugin #2074: Add wagon command in bundler plugin
• Berkshelf and Chef don't work well together #2076: Berkshelf and Chef don't work well together
• bundler plugin is causing mailcatcher gem stop working #2695: bundler plugin is causing mailcatcher gem stop working
• bundler plugin: Add sidekiq to bundled commands #2779: bundler plugin: Add sidekiq to bundled commands

Initialization mods

Quick background: #2243 introduced a bug that is solved by #2278; this last one makes sure it loads after rbenv (I don't know if it works).

#2243: bundle plugin throwing error when bundle is not in path while initializing
#2278: Bundle version check only needed bundler installation check, other commands to be bound

Bundler completion

• Add 'outdated' and 'platform' completions to bundler plugin #1818: Add 'outdated' and 'platform' completions to bundler plugin
  Split bundler completion and wrapping commands #1828: Split bundler completion and wrapping commands

Other

#2732: Adds bundler bump completion

---

And that's all I found. Disclaimer: I don't use bundler (nor ruby that much) to have any authority over any of this, but if I can help with the zsh part I'll gladly do it. I can't test any of it though (unless someone points me to a primer on bundler and I find the time to set it up).

[Kriechi]

I think #2278 no longer applies, because the underlying if-then-else is already refactored.
I removed ruby and berks from the bundled_commands list.
Already fixed but still open: #2695
I have merged #1818 into this PR.
I have merged #2885 into this PR and removed the double bundle-exec call.
Bundler binstubs are already with bundler.

[Kriechi]

Why not add a BUNDLED_COMMANDS too -- with some predefined commands that seem general and harmless enough for everybody, like rake or rspec -- and let people use that instead of submitting yet another PR.

Wouldn't this only work if bundler-plugin is loaded last, so that each other plugin can register a binary, and then the bundler-plugin creates all wrappers?

I'm not familiar with precmd - maybe you could show me a quick gist on how this could help us here?

[mcornella]

Wouldn't this only work if bundler-plugin is loaded last, so that each other plugin can register a binary, and then the bundler-plugin creates all wrappers?

I'm not sure I understand. It would work the same way the UNBUNDLED_COMMANDS array works now; see #2655 (comment) for reference.

---

About the precmd thing; zsh allows to execute functions before the prompt is written (precmd) or before the command is executed (preexec). In this case, we would make a custom function that looks if we're in a directory with binstubs, like so:

• If we enter a project with binstubs, alias them: alias COMMAND=bin/COMMAND; then track them by adding them in a dedicated array: aliased_binstubs+=(COMMAND)
• When we exit a project with binstubs, unalias them.

I don't think if it makes sense, nor how does it affect other ruby workflow commands like rbenv and chruby and such. I see that this approach is too complicated to put it in an easy-merge PR, so I could make a separate PR and discuss it there. What do you think?

[Kriechi]

UNBUNDLED_COMMANDS - yeah - now I get it. Yes, I will add a similiar thing for additional gems with BUNDLED_COMMANDS.

I will add a README.md to document those two configuration options for the users.

I think using a preexec hook (or something like that) is very invasive.
Wouldn't it be easier to just add ./bin/ to PATH? but this implies a lot of security issues.
So I would leave this out of this PR. Maybe later on we can work something out.

[Kriechi]

@mcornella @lorin @KevinBongart I have added additional things (README.md, BUNDLED_COMMANDS functionality)
Could you please take a look?

@mcornella I think I have covered all easy issues you mentioned in your first comment. Please let me know if there is anything major left.

[KevinBongart]

@Kriechi Nice work! Thanks for taking care of that :)

I've added three tiny comments since I couldn't update the README file directly

[Kriechi]

@KevinBongart thanks. I've fixed it.

[mcornella]

Awesome job @Kriechi, I added a little remark to the README.

I have also updated my comment to check those issues you tackled (:

Only remain:

• Add wagon command in bundler plugin #2074, which I commented but don't know if it should be closed or it makes sense to pull (add wagon to BUNDLED_COMMANDS)

Already dealt with:

• Split bundler completion and wrapping commands #1828 and Adds bundler bump completion #2732, which are more structural to the bundler plugin and should be discussed further (Split bundler completion and wrapping commands #1828 may not be necessary now that the bundler plugin is better). (Split bundler completion and wrapping commands #1828 is unnecessary and solvable with a custom plugin; Adds bundler bump completion #2732 doesn't seem to be integrated with bundler any time soon).
• Bundle version check only needed bundler installation check, other commands to be bound #2278, which hopefully the author will respond and confirm that is obsolete. He did :)

Thank you very much! 😄

[sabarish-soft]

@mcornella i have closed the PR as it is not required, Thanks for letting me know :)

[Kriechi]

I would not pull #2074 - IMHO is this gem fine without bundle exec.
Also, it seems to be a bit unmaintained or not popular enough to deserve the tag common gem.

So now everything mentioned should either be merged, fixed or otherwise dealt with.
I'm happy to pull in other (new) Bundler-related PRs (mention it and ping me).

IMHO this PR is ready to merge.

[mcornella]

Yeah that's what I thought too, thanks for confirming it. Maybe we can close it? The author doesn't seem to respond anymore.

Anyway, this PR is ready to merge too in my opinion 👍

[KevinBongart]

👍

[robbyrussell]

@Kriechi Can you rebase once more for me?

[Kriechi]

done.

[robbyrussell]

Thanks!

[fabn]

This is linked into readme, but I don't get why rails has been removed from bundled commands, what issue are caused to have it in bundled_commands?

[mcornella]

This is linked into readme, but I don't get why rails has been removed from bundled commands, what issue are caused to have it in bundled_commands?

From https://stackoverflow.com/a/23846736:

The one notorious exception to this rule is the rails command. The reason being that the first thing the rails command does is load up bundler and check which version of the command to execute. So you'd really just be slowing yourself down to involve bundler in the first place when running the rails command.