git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Manlio Perillo <manlio.perillo@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, szeder@ira.uka.de,
	felipe.contreras@gmail.com, peff@peff.net
Subject: Re: [PATCH v5] git-completion.bash: add support for path completion
Date: Sat, 12 Jan 2013 15:52:56 +0100	[thread overview]
Message-ID: <50F178C8.40806@gmail.com> (raw)
In-Reply-To: <7v8v7zbcoi.fsf@alter.siamese.dyndns.org>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 11/01/2013 23:02, Junio C Hamano ha scritto:
> Manlio Perillo <manlio.perillo@gmail.com> writes:
> 
>> +# Process path list returned by "ls-files" and "diff-index --name-only"
>> +# commands, in order to list only file names relative to a specified
>> +# directory, and append a slash to directory names.
>> +__git_index_file_list_filter ()
>> +{
>> +	# Default to Bash >= 4.x
>> +	__git_index_file_list_filter_bash
>> +}
>> +
>> +# Execute git ls-files, returning paths relative to the directory
>> +# specified in the first argument, and using the options specified in
>> +# the second argument.
>> +__git_ls_files_helper ()
>> +{
>> +	# NOTE: $2 is not quoted in order to support multiple options
>> +	cd "$1" && git ls-files --exclude-standard $2
>> +} 2>/dev/null
> 
> I think this redirection is correct but a bit tricky;

It's not tricky: it is POSIX:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_10


> it is in
> effect during the execution of the { block } (in other words, it is
> not about squelching errors during the function definition).
> 

What do you mean by "squelching"?

Note that I originally wrote the code as

__git_ls_files_helper ()
{
	# NOTE: $2 is not quoted in order to support multiple options
	 { cd "$1" && git ls-files --exclude-standard $2 } 2>/dev/null
}

but then I checked the POSIX standard, noting that it is redundant.

> -- >8 --
> #!/bin/sh
> cat >t.sh <<\EOF &&
> echo I am "$1"
> t () { echo "Goes to stdout"; echo >&2 "Goes to stderr"; } 2>/dev/null
> t
> for sh in bash dash ksh zsh
> do
> 	$sh t.sh $sh
> done
> -- 8< --
> 

There is a missing EOF delimiter.
And I'm not sure to understand the meaning of && after EOF.

> Bash does (so do dash and real AT&T ksh) grok this correctly, but
> zsh does not seem to (I tried zsh 4.3.10 and 4.3.17; also zsh
> pretending to be ksh gets this wrong as well).  Not that what ksh
> does matters, as it won't be dot-sourcing bash completion script.
>

I have added tcsh to the sh list, but it fails with:
Badly placed ()'s.


> It however may affect zsh, which does seem to dot-source this file.
> Perhaps zsh completion may have to be rewritten in a similar way as
> tcsh completion is done (i.e. does not dot-source this file but ask
> bash to do the heavy-lifting).
> 

Ok, I was wrong on assuming all modern shells were POSIX compliant.
I will change the code to use a nested {} group.

> This function seems to be always called in an subshell (e.g. as an
> upstream of a pipeline), so the "cd" may be harmless, but don't you
> need to disable CDPATH while doing this?
> 

I don't know.

> [..]


>> +# Try to count non option arguments passed on the command line for the
>> +# specified git command.
>> +# When options are used, it is necessary to use the special -- option to
>> +# tell the implementation were non option arguments begin.
>> +# XXX this can not be improved, since options can appear everywhere, as
>> +# an example:
>> +#	git mv x -n y
> 
> If that is the case, it is a bug in the command line parser, I
> think.  We should reject it, and the command line completer
> certainly should not encourage it.
> 

$ mkdir y
$ git mv x -n y
Checking rename of 'x' to 'y/x'
Renaming x to y/x
$ git status
# On branch master
nothing to commit, working directory clean

I was assuming it to be "normal", given how complex Git command line
parsing is (IMHO).


Thanks  Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDxeMgACgkQscQJ24LbaUTmaQCeMbZ0lRJxZIx3U31gMPmcqTLp
54sAmwYrjJVuvRYcsbGaMa3rb9/EKrBU
=ky30
-----END PGP SIGNATURE-----

  reply	other threads:[~2013-01-12 14:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-11 18:48 [PATCH v5] git-completion.bash: add support for path completion Manlio Perillo
2013-01-11 22:02 ` Junio C Hamano
2013-01-12 14:52   ` Manlio Perillo [this message]
2013-01-13 22:56     ` Junio C Hamano
2013-01-12 12:53 ` Manlio Perillo
2013-01-13 22:46   ` Junio C Hamano
2013-04-21 10:14 ` Felipe Contreras
2013-04-23 15:25   ` Manlio Perillo
2013-04-27  2:52     ` Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50F178C8.40806@gmail.com \
    --to=manlio.perillo@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=szeder@ira.uka.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).