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

build: Install zsh completion #537

Closed
wants to merge 2 commits into from

Conversation

danielshahaf
Copy link
Contributor

Fixes #534

OPT_ZSH_FPATH=default
AC_ARG_WITH(zsh-functions-dir,
AC_HELP_STRING([--with-zsh-functions-dir=PATH],[Install zsh completions to PATH])
AC_HELP_STRING([--without-nghttp2],[Do not install zsh completions]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you copy this from nghttp2? this line needs to be changed, also there is another line farther below. did you test this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the --with-with-argument and --with-without-argument codepaths but not the --without- codepath. Fixed.

@bagder
Copy link
Member

bagder commented Nov 24, 2015

Is zsh used commonly enough for this to be done by default?

@danielshahaf
Copy link
Contributor Author

I don't know where to find hard data on zsh's popularity. (There's the debian popcon data and systemd's default behaviour with respect to their zsh completion functions, but I don't know whether either of these datapoints is representative of anything.)

I suppose that, if --with-zsh-functions-dir isn't passed, you have three options:

  • Unconditionally install the completion.
  • Unconditionally do not install the completion.
  • Check whether /usr/local/share/zsh exists and only install the completion if it does.
    This may lead to unpredictable behaviour if zsh is installed sometimes before curl and sometimes after it.

@bagder bagder closed this in be0d414 Nov 24, 2015
@bagder
Copy link
Member

bagder commented Nov 24, 2015

Thanks! I merged your version now so it will install it unconditionally. Let's see if we get any protests...

@danielshahaf
Copy link
Contributor Author

Thanks @bagder!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants