From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon, 18 May 2015 14:37:17 GMT
Subject: Re: [PATCH 1/2] pull: handle git-fetch's options as well
Message-ID: <1a102d8dabff79207c0d7e3bf434bd6f@www.dscho.org>
In-Reply-To: <1431955855-16228-1-git-send-email-pyokagan@gmail.com>

Hi Paul,

On 2015-05-18 15:30, Paul Tan wrote:
> t5520: failing test for pull --all with no configured upstream
> t5521: test pull --all --dry-run does not make any changes
> 
> error_on_no_merge_candidates() does not consider the case where "$#"
> includes command-line flags that are passed to git-fetch.
> 
> As such, when the current branch has no configured upstream, and there
> are no merge candidates because of that, git-pull --all erroneously reports
> that we are pulling from "--all", as it believes that the first argument
> is the remote name.
> 
> Add a failing test that shows this case.
> 
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>  git-pull.sh             | 34 ++++++++++++++++++++++++++++++++--
>  t/t5520-pull.sh         | 20 ++++++++++++++++++++
>  t/t5521-pull-options.sh | 14 ++++++++++++++
>  3 files changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/git-pull.sh b/git-pull.sh
> index 9ed01fd..28d49ab 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -163,11 +163,39 @@ do
>  	--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
>  		dry_run=--dry-run
>  		;;
> +	--all|--no-all)
> +		all=$1 ;;

I *think* you also want to add a corresponding "all=" line just below the "dry_run=" line, to ensure that "all=blablabla git pull" does not interfere with this command-line setting.

The patch looks fine to me, otherwise.

Ciao,
Dscho