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

Use bash from $SHELL when it makes sense #408

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roberth
Copy link
Contributor

@roberth roberth commented Oct 14, 2020

This fixes file completion in cases where a virtual environment with a non-interactive bash is loaded with direnv, nix-shell or similar.

It solves the error:

$ mycommand --file <TAB>bash: compgen: command not found
bash: compgen: command not found
bash: compgen: command not found

@HuwCampbell
Copy link
Collaborator

Hi thanks for contributing to optparse!

I'll be honest, I'm very conflicted on this, though happy to discuss further.

What you've shown in your terminal printout is awful! We just regurgitate stderr of the base process into the user's shell when really, they don't want that. What I'm thinking is changing the bashCompleter function to something like this:

bashCompleter :: String -> Completer
bashCompleter action = Completer $ \word -> do
  let cmd = unwords ["compgen", "-A", action, "--", requote word]
  (ec, _, result) <-
    readProcessWithExitCode "bash" ["-c", cmd] ""

  return $
    if ec == ExitSuccess then
      lines result
    else
      []

Here we get rid of a exception handler and won't spew error messages to their terminal when they hit tab. Your solution won't fix that in the general case, but could reduce it for some.

I don't know, is that acceptable?

@roberth
Copy link
Contributor Author

roberth commented Nov 6, 2020

A change like that will make it practically impossible to troubleshoot, unless someone is dead set on solving a minor inconvenience.

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.

2 participants