Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] Fix zsh installation instructions
@ 2020-07-02 10:51 Alexey via GitGitGadget
  2020-07-06 22:42 ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Alexey via GitGitGadget @ 2020-07-02 10:51 UTC (permalink / raw)
  To: git; +Cc: Alexey, Alexey

From: Alexey <lesha.ogonkov@gmail.com>

- Fix wrong script in completion configuration. zsh wants bash completion
  path here, not path to itself.
- Add `compinit` autoload command, since whole thing didn't work
  if it is not loaded.

Signed-off-by: Alexey <lesha.ogonkov@gmail.com>
---
    Fix zsh installation instructions
    
     * Fix wrong script in completion configuration. zsh wants bash
       completion path here, not path to itself.
     * Add compinit autoload command, since whole thing didn't work if it is
       not loaded.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-814%2Fogonkov%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-814/ogonkov/patch-1-v1
Pull-Request: https://github.com/git/git/pull/814

 contrib/completion/git-completion.zsh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index ce47e86b60..107869036d 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -9,13 +9,14 @@
 #
 # If your script is somewhere else, you can configure it on your ~/.zshrc:
 #
-#  zstyle ':completion:*:*:git:*' script ~/.git-completion.zsh
+#  zstyle ':completion:*:*:git:*' script ~/.git-completion.bash
 #
 # The recommended way to install this script is to make a copy of it in
 # ~/.zsh/ directory as ~/.zsh/git-completion.zsh and then add the following
 # to your ~/.zshrc file:
 #
 #  fpath=(~/.zsh $fpath)
