Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Quoting Junio C Hamano <gitster@xxxxxxxxx>:

Jeff King <peff@xxxxxxxx> writes:

On Thu, Feb 04, 2016 at 11:34:59AM +0100, Sebastian Schuberth wrote:

This avoids output like

    warning: ignoring broken ref refs/remotes/origin/HEAD

while completing branch names.

Hmm. I feel like this case (HEAD points to a branch, then `fetch
--prune` deletes it) came up recently and we discussed quieting that
warning. But now I cannot seem to find it.

Anyway, I this is a reasonable workaround. Errors from bash completion
scripts are almost always going to be useless and get in the way of
reading your own prompt.

I think that is absolutely the right stance to take, but then I
wonder if it is a sensible execution to sprinkle 2>/dev/null
everywhere.

For example, couldn't we do something like this instead?

This is just for illustration and does not remove all 2>/dev/null
and replace them with a single redirection that covers the entire
shell function body, but something along this line smells a lot more
pleasant.  I dunno.

Please no :)

First, we don't have to redirect stderr of every completion function,
it's sufficient to do so only for the two "main" entry point functions
__git_main() and __gitk_main().

But:

 * It would swallow even those errors that we are interested in,
   e.g. (note the missing quotes around $foo):

      $ func () { if [ $foo = y ] ; then echo "foo is y" ; fi ; }
      $ foo=
      $ func 2>/dev/null
      $ func
      bash: [: =: unary operator expected

   Something like this should not happen, it's a bug in the
   completion script that should be fixed, and we should get a bug
   report.

 * I often find myself tracing/debugging the completion script
   through stderr by scattering

      echo >&2 "foo: '$foo'"

   and the like all over the place.  If completion functions' stderr
   were redirected, then I would have to disable that redirection
   first to be able do this kind of poor man's tracing.

 * I have a WIP patch series that deals with errors from git
   commands.
   It's a mixed bag of __gitdir()-related cleanups, fixes and
   optimizations, which factors out all git executions into a
   __git() wrapper function and redirects stderr only in that
   function, thereby eliminating most of the 2>/dev/null
   redirections in the completion script.
   It still needs some work to iron out a wrinkle or two around
   corner cases, though.



 contrib/completion/git-completion.bash | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ba4137d..637c42d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -47,14 +47,14 @@ __gitdir ()
 		elif [ -d .git ]; then
 			echo .git
 		else
-			git rev-parse --git-dir 2>/dev/null
+			git rev-parse --git-dir
 		fi
 	elif [ -d "$1/.git" ]; then
 		echo "$1/.git"
 	else
 		echo "$1"
 	fi
-}
+} 2>/dev/null

 # The following function is based on code from:
 #
@@ -320,7 +320,7 @@ __git_heads ()
 			refs/heads
 		return
 	fi
-}
+} 2>/dev/null

 __git_tags ()
 {
@@ -330,7 +330,7 @@ __git_tags ()
 			refs/tags
 		return
 	fi
-}
+} 2>/dev/null

 # __git_refs accepts 0, 1 (to pass to __gitdir), or 2 arguments
 # presence of 2nd argument means use the guess heuristic employed
@@ -389,7 +389,7 @@ __git_refs ()
 			"refs/remotes/$dir/" 2>/dev/null | sed -e "s#^$dir/##"
 		;;
 	esac
-}
+} 2>/dev/null

 # __git_refs2 requires 1 argument (to pass to __git_refs)
 __git_refs2 ()


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]