zsh-workers
 help / color / mirror / code / Atom feed
* perforce completion patch
@ 2014-09-15 19:56 Anthony Heading
  2014-09-16 19:35 ` Peter Stephenson
  0 siblings, 1 reply; 3+ messages in thread
From: Anthony Heading @ 2014-09-15 19:56 UTC (permalink / raw)
  To: zsh-workers

Hi,

I've had the following small patch for perforce completion sitting
around for a while,  it
tried to fix e.g. the completion of "foo" when doing  "p4 add
foo/bar.cpp".   That said,
it runs against the goal of the surrounding comments which aim to skip
directories;  I
couldn't work out though why that would ever be desirable.

Anthony

--- functions/Completion/Unix/_perforce.Orig    2014-09-15
15:35:22.820229754 -0400
+++ functions/Completion/Unix/_perforce 2014-09-15 15:38:40.662453398
-0400
@@ -1253,7 +1253,7 @@

     [[ $#omitpats -eq 1 && $omitpats[1] = '' ]] && omitpats=()
     if (( ${#omitpats} )); then
-      _path_files -g "*~(*/|)(${(j:|:)~omitpats})(D.)"
+      _path_files -g "*~(*/|)(${(j:|:)~omitpats})(D)"
     else
       _path_files
     fi


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

* Re: perforce completion patch
  2014-09-15 19:56 perforce completion patch Anthony Heading
@ 2014-09-16 19:35 ` Peter Stephenson
  2014-09-23 10:14   ` Peter Stephenson
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Stephenson @ 2014-09-16 19:35 UTC (permalink / raw)
  To: zsh-workers

On Mon, 15 Sep 2014 15:56:27 -0400
Anthony Heading <anthony@ajrh.net> wrote:
> I've had the following small patch for perforce completion sitting
> around for a while,  it
> tried to fix e.g. the completion of "foo" when doing  "p4 add
> foo/bar.cpp".   That said,
> it runs against the goal of the surrounding comments which aim to skip
> directories;  I
> couldn't work out though why that would ever be desirable.

The line you've changed is handling the case of unmaintained files
requested (-tu => unmaintained=1) without directories being requested
(no -td => dodirs='').  This is what the p4 add completion currently does,
but it probably does need to complete directories.  p4 add is the only case
where -tu is passed in.  So it probably doesn't make sense to test for
-z $dodirs there.

What I can't remember and can't test at the moment without a lot of
fiddling is whether that means the code in that function is supposed to
work by adding unmaintained files and directories in some other branch
-- but I doubt it because that seems to be the only branch explicitly
checking for unmaintained files.

So it might simply be a case (as well as changing the glob) of removing
the spurious "&& -z $dodirs" (which is true because of the way the
caller works but confusing) and modifying the following comment to say
we always complete directories and hence ignore the -td flag in that
branch.

Of course this isn't going to be smart enough to add only directories
that contain unmaintained files but that fits with other uses of
directory completion which give you the opportunity of completing files
rather than the assurance they're there.

pws


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

* Re: perforce completion patch
  2014-09-16 19:35 ` Peter Stephenson
@ 2014-09-23 10:14   ` Peter Stephenson
  0 siblings, 0 replies; 3+ messages in thread
From: Peter Stephenson @ 2014-09-23 10:14 UTC (permalink / raw)
  To: zsh-workers

On Tue, 16 Sep 2014 20:35:25 +0100
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> On Mon, 15 Sep 2014 15:56:27 -0400
> Anthony Heading <anthony@ajrh.net> wrote:
> > I've had the following small patch for perforce completion sitting
> > around for a while,  it
> > tried to fix e.g. the completion of "foo" when doing  "p4 add
> > foo/bar.cpp".   That said,
> > it runs against the goal of the surrounding comments which aim to skip
> > directories;  I
> > couldn't work out though why that would ever be desirable.
>
> ... it might simply be a case (as well as changing the glob) of removing
> the spurious "&& -z $dodirs" (which is true because of the way the
> caller works but confusing) and modifying the following comment to say
> we always complete directories and hence ignore the -td flag in that
> branch.

This gives us the following, which shouldn't materially change the
behaviour of the original patch.

diff --git a/Completion/Unix/Command/_perforce b/Completion/Unix/Command/_perforce
index 8398702..db91e11 100644
--- a/Completion/Unix/Command/_perforce
+++ b/Completion/Unix/Command/_perforce
@@ -1231,10 +1231,15 @@ _perforce_files() {
 #      "subdirs:subdirectory search:_perforce_subdirs"
     )
     _alternative $altfiles
-  elif [[ -n $unmaintained && -z $dodirs ]]; then
-    # a la _cvs_nonentried_files: directories are never maintained,
-    # so skip 'em.  Unmaintained files can't be integrated, opened
-    # or resolved, so treat as exclusive (just as well, since
+  elif [[ -n $unmaintained ]]; then
+    # As directories are always umaintained, but may contain files
+    # we want to add, we'll always complete directories here.  That's
+    # neater than the alternative of excluding them here and requesting
+    # them separately in the caller.  The only client for this
+    # branch is currently 'p4 add'.
+    #
+    # Unmaintained files can't be integrated, opened
+    # or resolved, so treat as exclusive to other options (just as well, since
     # this bit's messy).
     local MATCH MBEGIN MEND
     local -a omitpats
@@ -1253,7 +1258,7 @@ _perforce_files() {
 
     [[ $#omitpats -eq 1 && $omitpats[1] = '' ]] && omitpats=()
     if (( ${#omitpats} )); then
-      _path_files -g "*~(*/|)(${(j:|:)~omitpats})(D.)"
+      _path_files -g "*~(*/|)(${(j:|:)~omitpats})(D)"
     else
       _path_files
     fi


pws


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

end of thread, other threads:[~2014-09-23 10:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-15 19:56 perforce completion patch Anthony Heading
2014-09-16 19:35 ` Peter Stephenson
2014-09-23 10:14   ` Peter Stephenson

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).