+#  autoload -Uz compinit && compinit
 
 complete ()
 {

base-commit: a08a83db2bf27f015bec9a435f6d73e223c21c5e
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-07-02 10:51 [PATCH] Fix zsh installation instructions Alexey via GitGitGadget
@ 2020-07-06 22:42 ` Junio C Hamano
  2020-10-14 15:10   ` Stefan Haller
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2020-07-06 22:42 UTC (permalink / raw)
  To: Alexey via GitGitGadget; +Cc: git, Alexey

> From: Alexey <lesha.ogonkov@gmail.com>

Thanks.

Any zsh users raise their hands?  Does this change look sane?

> - Fix wrong script in completion configuration. zsh wants bash completion
>   path here, not path to itself.
> - Add `compinit` autoload command, since whole thing didn't work
>   if it is not loaded.
>
> Signed-off-by: Alexey <lesha.ogonkov@gmail.com>
> ---
>     Fix zsh installation instructions
>     
>      * Fix wrong script in completion configuration. zsh wants bash
>        completion path here, not path to itself.
>      * Add compinit autoload command, since whole thing didn't work if it is
>        not loaded.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-814%2Fogonkov%2Fpatch-1-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-814/ogonkov/patch-1-v1
> Pull-Request: https://github.com/git/git/pull/814
>
>  contrib/completion/git-completion.zsh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
> index ce47e86b60..107869036d 100644
> --- a/contrib/completion/git-completion.zsh
> +++ b/contrib/completion/git-completion.zsh
> @@ -9,13 +9,14 @@
>  #
>  # If your script is somewhere else, you can configure it on your ~/.zshrc:
>  #
> -#  zstyle ':completion:*:*:git:*' script ~/.git-completion.zsh
> +#  zstyle ':completion:*:*:git:*' script ~/.git-completion.bash
>  #
>  # The recommended way to install this script is to make a copy of it in
>  # ~/.zsh/ directory as ~/.zsh/git-completion.zsh and then add the following
>  # to your ~/.zshrc file:
>  #
>  #  fpath=(~/.zsh $fpath)
> +#  autoload -Uz compinit && compinit
>  
>  complete ()
>  {
>
> base-commit: a08a83db2bf27f015bec9a435f6d73e223c21c5e

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-07-06 22:42 ` Junio C Hamano
@ 2020-10-14 15:10   ` Stefan Haller
  2020-10-16 17:06     ` Junio C Hamano
  2020-10-25  3:29     ` Felipe Contreras
  0 siblings, 2 replies; 13+ messages in thread
From: Stefan Haller @ 2020-10-14 15:10 UTC (permalink / raw)
  To: Junio C Hamano, Alexey via GitGitGadget; +Cc: git, Alexey

On 07.07.20 0:42, Junio C Hamano wrote:
>> From: Alexey <lesha.ogonkov@gmail.com>
> 
> Thanks.
> 
> Any zsh users raise their hands?  Does this change look sane?

Sorry for the late reply, I only saw this now.

Yes, the change is sane. The wrong file extension (.zsh vs. .bash) was a 
documentation bug that I had noticed myself, but forgot to submit a 
patch for.

The other hunk (adding compinit) is not so important to me; I suppose it 
was not in the original version because most zsh users already have this 
in their .zshrc anyway. But it's not wrong, and doesn't hurt to have 
here, I guess.

Best,
Stefan


>> - Fix wrong script in completion configuration. zsh wants bash completion
>>    path here, not path to itself.
>> - Add `compinit` autoload command, since whole thing didn't work
>>    if it is not loaded.
>>
>> Signed-off-by: Alexey <lesha.ogonkov@gmail.com>
>> ---
>>      Fix zsh installation instructions
>>      
>>       * Fix wrong script in completion configuration. zsh wants bash
>>         completion path here, not path to itself.
>>       * Add compinit autoload command, since whole thing didn't work if it is
>>         not loaded.
>>
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-814%2Fogonkov%2Fpatch-1-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-814/ogonkov/patch-1-v1
>> Pull-Request: https://github.com/git/git/pull/814
>>
>>   contrib/completion/git-completion.zsh | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
>> index ce47e86b60..107869036d 100644
>> --- a/contrib/completion/git-completion.zsh
>> +++ b/contrib/completion/git-completion.zsh
>> @@ -9,13 +9,14 @@
>>   #
>>   # If your script is somewhere else, you can configure it on your ~/.zshrc:
>>   #
>> -#  zstyle ':completion:*:*:git:*' script ~/.git-completion.zsh
>> +#  zstyle ':completion:*:*:git:*' script ~/.git-completion.bash
>>   #
>>   # The recommended way to install this script is to make a copy of it in
>>   # ~/.zsh/ directory as ~/.zsh/git-completion.zsh and then add the following
>>   # to your ~/.zshrc file:
>>   #
>>   #  fpath=(~/.zsh $fpath)
>> +#  autoload -Uz compinit && compinit
>>   
>>   complete ()
>>   {
>>
>> base-commit: a08a83db2bf27f015bec9a435f6d73e223c21c5e
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-14 15:10   ` Stefan Haller
@ 2020-10-16 17:06     ` Junio C Hamano
  2020-10-25  3:29     ` Felipe Contreras
  1 sibling, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2020-10-16 17:06 UTC (permalink / raw)
  To: Stefan Haller; +Cc: Alexey via GitGitGadget, git, Alexey

Stefan Haller <lists@haller-berlin.de> writes:

> On 07.07.20 0:42, Junio C Hamano wrote:
>>> From: Alexey <lesha.ogonkov@gmail.com>
>> Thanks.
>> Any zsh users raise their hands?  Does this change look sane?
>
> Sorry for the late reply, I only saw this now.
>
> Yes, the change is sane. The wrong file extension (.zsh vs. .bash) was
> a documentation bug that I had noticed myself, but forgot to submit a 
> patch for.
>
> The other hunk (adding compinit) is not so important to me; I suppose
> it was not in the original version because most zsh users already have
> this in their .zshrc anyway. But it's not wrong, and doesn't hurt to
> have here, I guess.

Thanks.  Will queue.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-14 15:10   ` Stefan Haller
  2020-10-16 17:06     ` Junio C Hamano
@ 2020-10-25  3:29     ` Felipe Contreras
  2020-10-27  8:29       ` Stefan Haller
  2020-10-27  8:32       ` Stefan Haller
  1 sibling, 2 replies; 13+ messages in thread
From: Felipe Contreras @ 2020-10-25  3:29 UTC (permalink / raw)
  To: Stefan Haller; +Cc: Junio C Hamano, Alexey via GitGitGadget, Git, Alexey

On Wed, Oct 14, 2020 at 12:39 PM Stefan Haller <lists@haller-berlin.de> wrote:

> Yes, the change is sane. The wrong file extension (.zsh vs. .bash) was a
> documentation bug that I had noticed myself, but forgot to submit a
> patch for.

Yes, a patch was wrongly applied that changed it from .sh to .zsh.

> The other hunk (adding compinit) is not so important to me; I suppose it
> was not in the original version because most zsh users already have this
> in their .zshrc anyway. But it's not wrong, and doesn't hurt to have
> here, I guess.

If you don't have compinit, then how is the '_git' script being loaded
in the first place?

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-25  3:29     ` Felipe Contreras
@ 2020-10-27  8:29       ` Stefan Haller
  2020-10-27  8:32       ` Stefan Haller
  1 sibling, 0 replies; 13+ messages in thread
From: Stefan Haller @ 2020-10-27  8:29 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, Alexey via GitGitGadget, Git, Alexey

On 25.10.20 4:29, Felipe Contreras wrote:
> On Wed, Oct 14, 2020 at 12:39 PM Stefan Haller <lists@haller-berlin.de> wrote:
> 
>> Yes, the change is sane. The wrong file extension (.zsh vs. .bash) was a
>> documentation bug that I had noticed myself, but forgot to submit a
>> patch for.
> 
> Yes, a patch was wrongly applied that changed it from .sh to .zsh.
> 
>> The other hunk (adding compinit) is not so important to me; I suppose it
>> was not in the original version because most zsh users already have this
>> in their .zshrc anyway. But it's not wrong, and doesn't hurt to have
>> here, I guess.
> 
> If you don't have compinit, then how is the '_git' script being loaded
> in the first place?
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-25  3:29     ` Felipe Contreras
  2020-10-27  8:29       ` Stefan Haller
@ 2020-10-27  8:32       ` Stefan Haller
  2020-10-27  8:59         ` Лёша Огоньков
  1 sibling, 1 reply; 13+ messages in thread
From: Stefan Haller @ 2020-10-27  8:32 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, Alexey via GitGitGadget, Git, Alexey

On 25.10.20 4:29, Felipe Contreras wrote:
>> The other hunk (adding compinit) is not so important to me; I suppose it
>> was not in the original version because most zsh users already have this
>> in their .zshrc anyway. But it's not wrong, and doesn't hurt to have
>> here, I guess.
> 
> If you don't have compinit, then how is the '_git' script being loaded
> in the first place?

I didn't say it's unnecessary to have it in your .zshrc. I just said 
it's maybe unnecessary to document it here because most zsh users have 
it in their .zshrc already anyway.

(Sorry for the other empty mail, I hit the wrong button...)

Best,
Stefan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-27  8:32       ` Stefan Haller
@ 2020-10-27  8:59         ` Лёша Огоньков
  2020-10-27 20:00           ` Junio C Hamano
  2020-10-28  1:31           ` Felipe Contreras
  0 siblings, 2 replies; 13+ messages in thread
From: Лёша Огоньков @ 2020-10-27  8:59 UTC (permalink / raw)
  To: Stefan Haller
  Cc: Felipe Contreras, Junio C Hamano, Alexey via GitGitGadget, Git

As an inexperienced zsh user it took me ages to understand why the
whole thing is not working.

Regards
Alexey

On Tue, 27 Oct 2020 at 11:32, Stefan Haller <lists@haller-berlin.de> wrote:
>
> On 25.10.20 4:29, Felipe Contreras wrote:
> >> The other hunk (adding compinit) is not so important to me; I suppose it
> >> was not in the original version because most zsh users already have this
> >> in their .zshrc anyway. But it's not wrong, and doesn't hurt to have
> >> here, I guess.
> >
> > If you don't have compinit, then how is the '_git' script being loaded
> > in the first place?
>
> I didn't say it's unnecessary to have it in your .zshrc. I just said
> it's maybe unnecessary to document it here because most zsh users have
> it in their .zshrc already anyway.
>
> (Sorry for the other empty mail, I hit the wrong button...)
>
> Best,
> Stefan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-27  8:59         ` Лёша Огоньков
@ 2020-10-27 20:00           ` Junio C Hamano
  2020-10-28  1:45             ` Felipe Contreras
  2020-10-28  1:31           ` Felipe Contreras
  1 sibling, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2020-10-27 20:00 UTC (permalink / raw)
  To: Лёша
	Огоньков
  Cc: Stefan Haller, Felipe Contreras, Alexey via GitGitGadget, Git

Лёша Огоньков  <lesha.ogonkov@gmail.com> writes:

> On Tue, 27 Oct 2020 at 11:32, Stefan Haller <lists@haller-berlin.de> wrote:
>>
>> On 25.10.20 4:29, Felipe Contreras wrote:
>> >> The other hunk (adding compinit) is not so important to me; I suppose it
>> >> was not in the original version because most zsh users already have this
>> >> in their .zshrc anyway. But it's not wrong, and doesn't hurt to have
>> >> here, I guess.
>> >
>> > If you don't have compinit, then how is the '_git' script being loaded
>> > in the first place?
>>
>> I didn't say it's unnecessary to have it in your .zshrc. I just said
>> it's maybe unnecessary to document it here because most zsh users have
>> it in their .zshrc already anyway.
>
> As an inexperienced zsh user it took me ages to understand why the
> whole thing is not working.

So,... even though it may look to more experienced zsh users that it
is unnecessary to document it in this file, in your opinion, it is a
good idea to mention "compinit" to help less experienced users?

In any case, the patch in question is the only thing in flight that
conflicts with Felipe's 29 patch series, and the change to zstyle
line is common between both efforts, so it is just between adding
the "autoload -Uz compinit && compinit" near the fpath=(...) thing
or leaving it out.

I'll let those who know zsh to figure it out.  I suspect that the
resolution would be either to ask Felipe to rebase his on yours,
rebase yours on top of Felipe's, or just drop yours (if "autoload"
thing is unnecessary as Stefan suspects---I cannot tell what Felipe
wants to suggest by his response, between "unless you have compinit
you wouldn't be using this script so why write it?" and "your users
need to be taught about the need _somehow_, so why not make this the
place to do so?").

Thanks.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-27  8:59         ` Лёша Огоньков
  2020-10-27 20:00           ` Junio C Hamano
@ 2020-10-28  1:31           ` Felipe Contreras
  1 sibling, 0 replies; 13+ messages in thread
From: Felipe Contreras @ 2020-10-28  1:31 UTC (permalink / raw)
  To: Лёша
	Огоньков
  Cc: Stefan Haller, Junio C Hamano, Alexey via GitGitGadget, Git

On Tue, Oct 27, 2020 at 2:59 AM Лёша Огоньков <lesha.ogonkov@gmail.com> wrote:
>
> As an inexperienced zsh user it took me ages to understand why the
> whole thing is not working.

What was the reason? The filename wasn't correct?

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-27 20:00           ` Junio C Hamano
@ 2020-10-28  1:45             ` Felipe Contreras
  2020-10-29 17:50               ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Felipe Contreras @ 2020-10-28  1:45 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Лёша
	Огоньков,
	Stefan Haller, Alexey via GitGitGadget, Git

On Tue, Oct 27, 2020 at 2:00 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Лёша Огоньков  <lesha.ogonkov@gmail.com> writes:

> So,... even though it may look to more experienced zsh users that it
> is unnecessary to document it in this file, in your opinion, it is a
> good idea to mention "compinit" to help less experienced users?

The first time you run zsh you are greeted with a configuration dialog
that includes this question:

---
The new completion system (compsys) allows you to complete
commands, arguments and special shell syntax such as variables.  It provides
completions for a wide range of commonly used commands in most cases simply
by typing the TAB key.  Documentation is in the zshcompsys manual page.
If it is not turned on, only a few simple completions such as filenames
are available but the time to start the shell is slightly shorter.

You can:
  (1)  Turn on completion with the default options.

  (2)  Run the configuration tool (compinstall).  You can also run
       this from the command line with the following commands:
        autoload -Uz compinstall
        compinstall
       if you don't want to configure completion now.

  (0)  Don't turn on completion.
---

If you don't turn on completion, the completion doesn't work for *any* command.

I think most users would understand why git completion doesn't work if
completion doesn't work for any command.

> In any case, the patch in question is the only thing in flight that
> conflicts with Felipe's 29 patch series, and the change to zstyle
> line is common between both efforts, so it is just between adding
> the "autoload -Uz compinit && compinit" near the fpath=(...) thing
> or leaving it out.

I would rather leave it out.

If we add this configuration, I would do it in a separate patch that
includes text explaining what that line is, and why it might be
needed.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-28  1:45             ` Felipe Contreras
@ 2020-10-29 17:50               ` Junio C Hamano
  2020-10-29 19:09                 ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2020-10-29 17:50 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Лёша
	Огоньков,
	Stefan Haller, Alexey via GitGitGadget, Git

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Tue, Oct 27, 2020 at 2:00 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Лёша Огоньков  <lesha.ogonkov@gmail.com> writes:
>
>> So,... even though it may look to more experienced zsh users that it
>> is unnecessary to document it in this file, in your opinion, it is a
>> good idea to mention "compinit" to help less experienced users?
>
> The first time you run zsh you are greeted with a configuration dialog
> that includes this question:

Ahh, yes, that reminds me why I gave up trying it out the last time.
The large dialog asking my permission to contaminate ~/.* was simply
too scary and distasteful for a "dip my toe in the water" sightseer.

> If you don't turn on completion, the completion doesn't work for *any* command.
>
> I think most users would understand why git completion doesn't work if
> completion doesn't work for any command.

OK, that sort of makes sense.

> I would rather leave it out.

Makes sense.  Instead of adjusting my rerere database for that,
ejecting lo/zsh-completion topic would be easier---what the other
half of that patch does is already in your 29-patch series.

Thanks.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix zsh installation instructions
  2020-10-29 17:50               ` Junio C Hamano
@ 2020-10-29 19:09                 ` Junio C Hamano
  0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2020-10-29 19:09 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Лёша
	Огоньков,
	Stefan Haller, Alexey via GitGitGadget, Git

Junio C Hamano <gitster@pobox.com> writes:

>> I would rather leave it out.
>
> Makes sense.  Instead of adjusting my rerere database for that,
> ejecting lo/zsh-completion topic would be easier---what the other
> half of that patch does is already in your 29-patch series.

Actually, I already have an entry in my rerere database to resolve
the merge of these topics to drop the "autoload" suggestion, so I
won't have to do anything other than just queuing the v3 of your
29-patch series.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-02 10:51 [PATCH] Fix zsh installation instructions Alexey via GitGitGadget
2020-07-06 22:42 ` Junio C Hamano
2020-10-14 15:10   ` Stefan Haller
2020-10-16 17:06     ` Junio C Hamano
2020-10-25  3:29     ` Felipe Contreras
2020-10-27  8:29       ` Stefan Haller
2020-10-27  8:32       ` Stefan Haller
2020-10-27  8:59         ` Лёша Огоньков
2020-10-27 20:00           ` Junio C Hamano
2020-10-28  1:45             ` Felipe Contreras
2020-10-29 17:50               ` Junio C Hamano
2020-10-29 19:09                 ` Junio C Hamano
2020-10-28  1:31           ` Felipe Contreras

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git