zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH] Remove redundancies from `git` completion
@ 2021-08-30 11:17 Marlon Richert
  2021-08-30 11:41 ` Mikael Magnusson
  2021-09-01 13:56 ` Daniel Shahaf
  0 siblings, 2 replies; 13+ messages in thread
From: Marlon Richert @ 2021-08-30 11:17 UTC (permalink / raw)
  To: Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

In _git-show, remove calls to __git_commits and __git_tags.
  Rationale: 'commits' and 'tags' are already added by __git_trees.
  Adding them twice makes `zstyle ... tag-order` give unexpected
  results.

In __git_recent_commits, remove the line that adds 'heads'.
  Rationale: The completion for most subcommands already adds
  'heads-local' and 'heads-remote'. Adding an additional 'heads' inside
  __git_recent_commits results in heads being listed twice in two
  separate groups.


By the way: Why is there a `(( $+functions[_git-XXX] )) ||` statement
in front of each function inside Completion/Unix/Command/_git ? Can
those be removed?


Patch attached, below.

[-- Attachment #2: 0001-Remove-redundancies-from-git-completion.txt --]
[-- Type: text/plain, Size: 3742 bytes --]

From fcaa9f4b92b0e660f0c2789560251f456314d8af Mon Sep 17 00:00:00 2001
From: Marlon Richert <marlonrichert@users.noreply.github.com>
Date: Mon, 30 Aug 2021 12:55:05 +0300
Subject: [PATCH] Remove redundancies from `git` completion

In _git-show, remove calls to __git_commits and __git_tags.
  Rationale: 'commits' and 'tags' are already added by __git_trees.
  Adding them twice makes `zstyle ... tag-order` give unexpected
  results.

In __git_recent_commits, remove the line that adds 'heads'.
  Rationale: The completion for most subcommands already adds
  'heads-local' and 'heads-remote'. Adding an additional 'heads' inside
  __git_recent_commits results in heads being listed twice in two
  separate groups.
---
 Completion/Unix/Command/_git | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index a82b70e83..569ca7a1e 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -685,8 +685,8 @@ _git-commit () {
   # TODO: --interactive isn't explicitly listed in the documentation.
   _arguments -S -s $endopt \
     '(-a --all --interactive -o --only -i --include *)'{-a,--all}'[stage all modified and deleted paths]' \
-    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_recent_commits' \
-    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_recent_commits' \
+    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commit_objects_prefer_recent' \
+    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commit_objects_prefer_recent' \
     $reset_author_opt \
     '(        --porcelain --dry-run)--short[dry run with short output format]' \
     '--branch[show branch information]' \
@@ -1656,7 +1656,7 @@ _git-revert () {
     '*'{-X,--strategy-option=}'[pass merge-strategy-specific option to merge strategy]:option' \
     '(-S --gpg-sign --no-gpg-sign)'{-S-,--gpg-sign=-}'[GPG-sign the commit]::key id' \
     "(-S --gpg-sign --no-gpg-sign)--no-gpg-sign[don't GPG-sign the commit]" \
-    ': :__git_recent_commits'
+    ': :__git_commit_objects_prefer_recent'
 }
 
 (( $+functions[_git-rm] )) ||
@@ -1763,10 +1763,9 @@ _git-show () {
   case $state in
     (object)
       _alternative \
-        'commits::__git_commits' \
-        'tags::__git_tags' \
-        'trees::__git_trees' \
-        'blobs::__git_blobs' && ret=0
+            'trees::__git_trees' \
+            'blobs::__git_blobs' \
+          && ret=0
       ;;
   esac
 
@@ -6920,7 +6919,7 @@ __git_commit_objects () {
 (( $+functions[__git_recent_commits] )) ||
 __git_recent_commits () {
   local gitdir expl start
-  declare -a descr tags heads commits argument_array_names commit_opts
+  declare -a descr tags commits argument_array_names commit_opts
   local h i j k ret
   integer distance_from_head
   local label
@@ -6985,11 +6984,8 @@ __git_recent_commits () {
     j=${${j# \(}%\)} # strip leading ' (' and trailing ')'
     j=${j/ ->/,}  # Convert " -> master, origin/master".
     for j in ${(s:, :)j}; do
-      if [[ $j == 'tag: '* ]] ; then
-        tags+=( ${j#tag: } )
-      else
-        heads+=( $j )
-      fi
+      [[ $j == 'tag: '* ]] &&
+          tags+=( ${j#tag: } )
     done
   done
 
@@ -6999,8 +6995,6 @@ __git_recent_commits () {
   _describe -V -t commits 'recent commit object name' descr && ret=0
   expl=()
   _wanted commit-tags expl 'commit tag' compadd "$@" -a - tags && ret=0
-  expl=()
-  _wanted heads expl 'head' compadd -M "r:|/=* r:|=*" "$@" -a - heads && ret=0
   return ret
 }
 
-- 
2.33.0


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-08-30 11:17 [PATCH] Remove redundancies from `git` completion Marlon Richert
@ 2021-08-30 11:41 ` Mikael Magnusson
  2021-08-30 12:34   ` Marlon Richert
  2021-09-01 13:56 ` Daniel Shahaf
  1 sibling, 1 reply; 13+ messages in thread
From: Mikael Magnusson @ 2021-08-30 11:41 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

On 8/30/21, Marlon Richert <marlon.richert@gmail.com> wrote:
> In _git-show, remove calls to __git_commits and __git_tags.
>   Rationale: 'commits' and 'tags' are already added by __git_trees.
>   Adding them twice makes `zstyle ... tag-order` give unexpected
>   results.
>
> In __git_recent_commits, remove the line that adds 'heads'.
>   Rationale: The completion for most subcommands already adds
>   'heads-local' and 'heads-remote'. Adding an additional 'heads' inside
>   __git_recent_commits results in heads being listed twice in two
>   separate groups.
>
>
> By the way: Why is there a `(( $+functions[_git-XXX] )) ||` statement
> in front of each function inside Completion/Unix/Command/_git ? Can
> those be removed?

They are there so that you can override their implementation easily.

-- 
Mikael Magnusson


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-08-30 11:41 ` Mikael Magnusson
@ 2021-08-30 12:34   ` Marlon Richert
  2021-08-30 12:42     ` Mikael Magnusson
  0 siblings, 1 reply; 13+ messages in thread
From: Marlon Richert @ 2021-08-30 12:34 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: Zsh hackers list

On Mon, Aug 30, 2021 at 2:41 PM Mikael Magnusson <mikachu@gmail.com> wrote:
>
> On 8/30/21, Marlon Richert <marlon.richert@gmail.com> wrote:
> > By the way: Why is there a `(( $+functions[_git-XXX] )) ||` statement
> > in front of each function inside Completion/Unix/Command/_git ? Can
> > those be removed?
>
> They are there so that you can override their implementation easily.

Is that documented somewhere?

And for purposes of overriding, wouldn't it be cleaner to simply
supply all sub-functions as separate #autoload files?


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-08-30 12:34   ` Marlon Richert
@ 2021-08-30 12:42     ` Mikael Magnusson
  2021-08-31  5:52       ` Marlon Richert
  0 siblings, 1 reply; 13+ messages in thread
From: Mikael Magnusson @ 2021-08-30 12:42 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

On 8/30/21, Marlon Richert <marlon.richert@gmail.com> wrote:
> On Mon, Aug 30, 2021 at 2:41 PM Mikael Magnusson <mikachu@gmail.com> wrote:
>>
>> On 8/30/21, Marlon Richert <marlon.richert@gmail.com> wrote:
>> > By the way: Why is there a `(( $+functions[_git-XXX] )) ||` statement
>> > in front of each function inside Completion/Unix/Command/_git ? Can
>> > those be removed?
>>
>> They are there so that you can override their implementation easily.
>
> Is that documented somewhere?

Not explicitly, afaik.

> And for purposes of overriding, wouldn't it be cleaner to simply
> supply all sub-functions as separate #autoload files?

I don't think anyone is interested in maintaining 285 separate
completer files for git :).

-- 
Mikael Magnusson


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-08-30 12:42     ` Mikael Magnusson
@ 2021-08-31  5:52       ` Marlon Richert
  2021-09-01 14:03         ` Daniel Shahaf
  0 siblings, 1 reply; 13+ messages in thread
From: Marlon Richert @ 2021-08-31  5:52 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: Zsh hackers list

On Mon, Aug 30, 2021 at 3:42 PM Mikael Magnusson <mikachu@gmail.com> wrote:
>
> On 8/30/21, Marlon Richert <marlon.richert@gmail.com> wrote:
> > On Mon, Aug 30, 2021 at 2:41 PM Mikael Magnusson <mikachu@gmail.com> wrote:
> >>
> >> On 8/30/21, Marlon Richert <marlon.richert@gmail.com> wrote:
> >> > By the way: Why is there a `(( $+functions[_git-XXX] )) ||` statement
> >> > in front of each function inside Completion/Unix/Command/_git ? Can
> >> > those be removed?
> >>
> >> They are there so that you can override their implementation easily.
> >
> > Is that documented somewhere?
>
> Not explicitly, afaik.

I see this pattern is used in several completer functions.Should it be
documented?

>
> > And for purposes of overriding, wouldn't it be cleaner to simply
> > supply all sub-functions as separate #autoload files?
>
> I don't think anyone is interested in maintaining 285 separate
> completer files for git :).

If you count the #autoload line, it would be the exact same number of
lines of code, but you wouldn't need to duplicate the function name
for each function in an `if` statement. That would be taken care of
automatically by `autoload`.


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-08-30 11:17 [PATCH] Remove redundancies from `git` completion Marlon Richert
  2021-08-30 11:41 ` Mikael Magnusson
@ 2021-09-01 13:56 ` Daniel Shahaf
  2021-09-02  8:57   ` Marlon Richert
  1 sibling, 1 reply; 13+ messages in thread
From: Daniel Shahaf @ 2021-09-01 13:56 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

Marlon Richert wrote on Mon, Aug 30, 2021 at 14:17:17 +0300:
> In _git-show, remove calls to __git_commits and __git_tags.
>   Rationale: 'commits' and 'tags' are already added by __git_trees.
>   Adding them twice makes `zstyle ... tag-order` give unexpected
>   results.
> 

Complete example, please?

> In __git_recent_commits, remove the line that adds 'heads'.
>   Rationale: The completion for most subcommands already adds
>   'heads-local' and 'heads-remote'. Adding an additional 'heads' inside
>   __git_recent_commits results in heads being listed twice in two
>   separate groups.

Complete example, please?  Also, what about those subcommands that
*don't* already add heads-local and heads-remote?



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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-08-31  5:52       ` Marlon Richert
@ 2021-09-01 14:03         ` Daniel Shahaf
  2021-09-01 18:22           ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Shahaf @ 2021-09-01 14:03 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Mikael Magnusson, Zsh hackers list

Marlon Richert wrote on Tue, Aug 31, 2021 at 08:52:02 +0300:
> On Mon, Aug 30, 2021 at 3:42 PM Mikael Magnusson <mikachu@gmail.com> wrote:
> >
> > On 8/30/21, Marlon Richert <marlon.richert@gmail.com> wrote:
> > > On Mon, Aug 30, 2021 at 2:41 PM Mikael Magnusson <mikachu@gmail.com> wrote:
> > >>
> > >> On 8/30/21, Marlon Richert <marlon.richert@gmail.com> wrote:
> > >> > By the way: Why is there a `(( $+functions[_git-XXX] )) ||` statement
> > >> > in front of each function inside Completion/Unix/Command/_git ? Can
> > >> > those be removed?
> > >>
> > >> They are there so that you can override their implementation easily.
> > >
> > > Is that documented somewhere?
> >
> > Not explicitly, afaik.
> 
> I see this pattern is used in several completer functions.Should it be
> documented?

Maybe.

If we document it, that means we'll have to also document compatibility
promises for those functions, because people can't be expected to
upgrade their dotfiles and fpath's in lockstep (e.g., on shared
machines).

I don't think we generally treat those functions as stable API's.
Therefore, it might be better to deprecate the practice of making them
overridable in this way.

Anyone who wants to override them will still be able to do «autoload +X
_git && _git && _git-foo() { … }» (or a better solution which someone
will probably post in reply to this one).

> >
> > > And for purposes of overriding, wouldn't it be cleaner to simply
> > > supply all sub-functions as separate #autoload files?
> >
> > I don't think anyone is interested in maintaining 285 separate
> > completer files for git :).
> 
> If you count the #autoload line,

We don't count lines.


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-09-01 14:03         ` Daniel Shahaf
@ 2021-09-01 18:22           ` Bart Schaefer
  2021-09-02  6:17             ` Marlon Richert
  0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2021-09-01 18:22 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Marlon Richert, Mikael Magnusson, Zsh hackers list

On Wed, Sep 1, 2021 at 7:04 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Anyone who wants to override them will still be able to do «autoload +X
> _git && _git && _git-foo() { … }» (or a better solution which someone
> will probably post in reply to this one).

It would be nice if e.g. «autoload $HOME/gitfuncs/_git_foo» were
sufficient, but the explicit definition of _git_foo inside _git will
clobber that autoload.


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-09-01 18:22           ` Bart Schaefer
@ 2021-09-02  6:17             ` Marlon Richert
  2021-09-02 20:51               ` Oliver Kiddle
  0 siblings, 1 reply; 13+ messages in thread
From: Marlon Richert @ 2021-09-02  6:17 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Daniel Shahaf, Mikael Magnusson, Zsh hackers list

On Wed, Sep 1, 2021 at 9:23 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Wed, Sep 1, 2021 at 7:04 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > Anyone who wants to override them will still be able to do «autoload +X
> > _git && _git && _git-foo() { … }» (or a better solution which someone
> > will probably post in reply to this one).
>
> It would be nice if e.g. «autoload $HOME/gitfuncs/_git_foo» were
> sufficient, but the explicit definition of _git_foo inside _git will
> clobber that autoload.

Moving all _git_foo functions from the _git file into separate,
#autoload files would solve that.


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-09-01 13:56 ` Daniel Shahaf
@ 2021-09-02  8:57   ` Marlon Richert
  0 siblings, 0 replies; 13+ messages in thread
From: Marlon Richert @ 2021-09-02  8:57 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 1748 bytes --]

On Wed, Sep 1, 2021 at 4:56 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Marlon Richert wrote on Mon, Aug 30, 2021 at 14:17:17 +0300:
> > In _git-show, remove calls to __git_commits and __git_tags.
> >   Rationale: 'commits' and 'tags' are already added by __git_trees.
> >   Adding them twice makes `zstyle ... tag-order` give unexpected
> >   results.
> >
>
> Complete example, please?

Before the patch, setting `zstyle '*' tag-order '! commit-tags
heads-remote'` failed to hide 'commit-tags' and 'heads-remote' from
`git show` completion. After the patch, this works correctly and will
show 'commit-tags' or 'heads-remote' only if other tags produce no
matches.

>
> > In __git_recent_commits, remove the line that adds 'heads'.
> >   Rationale: The completion for most subcommands already adds
> >   'heads-local' and 'heads-remote'. Adding an additional 'heads' inside
> >   __git_recent_commits results in heads being listed twice in two
> >   separate groups.
>
> Complete example, please?

Before the patch, `git log` completion listed groups 'local head',
'remote head', then other groups, and then group 'head', which
repeated items from the first two groups. After the patch, the last
group is omitted.

> Also, what about those subcommands that
> *don't* already add heads-local and heads-remote?

Good observation. Those would be the completion functions for `git
commit --fixup`, `git commit --squash`, `git revert` and `git
send-email`. My patch tried to handle that case, too, but some more
testing revealed that it didn't work correctly.

Here is a new patch that fixes this by making these four completions
use __git_commits, which prefers recent commits and tags, but can also
complete any other commits, tags or heads.

[-- Attachment #2: 0001-Eliminate-duplicate-completions-from-_git.txt --]
[-- Type: text/plain, Size: 4268 bytes --]

From 4442829b53506d705e5ffc5ea2306fe9116d6474 Mon Sep 17 00:00:00 2001
From: Marlon Richert <marlonrichert@users.noreply.github.com>
Date: Thu, 2 Sep 2021 11:46:15 +0300
Subject: [PATCH] Eliminate duplicate completions from _git

Before this patch, setting
`zstyle '*' tag-order '! commit-tags heads-remote'`
failed to hide 'commit-tags' and 'heads-remote' from `git show`
completion. After this patch, this works correctly and will show
'commit-tags' or 'heads-remote' only if other tags produce no matches.

Before this patch, any completion using __git_commits (for example,
`git log`) listed groups 'local head', 'remote head', then other groups,
and then group 'head', which repeated items from the first two groups.
After this patch, the last group is omitted.

Additionally, after this patch, completions that previously could match
only recent commits, tags and heads, can now match _any_ commits, tags
or heads, but will prefer recent commits and tags.
---
 Completion/Unix/Command/_git | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index a82b70e83..bbe7475bd 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -685,8 +685,8 @@ _git-commit () {
   # TODO: --interactive isn't explicitly listed in the documentation.
   _arguments -S -s $endopt \
     '(-a --all --interactive -o --only -i --include *)'{-a,--all}'[stage all modified and deleted paths]' \
-    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_recent_commits' \
-    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_recent_commits' \
+    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commits' \
+    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commits' \
     $reset_author_opt \
     '(        --porcelain --dry-run)--short[dry run with short output format]' \
     '--branch[show branch information]' \
@@ -1656,7 +1656,7 @@ _git-revert () {
     '*'{-X,--strategy-option=}'[pass merge-strategy-specific option to merge strategy]:option' \
     '(-S --gpg-sign --no-gpg-sign)'{-S-,--gpg-sign=-}'[GPG-sign the commit]::key id' \
     "(-S --gpg-sign --no-gpg-sign)--no-gpg-sign[don't GPG-sign the commit]" \
-    ': :__git_recent_commits'
+    ': :__git_commits'
 }
 
 (( $+functions[_git-rm] )) ||
@@ -1763,10 +1763,9 @@ _git-show () {
   case $state in
     (object)
       _alternative \
-        'commits::__git_commits' \
-        'tags::__git_tags' \
-        'trees::__git_trees' \
-        'blobs::__git_blobs' && ret=0
+            'trees::__git_trees' \
+            'blobs::__git_blobs' \
+          && ret=0
       ;;
   esac
 
@@ -4536,7 +4535,7 @@ _git-send-email () {
     '(- *)--dump-aliases[dump configured aliases and exit]' \
     '*: : _alternative -O expl
       "files:file:_files"
-      "commits:recent commit object name:__git_commit_objects_prefer_recent"'
+      "commits:recent commit object name:__git_commits"'
 }
 
 (( $+functions[_git-svn] )) ||
@@ -6920,7 +6919,7 @@ __git_commit_objects () {
 (( $+functions[__git_recent_commits] )) ||
 __git_recent_commits () {
   local gitdir expl start
-  declare -a descr tags heads commits argument_array_names commit_opts
+  declare -a descr tags commits argument_array_names commit_opts
   local h i j k ret
   integer distance_from_head
   local label
@@ -6985,11 +6984,8 @@ __git_recent_commits () {
     j=${${j# \(}%\)} # strip leading ' (' and trailing ')'
     j=${j/ ->/,}  # Convert " -> master, origin/master".
     for j in ${(s:, :)j}; do
-      if [[ $j == 'tag: '* ]] ; then
-        tags+=( ${j#tag: } )
-      else
-        heads+=( $j )
-      fi
+      [[ $j == 'tag: '* ]] &&
+          tags+=( ${j#tag: } )
     done
   done
 
@@ -6999,8 +6995,6 @@ __git_recent_commits () {
   _describe -V -t commits 'recent commit object name' descr && ret=0
   expl=()
   _wanted commit-tags expl 'commit tag' compadd "$@" -a - tags && ret=0
-  expl=()
-  _wanted heads expl 'head' compadd -M "r:|/=* r:|=*" "$@" -a - heads && ret=0
   return ret
 }
 
-- 
2.33.0


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-09-02  6:17             ` Marlon Richert
@ 2021-09-02 20:51               ` Oliver Kiddle
  2021-09-02 20:54                 ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Oliver Kiddle @ 2021-09-02 20:51 UTC (permalink / raw)
  To: Zsh hackers list

Marlon Richert wrote:
> On Wed, Sep 1, 2021 at 9:23 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > It would be nice if e.g. «autoload $HOME/gitfuncs/_git_foo» were
> > sufficient, but the explicit definition of _git_foo inside _git will
> > clobber that autoload.
>
> Moving all _git_foo functions from the _git file into separate,
> #autoload files would solve that.

The existing (( $+functions[_git-foo] )) || test also solves it. The
only thing you need do is put your overrides in $fpath.

In practice, I doubt many people ever do. And if they do, we've probably
done a poor job somewhere.

This function existence check is not a pattern I followed elsewhere.
There are a few things that were done somewhat differently in _git
compared to other functions and some of that has been copied. I don't
see the point in the double initial underscore naming on some functions.
This particular test was rather annoying before $functions_source
existed because it made it more difficult to fully reload all _git
functions.

Oliver


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-09-02 20:51               ` Oliver Kiddle
@ 2021-09-02 20:54                 ` Bart Schaefer
  2021-09-02 21:17                   ` Oliver Kiddle
  0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2021-09-02 20:54 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: Zsh hackers list

On Thu, Sep 2, 2021 at 1:51 PM Oliver Kiddle <opk@zsh.org> wrote:
>
> This particular test was rather annoying before $functions_source
> existed because it made it more difficult to fully reload all _git
> functions.

I can't tell if you're arguing for simply removing the test, or for
exploding all the functions into their own files (which I think would
necessitate another layer of subdirectory, just for sanity).


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

* Re: [PATCH] Remove redundancies from `git` completion
  2021-09-02 20:54                 ` Bart Schaefer
@ 2021-09-02 21:17                   ` Oliver Kiddle
  0 siblings, 0 replies; 13+ messages in thread
From: Oliver Kiddle @ 2021-09-02 21:17 UTC (permalink / raw)
  To: Zsh hackers list

Bart Schaefer wrote:
> On Thu, Sep 2, 2021 at 1:51 PM Oliver Kiddle <opk@zsh.org> wrote:
> >
> > This particular test was rather annoying before $functions_source
> > existed because it made it more difficult to fully reload all _git
> > functions.
>
> I can't tell if you're arguing for simply removing the test, or for
> exploding all the functions into their own files (which I think would
> necessitate another layer of subdirectory, just for sanity).

For the sake of sanity (and consistency), I'd keep it in the single
file.

I wouldn't object to removing the tests, either wholesale or selectively.
But I'm not inclined to be proactive about removing them either.

Oliver


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

end of thread, other threads:[~2021-09-02 21:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-30 11:17 [PATCH] Remove redundancies from `git` completion Marlon Richert
2021-08-30 11:41 ` Mikael Magnusson
2021-08-30 12:34   ` Marlon Richert
2021-08-30 12:42     ` Mikael Magnusson
2021-08-31  5:52       ` Marlon Richert
2021-09-01 14:03         ` Daniel Shahaf
2021-09-01 18:22           ` Bart Schaefer
2021-09-02  6:17             ` Marlon Richert
2021-09-02 20:51               ` Oliver Kiddle
2021-09-02 20:54                 ` Bart Schaefer
2021-09-02 21:17                   ` Oliver Kiddle
2021-09-01 13:56 ` Daniel Shahaf
2021-09-02  8:57   ` Marlon Richert

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).