* [PATCH] completion: remote set-* <name> and <branch> @ 2012-02-18 13:32 Philip Jägenstedt 2012-02-20 7:58 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Philip Jägenstedt @ 2012-02-18 13:32 UTC (permalink / raw) To: Shawn O. Pearce; +Cc: git, Philip Jägenstedt Complete <name> only for set-url. For set-branches and set-head, complete <name> and <branch> over the network, like e.g. git pull already does. Signed-off-by: Philip Jägenstedt <philip@foolip.org> --- contrib/completion/git-completion.bash | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 1505cff..8e7abb6 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -738,6 +738,9 @@ __git_complete_remote_or_refspec () { local cur_="$cur" cmd="${words[1]}" local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0 + if [ "$cmd" = "remote" ]; then + c=$((++c)) + fi while [ $c -lt $cword ]; do i="${words[c]}" case "$i" in @@ -788,7 +791,7 @@ __git_complete_remote_or_refspec () __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_" fi ;; - pull) + pull|remote) if [ $lhs = 1 ]; then __gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_" else @@ -2289,7 +2292,7 @@ _git_config () _git_remote () { - local subcommands="add rename rm show prune update set-head" + local subcommands="add rename rm set-head set-branches set-url show prune update" local subcommand="$(__git_find_on_cmdline "$subcommands")" if [ -z "$subcommand" ]; then __gitcomp "$subcommands" @@ -2297,9 +2300,12 @@ _git_remote () fi case "$subcommand" in - rename|rm|show|prune) + rename|rm|set-url|show|prune) __gitcomp_nl "$(__git_remotes)" ;; + set-head|set-branches) + __git_complete_remote_or_refspec + ;; update) local i c='' IFS=$'\n' for i in $(git --git-dir="$(__gitdir)" config --get-regexp "remotes\..*" 2>/dev/null); do -- 1.7.9.1.245.g4cbe6 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] completion: remote set-* <name> and <branch> 2012-02-18 13:32 [PATCH] completion: remote set-* <name> and <branch> Philip Jägenstedt @ 2012-02-20 7:58 ` Junio C Hamano 2012-02-21 21:29 ` Philip Jägenstedt 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2012-02-20 7:58 UTC (permalink / raw) To: Philip Jägenstedt Cc: git, SZEDER Gábor, Felipe Contreras, Teemu Likonen Philip Jägenstedt <philip@foolip.org> writes: > Complete <name> only for set-url. For set-branches and > set-head, complete <name> and <branch> over the network, > like e.g. git pull already does. > > Signed-off-by: Philip Jägenstedt <philip@foolip.org> You addressed your patch to Shawn, who originally wrote this, but "git shortlog -n -s --no-merges --since=9.months pu contrib/completion" indicates that he no longer is involved in enhancing this script, and it has seen actions primarily from three people I Cc'ed this message to. > contrib/completion/git-completion.bash | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 1505cff..8e7abb6 100755 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -738,6 +738,9 @@ __git_complete_remote_or_refspec () > { > local cur_="$cur" cmd="${words[1]}" > local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0 > + if [ "$cmd" = "remote" ]; then > + c=$((++c)) > + fi I don't know about others, but auto-incrementing a variable and assigning the result to the same variable, while not wrong at all, hurts my brain. c=$(($c + 1)) is far more readable and does not suggest there is any funky magic involved. Also it is a good habit to get into not to omit $ from variables inside arithmetic substitution, even though bash allows it and this script is meant to be consumed only by shells that understand this bash-ism. I do not know offhand if zsh groks it, but the point is that you do not have to worry about it if you write "$(($c+1))" instead of "$((c+1))". I'll let the area experts to comment on the remainder of the patch. Thanks. > @@ -788,7 +791,7 @@ __git_complete_remote_or_refspec () > __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_" > fi > ;; > - pull) > + pull|remote) > if [ $lhs = 1 ]; then > __gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_" > else > @@ -2289,7 +2292,7 @@ _git_config () > > _git_remote () > { > - local subcommands="add rename rm show prune update set-head" > + local subcommands="add rename rm set-head set-branches set-url show prune update" > local subcommand="$(__git_find_on_cmdline "$subcommands")" > if [ -z "$subcommand" ]; then > __gitcomp "$subcommands" > @@ -2297,9 +2300,12 @@ _git_remote () > fi > > case "$subcommand" in > - rename|rm|show|prune) > + rename|rm|set-url|show|prune) > __gitcomp_nl "$(__git_remotes)" > ;; > + set-head|set-branches) > + __git_complete_remote_or_refspec > + ;; > update) > local i c='' IFS=$'\n' > for i in $(git --git-dir="$(__gitdir)" config --get-regexp "remotes\..*" 2>/dev/null); do ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] completion: remote set-* <name> and <branch> 2012-02-20 7:58 ` Junio C Hamano @ 2012-02-21 21:29 ` Philip Jägenstedt 2012-02-21 21:54 ` [PATCH v2] " Philip Jägenstedt 2012-02-21 22:23 ` [PATCH] " Junio C Hamano 0 siblings, 2 replies; 14+ messages in thread From: Philip Jägenstedt @ 2012-02-21 21:29 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, SZEDER Gábor, Felipe Contreras, Teemu Likonen On Mon, Feb 20, 2012 at 08:58, Junio C Hamano <gitster@pobox.com> wrote: > Philip Jägenstedt <philip@foolip.org> writes: > >> Complete <name> only for set-url. For set-branches and >> set-head, complete <name> and <branch> over the network, >> like e.g. git pull already does. >> >> Signed-off-by: Philip Jägenstedt <philip@foolip.org> > > You addressed your patch to Shawn, who originally wrote this, but > > "git shortlog -n -s --no-merges --since=9.months pu contrib/completion" > > indicates that he no longer is involved in enhancing this script, and it > has seen actions primarily from three people I Cc'ed this message to. Thanks. Perhaps git-completion.bash should not say "Send all patches to the current maintainer" and simply defer to SubmittingPatches? >> contrib/completion/git-completion.bash | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash >> index 1505cff..8e7abb6 100755 >> --- a/contrib/completion/git-completion.bash >> +++ b/contrib/completion/git-completion.bash >> @@ -738,6 +738,9 @@ __git_complete_remote_or_refspec () >> { >> local cur_="$cur" cmd="${words[1]}" >> local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0 >> + if [ "$cmd" = "remote" ]; then >> + c=$((++c)) >> + fi > > I don't know about others, but auto-incrementing a variable and assigning > the result to the same variable, while not wrong at all, hurts my brain. > > c=$(($c + 1)) > > is far more readable and does not suggest there is any funky magic > involved. Also it is a good habit to get into not to omit $ from > variables inside arithmetic substitution, even though bash allows it and > this script is meant to be consumed only by shells that understand this > bash-ism. > > I do not know offhand if zsh groks it, but the point is that you do not > have to worry about it if you write "$(($c+1))" instead of "$((c+1))". CodingGuidlines suggests to follow local convention, which was "c=$((++c))". This file also uses "++n", "i++" and "((i++))". I will send a v2 patch that normalizes these, open to discussion of course. -- Philip Jägenstedt ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] completion: remote set-* <name> and <branch> 2012-02-21 21:29 ` Philip Jägenstedt @ 2012-02-21 21:54 ` Philip Jägenstedt 2012-02-22 0:17 ` SZEDER Gábor 2012-02-21 22:23 ` [PATCH] " Junio C Hamano 1 sibling, 1 reply; 14+ messages in thread From: Philip Jägenstedt @ 2012-02-21 21:54 UTC (permalink / raw) To: git Cc: SZEDER Gábor, Felipe Contreras, Teemu Likonen, Philip Jägenstedt Complete <name> only for set-url. For set-branches and set-head, complete <name> and <branch> over the network, like e.g. git pull already does. The style used for incrementing and decrementing variables was fairly inconsistenty and was normalized to use ++x, or ((++x)) in contexts where the former would otherwise be interpreted as a command. This is a bash-ism, but for obvious reasons this script is already bash-specific. Finally, remove out-of-date documentation for how to submit patches and (silently) defer to Documentation/SubmittingPatches like all other code. Signed-off-by: Philip Jägenstedt <philip@foolip.org> --- contrib/completion/git-completion.bash | 42 +++++++++++++------------------ 1 files changed, 18 insertions(+), 24 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 1505cff..9dac084 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -60,18 +60,6 @@ # per-repository basis by setting the bash.showUpstream config # variable. # -# -# To submit patches: -# -# *) Read Documentation/SubmittingPatches -# *) Send all patches to the current maintainer: -# -# "Shawn O. Pearce" <spearce@spearce.org> -# -# *) Always CC the Git mailing list: -# -# git@vger.kernel.org -# if [[ -n ${ZSH_VERSION-} ]]; then autoload -U +X bashcompinit && bashcompinit @@ -395,7 +383,7 @@ __git_reassemble_comp_words_by_ref() fi # List of word completion separators has shrunk; # re-assemble words to complete. - for ((i=0, j=0; i < ${#COMP_WORDS[@]}; i++, j++)); do + for ((i=0, j=0; i < ${#COMP_WORDS[@]}; ++i, ++j)); do # Append each nonempty word consisting of just # word separator characters to the current word. first=t @@ -408,7 +396,7 @@ __git_reassemble_comp_words_by_ref() # Attach to the previous token, # unless the previous token is the command name. if [ $j -ge 2 ] && [ -n "$first" ]; then - ((j--)) + ((--j)) fi first= words_[$j]=${words_[j]}${COMP_WORDS[i]} @@ -416,7 +404,7 @@ __git_reassemble_comp_words_by_ref() cword_=$j fi if (($i < ${#COMP_WORDS[@]} - 1)); then - ((i++)) + ((++i)) else # Done. return @@ -738,6 +726,9 @@ __git_complete_remote_or_refspec () { local cur_="$cur" cmd="${words[1]}" local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0 + if [ "$cmd" = "remote" ]; then + ((++c)) + fi while [ $c -lt $cword ]; do i="${words[c]}" case "$i" in @@ -755,7 +746,7 @@ __git_complete_remote_or_refspec () -*) ;; *) remote="$i"; break ;; esac - c=$((++c)) + ((++c)) done if [ -z "$remote" ]; then __gitcomp_nl "$(__git_remotes)" @@ -788,7 +779,7 @@ __git_complete_remote_or_refspec () __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_" fi ;; - pull) + pull|remote) if [ $lhs = 1 ]; then __gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_" else @@ -995,7 +986,7 @@ __git_find_on_cmdline () return fi done - c=$((++c)) + ((++c)) done } @@ -1006,7 +997,7 @@ __git_has_doubledash () if [ "--" = "${words[c]}" ]; then return 0 fi - c=$((++c)) + ((++c)) done return 1 } @@ -1129,7 +1120,7 @@ _git_branch () -d|-m) only_local_ref="y" ;; -r) has_r="y" ;; esac - c=$((++c)) + ((++c)) done case "$cur" in @@ -2289,7 +2280,7 @@ _git_config () _git_remote () { - local subcommands="add rename rm show prune update set-head" + local subcommands="add rename rm set-head set-branches set-url show prune update" local subcommand="$(__git_find_on_cmdline "$subcommands")" if [ -z "$subcommand" ]; then __gitcomp "$subcommands" @@ -2297,9 +2288,12 @@ _git_remote () fi case "$subcommand" in - rename|rm|show|prune) + rename|rm|set-url|show|prune) __gitcomp_nl "$(__git_remotes)" ;; + set-head|set-branches) + __git_complete_remote_or_refspec + ;; update) local i c='' IFS=$'\n' for i in $(git --git-dir="$(__gitdir)" config --get-regexp "remotes\..*" 2>/dev/null); do @@ -2580,7 +2574,7 @@ _git_tag () f=1 ;; esac - c=$((++c)) + ((++c)) done case "$prev" in @@ -2633,7 +2627,7 @@ _git () --help) command="help"; break ;; *) command="$i"; break ;; esac - c=$((++c)) + ((++c)) done if [ -z "$command" ]; then -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] completion: remote set-* <name> and <branch> 2012-02-21 21:54 ` [PATCH v2] " Philip Jägenstedt @ 2012-02-22 0:17 ` SZEDER Gábor 2012-02-22 8:58 ` [PATCH v3] " Philip Jägenstedt 0 siblings, 1 reply; 14+ messages in thread From: SZEDER Gábor @ 2012-02-22 0:17 UTC (permalink / raw) To: Philip Jägenstedt; +Cc: git, Felipe Contreras, Teemu Likonen Hi, On Tue, Feb 21, 2012 at 10:54:34PM +0100, Philip Jägenstedt wrote: > Complete <name> only for set-url. For set-branches and > set-head, complete <name> and <branch> over the network, > like e.g. git pull already does. > > The style used for incrementing and decrementing variables was fairly > inconsistenty and was normalized to use ++x, or ((++x)) in contexts > where the former would otherwise be interpreted as a command. This is a > bash-ism, but for obvious reasons this script is already bash-specific. > > Finally, remove out-of-date documentation for how to submit patches and > (silently) defer to Documentation/SubmittingPatches like all other code. > > Signed-off-by: Philip Jägenstedt <philip@foolip.org> > --- > contrib/completion/git-completion.bash | 42 +++++++++++++------------------ > 1 files changed, 18 insertions(+), 24 deletions(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 1505cff..9dac084 100755 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -60,18 +60,6 @@ > # per-repository basis by setting the bash.showUpstream config > # variable. > # > -# > -# To submit patches: > -# > -# *) Read Documentation/SubmittingPatches Um, well... Did you actually read it? I mean the part that talks about making separate commits for logically separate changes? ;) You clearly squashed three separate changes into a single patch. Your proposed changes to __git_complete_remote_or_refspec() and __git_remote() make sense, modulo the increment/decrement part. I don't have strong preference either way, but please submit those changes in separate patches. Otherwise if ever bisect points to this commit during a hunt for a 'git remote' completion bug, we'll need to think about whether the increment style change or the functional change is causing the trouble. Thanks, Gábor ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3] completion: remote set-* <name> and <branch> 2012-02-22 0:17 ` SZEDER Gábor @ 2012-02-22 8:58 ` Philip Jägenstedt 2012-02-22 8:58 ` [PATCH 1/4] " Philip Jägenstedt ` (4 more replies) 0 siblings, 5 replies; 14+ messages in thread From: Philip Jägenstedt @ 2012-02-22 8:58 UTC (permalink / raw) To: git; +Cc: SZEDER Gábor, Felipe Contreras, Teemu Likonen Hope this works better. Is it possible to use format-patch or send-email to get subjects like [PATCH 1/4 v3], as opposed to what I am sending? ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] completion: remote set-* <name> and <branch> 2012-02-22 8:58 ` [PATCH v3] " Philip Jägenstedt @ 2012-02-22 8:58 ` Philip Jägenstedt 2012-02-22 8:58 ` [PATCH 2/4] completion: normalize increment/decrement style Philip Jägenstedt ` (3 subsequent siblings) 4 siblings, 0 replies; 14+ messages in thread From: Philip Jägenstedt @ 2012-02-22 8:58 UTC (permalink / raw) To: git Cc: SZEDER Gábor, Felipe Contreras, Teemu Likonen, Philip Jägenstedt Complete <name> only for set-url. For set-branches and set-head, complete <name> and <branch> over the network, like e.g. git pull already does. Signed-off-by: Philip Jägenstedt <philip@foolip.org> --- contrib/completion/git-completion.bash | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index ab24310..c63a408 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -738,6 +738,9 @@ __git_complete_remote_or_refspec () { local cur_="$cur" cmd="${words[1]}" local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0 + if [ "$cmd" = "remote" ]; then + c=$((++c)) + fi while [ $c -lt $cword ]; do i="${words[c]}" case "$i" in @@ -788,7 +791,7 @@ __git_complete_remote_or_refspec () __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_" fi ;; - pull) + pull|remote) if [ $lhs = 1 ]; then __gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_" else @@ -2289,7 +2292,7 @@ _git_config () _git_remote () { - local subcommands="add rename rm show prune update set-head" + local subcommands="add rename rm set-head set-branches set-url show prune update" local subcommand="$(__git_find_on_cmdline "$subcommands")" if [ -z "$subcommand" ]; then __gitcomp "$subcommands" @@ -2297,9 +2300,12 @@ _git_remote () fi case "$subcommand" in - rename|rm|show|prune) + rename|rm|set-url|show|prune) __gitcomp_nl "$(__git_remotes)" ;; + set-head|set-branches) + __git_complete_remote_or_refspec + ;; update) local i c='' IFS=$'\n' for i in $(git --git-dir="$(__gitdir)" config --get-regexp "remotes\..*" 2>/dev/null); do -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] completion: normalize increment/decrement style 2012-02-22 8:58 ` [PATCH v3] " Philip Jägenstedt 2012-02-22 8:58 ` [PATCH 1/4] " Philip Jägenstedt @ 2012-02-22 8:58 ` Philip Jägenstedt 2012-02-22 22:05 ` Junio C Hamano 2012-02-22 8:58 ` [PATCH 3/4] completion: remove stale "to submit patches" documentation Philip Jägenstedt ` (2 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: Philip Jägenstedt @ 2012-02-22 8:58 UTC (permalink / raw) To: git Cc: SZEDER Gábor, Felipe Contreras, Teemu Likonen, Philip Jägenstedt The style used for incrementing and decrementing variables was fairly inconsistenty and was normalized to use x++, or ((x++)) in contexts where the former would otherwise be interpreted as a command. This is a bash-ism, but for obvious reasons this script is already bash-specific. Signed-off-by: Philip Jägenstedt <philip@foolip.org> --- contrib/completion/git-completion.bash | 22 ++++++++++------------ 1 files changed, 10 insertions(+), 12 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index c63a408..1903bc9 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -149,7 +149,7 @@ __git_ps1_show_upstream () svn_upstream=${svn_upstream[ ${#svn_upstream[@]} - 2 ]} svn_upstream=${svn_upstream%@*} local n_stop="${#svn_remote[@]}" - for ((n=1; n <= n_stop; ++n)); do + for ((n=1; n <= n_stop; n++)); do svn_upstream=${svn_upstream#${svn_remote[$n]}} done @@ -178,10 +178,8 @@ __git_ps1_show_upstream () for commit in $commits do case "$commit" in - "<"*) let ++behind - ;; - *) let ++ahead - ;; + "<"*) ((behind++)) ;; + *) ((ahead++)) ;; esac done count="$behind $ahead" @@ -739,7 +737,7 @@ __git_complete_remote_or_refspec () local cur_="$cur" cmd="${words[1]}" local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0 if [ "$cmd" = "remote" ]; then - c=$((++c)) + ((c++)) fi while [ $c -lt $cword ]; do i="${words[c]}" @@ -758,7 +756,7 @@ __git_complete_remote_or_refspec () -*) ;; *) remote="$i"; break ;; esac - c=$((++c)) + ((c++)) done if [ -z "$remote" ]; then __gitcomp_nl "$(__git_remotes)" @@ -998,7 +996,7 @@ __git_find_on_cmdline () return fi done - c=$((++c)) + ((c++)) done } @@ -1009,7 +1007,7 @@ __git_has_doubledash () if [ "--" = "${words[c]}" ]; then return 0 fi - c=$((++c)) + ((c++)) done return 1 } @@ -1132,7 +1130,7 @@ _git_branch () -d|-m) only_local_ref="y" ;; -r) has_r="y" ;; esac - c=$((++c)) + ((c++)) done case "$cur" in @@ -2586,7 +2584,7 @@ _git_tag () f=1 ;; esac - c=$((++c)) + ((c++)) done case "$prev" in @@ -2639,7 +2637,7 @@ _git () --help) command="help"; break ;; *) command="$i"; break ;; esac - c=$((++c)) + ((c++)) done if [ -z "$command" ]; then -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] completion: normalize increment/decrement style 2012-02-22 8:58 ` [PATCH 2/4] completion: normalize increment/decrement style Philip Jägenstedt @ 2012-02-22 22:05 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2012-02-22 22:05 UTC (permalink / raw) To: Philip Jägenstedt Cc: git, SZEDER Gábor, Felipe Contreras, Teemu Likonen Philip Jägenstedt <philip@foolip.org> writes: > The style used for incrementing and decrementing variables was fairly > inconsistenty and was normalized to use x++, or ((x++)) in contexts > where the former would otherwise be interpreted as a command. This is a > bash-ism, but for obvious reasons this script is already bash-specific. > > Signed-off-by: Philip Jägenstedt <philip@foolip.org> I am not sure if the kind of changes seen in the following hunks are necessary but I do not care too deeply about it. Assuming that the change does not make the emulation by zsh unhappy, will apply. Please stop me if somebody has issues with this rewrite. Thanks. > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index c63a408..1903bc9 100755 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -178,10 +178,8 @@ __git_ps1_show_upstream () > for commit in $commits > do > case "$commit" in > - "<"*) let ++behind > - ;; > - *) let ++ahead > - ;; > + "<"*) ((behind++)) ;; > + *) ((ahead++)) ;; > esac > done > count="$behind $ahead" > @@ -739,7 +737,7 @@ __git_complete_remote_or_refspec () > local cur_="$cur" cmd="${words[1]}" > local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0 > if [ "$cmd" = "remote" ]; then > - c=$((++c)) > + ((c++)) > fi > while [ $c -lt $cword ]; do > i="${words[c]}" ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] completion: remove stale "to submit patches" documentation 2012-02-22 8:58 ` [PATCH v3] " Philip Jägenstedt 2012-02-22 8:58 ` [PATCH 1/4] " Philip Jägenstedt 2012-02-22 8:58 ` [PATCH 2/4] completion: normalize increment/decrement style Philip Jägenstedt @ 2012-02-22 8:58 ` Philip Jägenstedt 2012-02-22 8:58 ` [PATCH 4/4] completion: use tabs for indentation Philip Jägenstedt 2012-02-22 9:52 ` [PATCH v3] completion: remote set-* <name> and <branch> Thomas Rast 4 siblings, 0 replies; 14+ messages in thread From: Philip Jägenstedt @ 2012-02-22 8:58 UTC (permalink / raw) To: git Cc: SZEDER Gábor, Felipe Contreras, Teemu Likonen, Philip Jägenstedt It was out-of-sync with the reality of who works on this script. Defer (silently) to Documentation/SubmittingPatches like all other code. Signed-off-by: Philip Jägenstedt <philip@foolip.org> --- contrib/completion/git-completion.bash | 12 ------------ 1 files changed, 0 insertions(+), 12 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 1903bc9..48237c8 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -60,18 +60,6 @@ # per-repository basis by setting the bash.showUpstream config # variable. # -# -# To submit patches: -# -# *) Read Documentation/SubmittingPatches -# *) Send all patches to the current maintainer: -# -# "Shawn O. Pearce" <spearce@spearce.org> -# -# *) Always CC the Git mailing list: -# -# git@vger.kernel.org -# if [[ -n ${ZSH_VERSION-} ]]; then autoload -U +X bashcompinit && bashcompinit -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/4] completion: use tabs for indentation 2012-02-22 8:58 ` [PATCH v3] " Philip Jägenstedt ` (2 preceding siblings ...) 2012-02-22 8:58 ` [PATCH 3/4] completion: remove stale "to submit patches" documentation Philip Jägenstedt @ 2012-02-22 8:58 ` Philip Jägenstedt 2012-02-22 9:52 ` [PATCH v3] completion: remote set-* <name> and <branch> Thomas Rast 4 siblings, 0 replies; 14+ messages in thread From: Philip Jägenstedt @ 2012-02-22 8:58 UTC (permalink / raw) To: git Cc: SZEDER Gábor, Felipe Contreras, Teemu Likonen, Philip Jägenstedt CodingGuidlines confidently declares "We use tabs for indentation." It would be a shame if it were caught lying. Signed-off-by: Philip Jägenstedt <philip@foolip.org> --- contrib/completion/git-completion.bash | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 48237c8..03f0b8c 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -284,13 +284,13 @@ __git_ps1 () fi fi if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then - git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$" + git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$" fi if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then - if [ -n "$(git ls-files --others --exclude-standard)" ]; then - u="%" - fi + if [ -n "$(git ls-files --others --exclude-standard)" ]; then + u="%" + fi fi if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3] completion: remote set-* <name> and <branch> 2012-02-22 8:58 ` [PATCH v3] " Philip Jägenstedt ` (3 preceding siblings ...) 2012-02-22 8:58 ` [PATCH 4/4] completion: use tabs for indentation Philip Jägenstedt @ 2012-02-22 9:52 ` Thomas Rast 4 siblings, 0 replies; 14+ messages in thread From: Thomas Rast @ 2012-02-22 9:52 UTC (permalink / raw) To: Philip Jägenstedt Cc: git, SZEDER Gábor, Felipe Contreras, Teemu Likonen Philip Jägenstedt <philip@foolip.org> writes: > Hope this works better. Is it possible to use format-patch or send-email > to get subjects like [PATCH 1/4 v3], as opposed to what I am sending? 'git format-patch --subject-prefix="PATCH v3"' gives you [PATCH v3 1/4] which I think is what everyone does around here... -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] completion: remote set-* <name> and <branch> 2012-02-21 21:29 ` Philip Jägenstedt 2012-02-21 21:54 ` [PATCH v2] " Philip Jägenstedt @ 2012-02-21 22:23 ` Junio C Hamano 2012-02-22 0:03 ` SZEDER Gábor 1 sibling, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2012-02-21 22:23 UTC (permalink / raw) To: Philip Jägenstedt Cc: git, SZEDER Gábor, Felipe Contreras, Teemu Likonen Philip Jägenstedt <philip@foolip.org> writes: > Thanks. Perhaps git-completion.bash should not say "Send all patches > to the current maintainer" and simply defer to SubmittingPatches? I see you did this in your follow-up patch. Thanks. >>> { >>> local cur_="$cur" cmd="${words[1]}" >>> local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0 >>> + if [ "$cmd" = "remote" ]; then >>> + c=$((++c)) >>> + fi >> >> I don't know about others, but auto-incrementing a variable and assigning >> the result to the same variable, while not wrong at all, hurts my brain. >> >> c=$(($c + 1)) >> >> is far more readable and does not suggest there is any funky magic >> involved. Also it is a good habit to get into not to omit $ from >> variables inside arithmetic substitution, even though bash allows it and >> this script is meant to be consumed only by shells that understand this >> bash-ism. >> >> I do not know offhand if zsh groks it, but the point is that you do not >> have to worry about it if you write "$(($c+1))" instead of "$((c+1))". > > CodingGuidlines suggests to follow local convention, which was > "c=$((++c))". This file also uses "++n", "i++" and "((i++))". I will > send a v2 patch that normalizes these, open to discussion of course. OK, it was my mistake to suggest $c++ in this file; it liberally uses dollar-less variables, and I agree that it is a good idea to stick to that local convention. But I think you went too far in your follow-up patch to make the increment and decrement uniform. "i++" is so much easier on the eye unless you must use "++i" in order to use the value of the incremented "i" in an expression, and the changes to turn existing instances of free-standing "i++" to "++i" done only for the side effect of incrementing the variables look totally backwards. Although I do not deeply care. Just leaving the new one as you originally wrote, i.e. c=$((++c)) would have been easier to review for the area experts, I would think. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] completion: remote set-* <name> and <branch> 2012-02-21 22:23 ` [PATCH] " Junio C Hamano @ 2012-02-22 0:03 ` SZEDER Gábor 0 siblings, 0 replies; 14+ messages in thread From: SZEDER Gábor @ 2012-02-22 0:03 UTC (permalink / raw) To: Junio C Hamano Cc: Philip Jägenstedt, git, SZEDER Gábor, Felipe Contreras, Teemu Likonen On Tue, Feb 21, 2012 at 02:23:11PM -0800, Junio C Hamano wrote: > >>> { > >>> local cur_="$cur" cmd="${words[1]}" > >>> local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0 > >>> + if [ "$cmd" = "remote" ]; then > >>> + c=$((++c)) > >>> + fi > >> > >> I don't know about others, but auto-incrementing a variable and assigning > >> the result to the same variable, while not wrong at all, hurts my brain. > >> > >> c=$(($c + 1)) > >> > >> is far more readable and does not suggest there is any funky magic > >> involved. Also it is a good habit to get into not to omit $ from > >> variables inside arithmetic substitution, even though bash allows it and > >> this script is meant to be consumed only by shells that understand this > >> bash-ism. > >> > >> I do not know offhand if zsh groks it, but the point is that you do not > >> have to worry about it if you write "$(($c+1))" instead of "$((c+1))". This c=$((++c)) style increment is used in several completion functions, even in the main entry function _git() itself. If zsh didn't grok it, then completion wouldn't work at all on zsh, because the while loop in _git() would be an endless loop. > > CodingGuidlines suggests to follow local convention, which was > > "c=$((++c))". This file also uses "++n", "i++" and "((i++))". I will > > send a v2 patch that normalizes these, open to discussion of course. > > OK, it was my mistake to suggest $c++ in this file; it liberally uses > dollar-less variables, and I agree that it is a good idea to stick to that > local convention. > > But I think you went too far in your follow-up patch to make the increment > and decrement uniform. > > "i++" is so much easier on the eye unless you must use "++i" in order to > use the value of the incremented "i" in an expression, and the changes to > turn existing instances of free-standing "i++" to "++i" done only for the > side effect of incrementing the variables look totally backwards. > > Although I do not deeply care. Just leaving the new one as you originally > wrote, i.e. > > c=$((++c)) > > would have been easier to review for the area experts, I would think. The issue of this weird increment came up before http://thread.gmane.org/gmane.comp.version-control.git/87650/focus=87806 and the conclusion back then was that we kept the existing style. But otherwise I agree, it hurts my brain, too. Best, Gábor ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-02-22 22:05 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-18 13:32 [PATCH] completion: remote set-* <name> and <branch> Philip Jägenstedt 2012-02-20 7:58 ` Junio C Hamano 2012-02-21 21:29 ` Philip Jägenstedt 2012-02-21 21:54 ` [PATCH v2] " Philip Jägenstedt 2012-02-22 0:17 ` SZEDER Gábor 2012-02-22 8:58 ` [PATCH v3] " Philip Jägenstedt 2012-02-22 8:58 ` [PATCH 1/4] " Philip Jägenstedt 2012-02-22 8:58 ` [PATCH 2/4] completion: normalize increment/decrement style Philip Jägenstedt 2012-02-22 22:05 ` Junio C Hamano 2012-02-22 8:58 ` [PATCH 3/4] completion: remove stale "to submit patches" documentation Philip Jägenstedt 2012-02-22 8:58 ` [PATCH 4/4] completion: use tabs for indentation Philip Jägenstedt 2012-02-22 9:52 ` [PATCH v3] completion: remote set-* <name> and <branch> Thomas Rast 2012-02-21 22:23 ` [PATCH] " Junio C Hamano 2012-02-22 0:03 ` SZEDER Gábor
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).