[PATCH 0/7] completion bash: add more options and commands

[PATCH 0/7] completion bash: add more options and commands

[bitte.keine.werbung.einwerfen]

From: Cornelius Weig <cornelius.weig@tngtech.com>

The cli interface for git has the greatest versatility of all git tools.
Its productivity depends on how easy it is to type commands and to remember
options. The command line completion helps two-fold by easing input and
displaying possible options.

Bash completion lacks support for several options and some useful commands.
Although the selection of recognized options is always opinionated (e.g. the
documentation for git-completion.bash says that 'common' long options are
recognized), I think that extending the cli completion will always increase the
productivity.

To that end, the following commits add completion support for several existing
options and subcommands. For every command all options that are mentioned in
the introduction of its man page should now be available for completion.

Cornelius Weig (7):
  completion: teach options to submodule subcommands
  completion: add subcommand completion for rerere
  completion: improve bash completion for git-add
  completion: teach ls-remote to complete options
  completion: teach replace to complete options
  completion: teach remote subcommands option completion
  completion: recognize more long-options

 contrib/completion/git-completion.bash | 132 +++++++++++++++++++++++++++------
 1 file changed, 110 insertions(+), 22 deletions(-)

-- 
2.10.2

[PATCH 1/7] completion: teach options to submodule subcommands

[bitte.keine.werbung.einwerfen]

From: Cornelius Weig <cornelius.weig@tngtech.com>

The subcommands of submodule have different long-options which command
line users need to type in. Therefore, teach bash completion to support
most subcommand options for submodule.
---
 contrib/completion/git-completion.bash | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6721ff8..c54a557 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2556,10 +2556,11 @@ _git_submodule ()
 	__git_has_doubledash && return
 
 	local subcommands="add status init deinit update summary foreach sync"
-	if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then
+	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	if [ -z "$subcommand" ]; then
 		case "$cur" in
 		--*)
-			__gitcomp "--quiet --cached"
+			__gitcomp "--quiet"
 			;;
 		*)
 			__gitcomp "$subcommands"
@@ -2567,6 +2568,33 @@ _git_submodule ()
 		esac
 		return
 	fi
+
+	case "$subcommand,$cur" in
+	add,--*)
+		__gitcomp "--branch --force --name --reference --depth"
+		;;
+	status,--*)
+		__gitcomp "--cached --recursive"
+		;;
+	deinit,--*)
+		__gitcomp "--force --all"
+		;;
+	update,--*)
+		__gitcomp "
+			--init --remote --no-fetch
+			--recommend-shallow --no-recommend-shallow
+			--force --rebase --merge --reference --depth --recursive --jobs
+		"
+		;;
+	summary,--*)
+		__gitcomp "--cached --files --summary-limit"
+		;;
+	foreach,--*|sync,--*)
+		__gitcomp "--recursive"
+		;;
+	*)
+		;;
+	esac
 }
 
 _git_svn ()
-- 
2.10.2

[PATCH 2/7] completion: add subcommand completion for rerere

[bitte.keine.werbung.einwerfen]

From: Cornelius Weig <cornelius.weig@tngtech.com>

Managing recorded resolutions requires command-line usage of git-rerere.
Added subcommand completion for rerere and path completion for its
subcommand forget.
---
 contrib/completion/git-completion.bash | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c54a557..8329f09 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2401,6 +2401,17 @@ _git_replace ()
 	__gitcomp_nl "$(__git_refs)"
 }
 
+_git_rerere ()
+{
+	local subcommands="clear forget diff remaining status gc"
+	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	if test -z "$subcommand"
+	then
+		__gitcomp "$subcommands"
+		return
+	fi
+}
+
 _git_reset ()
 {
 	__git_has_doubledash && return
-- 
2.10.2

Re: [PATCH 2/7] completion: add subcommand completion for rerere

[SZEDER Gábor]

> Managing recorded resolutions requires command-line usage of git-rerere.
> Added subcommand completion for rerere and path completion for its
> subcommand forget.
> ---
>  contrib/completion/git-completion.bash | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index c54a557..8329f09 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2401,6 +2401,17 @@ _git_replace ()
>  	__gitcomp_nl "$(__git_refs)"
>  }
>  
> +_git_rerere ()
> +{
> +	local subcommands="clear forget diff remaining status gc"
> +	local subcommand="$(__git_find_on_cmdline "$subcommands")"
> +	if test -z "$subcommand"
> +	then
> +		__gitcomp "$subcommands"
> +		return
> +	fi
> +}
> +
>  _git_reset ()
>  {
>  	__git_has_doubledash && return
> -- 
> 2.10.2

You didn't add 'rerere' to the list of porcelain commands, i.e. it
won't be listed after 'git <TAB><TAB>'.  I'm not saying it should be
listed there, because I can't decide whether 'rerere' is considered
porcelain or plumbing...  Just wanted to make sure that this omission
was intentional.

Re: [PATCH 2/7] completion: add subcommand completion for rerere

[Cornelius Weig]

On 02/02/2017 01:57 AM, SZEDER Gábor wrote:
> You didn't add 'rerere' to the list of porcelain commands, i.e. it
> won't be listed after 'git <TAB><TAB>'.  I'm not saying it should be
> listed there, because I can't decide whether 'rerere' is considered
> porcelain or plumbing...  Just wanted to make sure that this omission
> was intentional.

Yes this is intentional. There is a number of plumbing commands that
have command completion, but are not listed in 'git <Tab><Tab>' (e.g.
ls-tree, ls-files, ls-remote, ...). Given that rerere will not be
frequently invoked, I would not add it to the porcelain commands.

[PATCH 3/7] completion: improve bash completion for git-add

[bitte.keine.werbung.einwerfen]

From: Cornelius Weig <cornelius.weig@tngtech.com>

Add some long-options for git-add and improve path completion when the
--update flag is given.
---
 contrib/completion/git-completion.bash | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8329f09..652c7e2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -947,13 +947,17 @@ _git_add ()
 	--*)
 		__gitcomp "
 			--interactive --refresh --patch --update --dry-run
-			--ignore-errors --intent-to-add
+			--ignore-errors --intent-to-add --force --edit --chmod=
 			"
 		return
 	esac
 
-	# XXX should we check for --update and --all options ?
-	__git_complete_index_file "--others --modified --directory --no-empty-directory"
+	local complete_opt="--others --modified --directory --no-empty-directory"
+	if test -n "$(__git_find_on_cmdline "-u --update")"
+	then
+		complete_opt="--modified"
+	fi
+	__git_complete_index_file "$complete_opt"
 }
 
 _git_archive ()
-- 
2.10.2

[PATCH 4/7] completion: teach ls-remote to complete options

[bitte.keine.werbung.einwerfen]

From: Cornelius Weig <cornelius.weig@tngtech.com>

ls-remote needs to complete remote names and its own options. In
addition to the existing remote name completions, do also complete
the options --heads, --tags, --refs, and --get-url.
---
 contrib/completion/git-completion.bash | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 652c7e2..36fe439 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,6 +1449,12 @@ _git_ls_files ()
 
 _git_ls_remote ()
 {
+	case "$cur" in
+	--*)
+		__gitcomp "--heads --tags --refs --get-url"
+		return
+		;;
+	esac
 	__gitcomp_nl "$(__git_remotes)"
 }
 
-- 
2.10.2

Re: [PATCH 4/7] completion: teach ls-remote to complete options

[SZEDER Gábor]

> ls-remote needs to complete remote names and its own options.

And refnames, too.

> In
> addition to the existing remote name completions, do also complete
> the options --heads, --tags, --refs, and --get-url.

Why only these four options and not the other four?

There are eight options in total here, so there is really no chance
for cluttered terminals, and all eight are mentioned in the synopsis.

> ---
>  contrib/completion/git-completion.bash | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 652c7e2..36fe439 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1449,6 +1449,12 @@ _git_ls_files ()
>  
>  _git_ls_remote ()
>  {
> +	case "$cur" in
> +	--*)
> +		__gitcomp "--heads --tags --refs --get-url"
> +		return
> +		;;
> +	esac
>  	__gitcomp_nl "$(__git_remotes)"
>  }
>  
> -- 
> 2.10.2

Re: [PATCH 4/7] completion: teach ls-remote to complete options

[Cornelius Weig]

On 02/02/2017 02:40 AM, SZEDER Gábor wrote:
> 
>> ls-remote needs to complete remote names and its own options.
> 
> And refnames, too.

Yes, right. However, do you think it is reasonable to complete remote
refnames? I don't think so, because it would mean we would have to run
ls-remote during completion -- and waiting for ls-remote could be quite
lengthy.

>> In
>> addition to the existing remote name completions, do also complete
>> the options --heads, --tags, --refs, and --get-url.
> 
> Why only these four options and not the other four?
> 
> There are eight options in total here, so there is really no chance
> for cluttered terminals, and all eight are mentioned in the synopsis.

My line of thought is the following:

--quiet: does not print anything and is therefore only useful for
scripting. Thus, there is no need to have it on the command line completion.

--exit-code: has no visible effect and is only useful for scripting.

--upload-pack: is really exotic. Nobody will ever use it without digging
deep in the manuals. Therefore, I think it's unnecessary to have the
option completable.


--symref: Should probably be added, thanks.

However, if you don't find my reasoning for omitting the three options
above conclusive, I have no problem including them.

Re: [PATCH 4/7] completion: teach ls-remote to complete options

[Junio C Hamano]

Cornelius Weig <cornelius.weig@tngtech.com> writes:

> On 02/02/2017 02:40 AM, SZEDER Gábor wrote:
>> 
>>> ls-remote needs to complete remote names and its own options.
>> 
>> And refnames, too.
>
> Yes, right. However, do you think it is reasonable to complete remote
> refnames? I don't think so, because it would mean we would have to run
> ls-remote during completion -- and waiting for ls-remote could be quite
> lengthy.

... and by the time the completion script knew what they are, we
have all information necessary without actually having the user run
the command ;-)  That does sound backwards.

I am however not sure what Szeder really meant by "refnames".  For
example, you may want to see that this

	$ git ls-remote origin mast<TAG>

peek into refs/remotes/origin/* and find those matching, i.e. most
likely "master", and that can be done without talking to the remote
side.  It won't catch the case where the remote end added a new
branch that also match, e.g. "mastiff", and it will actively harm
the users by giving the impression that Git (collectively, even
though from technical point of view, what the completion script does
is not part of Git) told them that there is no such new branch they
need to worry about.

So probably even with the "peek local" optimization, I have a feeling
that completion of refnames may not be such a good idea.

[PATCH 5/7] completion: teach replace to complete options

[bitte.keine.werbung.einwerfen]

From: Cornelius Weig <cornelius.weig@tngtech.com>

Git-replace needs to complete references and its own options. In
addition to the existing references completions, do also complete the
options --edit --graft --format= --list --delete.
---
 contrib/completion/git-completion.bash | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 36fe439..e76cbd7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2408,6 +2408,12 @@ _git_remote ()
 
 _git_replace ()
 {
+	case "$cur" in
+	--*)
+		__gitcomp "--edit --graft --format= --list --delete"
+		return
+		;;
+	esac
 	__gitcomp_nl "$(__git_refs)"
 }
 
-- 
2.10.2

[PATCH 6/7] completion: teach remote subcommands option completion

[bitte.keine.werbung.einwerfen]

From: Cornelius Weig <cornelius.weig@tngtech.com>

Git-remote needs to complete remote names, its subcommands, and options
thereof. In addition to the existing subcommand and remote name
completion, do also complete the options

 - add: --track --master --fetch --tags --no-tags --mirror=
 - set-url: --push --add --delete
 - get-url: --push --all
 - prune: --dry-run
---
 contrib/completion/git-completion.bash | 36 +++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e76cbd7..0e09519 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2384,24 +2384,46 @@ _git_config ()
 
 _git_remote ()
 {
-	local subcommands="add rename remove set-head set-branches set-url show prune update"
+	local subcommands="
+		add rename remove set-head set-branches
+		get-url set-url show prune update
+		"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
-		__gitcomp "$subcommands"
+		case "$cur" in
+		--*)
+			__gitcomp "--verbose"
+			;;
+		*)
+			__gitcomp "$subcommands"
+			;;
+		esac
 		return
 	fi
 
-	case "$subcommand" in
-	rename|remove|set-url|show|prune)
-		__gitcomp_nl "$(__git_remotes)"
+	case "$subcommand,$cur" in
+	add,--*)
+		__gitcomp "--track --master --fetch --tags --no-tags --mirror="
 		;;
-	set-head|set-branches)
+	add,*)
+		;;
+	set-head,*|set-branches,*)
 		__git_complete_remote_or_refspec
 		;;
-	update)
+	update,*)
 		__gitcomp "$(__git_get_config_variables "remotes")"
 		;;
+	set-url,--*)
+		__gitcomp "--push --add --delete"
+		;;
+	get-url,--*)
+		__gitcomp "--push --all"
+		;;
+	prune,--*)
+		__gitcomp "--dry-run"
+		;;
 	*)
+		__gitcomp_nl "$(__git_remotes)"
 		;;
 	esac
 }
-- 
2.10.2

Re: [PATCH 6/7] completion: teach remote subcommands option completion

[SZEDER Gábor]

> Git-remote needs to complete remote names, its subcommands, and options
> thereof. In addition to the existing subcommand and remote name
> completion, do also complete the options
> 
>  - add: --track --master --fetch --tags --no-tags --mirror=

Oh, '--track' and '--master' are not even in the manpage or in 'git
remote -h', I could only find them after looking at the source code...

Good eyes :)

>  - set-url: --push --add --delete
>  - get-url: --push --all
>  - prune: --dry-run
> ---
>  contrib/completion/git-completion.bash | 36 +++++++++++++++++++++++++++-------
>  1 file changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index e76cbd7..0e09519 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2384,24 +2384,46 @@ _git_config ()
>  
>  _git_remote ()
>  {
> -	local subcommands="add rename remove set-head set-branches set-url show prune update"
> +	local subcommands="
> +		add rename remove set-head set-branches
> +		get-url set-url show prune update
> +		"
>  	local subcommand="$(__git_find_on_cmdline "$subcommands")"
>  	if [ -z "$subcommand" ]; then
> -		__gitcomp "$subcommands"
> +		case "$cur" in
> +		--*)
> +			__gitcomp "--verbose"
> +			;;
> +		*)
> +			__gitcomp "$subcommands"
> +			;;
> +		esac
>  		return
>  	fi
>  
> -	case "$subcommand" in
> -	rename|remove|set-url|show|prune)
> -		__gitcomp_nl "$(__git_remotes)"
> +	case "$subcommand,$cur" in
> +	add,--*)
> +		__gitcomp "--track --master --fetch --tags --no-tags --mirror="
>  		;;
> -	set-head|set-branches)
> +	add,*)
> +		;;
> +	set-head,*|set-branches,*)

The 'set-head' subcommand has '--auto' and '--delete' options, and
'set-branches' has '--add'.

>  		__git_complete_remote_or_refspec
>  		;;
> -	update)
> +	update,*)

The 'update' subcommand has a '--prune' option.

Otherwise it all looks good.


>  		__gitcomp "$(__git_get_config_variables "remotes")"
>  		;;
> +	set-url,--*)
> +		__gitcomp "--push --add --delete"
> +		;;
> +	get-url,--*)
> +		__gitcomp "--push --all"
> +		;;
> +	prune,--*)
> +		__gitcomp "--dry-run"
> +		;;
>  	*)
> +		__gitcomp_nl "$(__git_remotes)"
>  		;;
>  	esac
>  }
> -- 
> 2.10.2

Re: [PATCH 6/7] completion: teach remote subcommands option completion

[Cornelius Weig]

On 02/02/2017 02:37 AM, SZEDER Gábor wrote:
> The 'set-head' subcommand has '--auto' and '--delete' options, and
> 'set-branches' has '--add'.

Oops. Thanks for spotting this..

>>  		__git_complete_remote_or_refspec
>>  		;;
>> -	update)
>> +	update,*)
> 
> The 'update' subcommand has a '--prune' option.
> 

..and that.

[PATCH 7/7] completion: recognize more long-options

[bitte.keine.werbung.einwerfen]

From: Cornelius Weig <cornelius.weig@tngtech.com>

Recognize several new long-options for bash completion in the following
commands:

 - apply: --recount --directory= --unsafe-paths
 - archive: --prefix= --remote= --exec= --output
 - branch: --column --no-column --sort= --points-at
 - clone: --no-single-branch --shallow-submodules
 - commit: --patch --short --date --allow-empty
 - describe: --first-parent
 - fetch, pull: --unshallow --update-shallow
 - fsck: --name-objects
 - grep: --break --heading --show-function --function-context
         --untracked --no-index
 - mergetool: --prompt --no-prompt
 - reset: --merge --mixed --hard --soft --patch --keep
 - revert: --strategy= --strategy-option=
 - rm: --force
 - shortlog: --email
 - tag: --merged --no-merged --create-reflog
---
 contrib/completion/git-completion.bash | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0e09519..87d3d2c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -936,6 +936,7 @@ _git_apply ()
 			--apply --no-add --exclude=
 			--ignore-whitespace --ignore-space-change
 			--whitespace= --inaccurate-eof --verbose
+			--recount --directory= --unsafe-paths
 			"
 		return
 	esac
@@ -974,7 +975,7 @@ _git_archive ()
 	--*)
 		__gitcomp "
 			--format= --list --verbose
-			--prefix= --remote= --exec=
+			--prefix= --remote= --exec= --output
 			"
 		return
 		;;
@@ -1029,6 +1030,7 @@ _git_branch ()
 			--track --no-track --contains --merged --no-merged
 			--set-upstream-to= --edit-description --list
 			--unset-upstream --delete --move --remotes
+			--column --no-column --sort= --points-at
 			"
 		;;
 	*)
@@ -1142,6 +1144,8 @@ _git_clone ()
 			--single-branch
 			--branch
 			--recurse-submodules
+			--no-single-branch
+			--shallow-submodules
 			"
 		return
 		;;
@@ -1183,6 +1187,7 @@ _git_commit ()
 			--reset-author --file= --message= --template=
 			--cleanup= --untracked-files --untracked-files=
 			--verbose --quiet --fixup= --squash=
+			--patch --short --date --allow-empty
 			"
 		return
 	esac
@@ -1201,7 +1206,7 @@ _git_describe ()
 	--*)
 		__gitcomp "
 			--all --tags --contains --abbrev= --candidates=
-			--exact-match --debug --long --match --always
+			--exact-match --debug --long --match --always --first-parent
 			"
 		return
 	esac
@@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no"
 __git_fetch_options="
 	--quiet --verbose --append --upload-pack --force --keep --depth=
 	--tags --no-tags --all --prune --dry-run --recurse-submodules=
+	--unshallow --update-shallow
 "
 
 _git_fetch ()
@@ -1333,7 +1339,7 @@ _git_fsck ()
 	--*)
 		__gitcomp "
 			--tags --root --unreachable --cache --no-reflogs --full
-			--strict --verbose --lost-found
+			--strict --verbose --lost-found --name-objects
 			"
 		return
 		;;
@@ -1377,6 +1383,8 @@ _git_grep ()
 			--max-depth
 			--count
 			--and --or --not --all-match
+			--break --heading --show-function --function-context
+			--untracked --no-index
 			"
 		return
 		;;
@@ -1576,7 +1584,7 @@ _git_mergetool ()
 		return
 		;;
 	--*)
-		__gitcomp "--tool="
+		__gitcomp "--tool= --prompt --no-prompt"
 		return
 		;;
 	esac
@@ -2456,7 +2464,7 @@ _git_reset ()
 
 	case "$cur" in
 	--*)
-		__gitcomp "--merge --mixed --hard --soft --patch"
+		__gitcomp "--merge --mixed --hard --soft --patch --keep"
 		return
 		;;
 	esac
@@ -2472,7 +2480,10 @@ _git_revert ()
 	fi
 	case "$cur" in
 	--*)
-		__gitcomp "--edit --mainline --no-edit --no-commit --signoff"
+		__gitcomp "
+			--edit --mainline --no-edit --no-commit --signoff
+			--strategy= --strategy-option=
+			"
 		return
 		;;
 	esac
@@ -2483,7 +2494,7 @@ _git_rm ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
+		__gitcomp "--cached --dry-run --ignore-unmatch --quiet --force"
 		return
 		;;
 	esac
@@ -2500,7 +2511,7 @@ _git_shortlog ()
 		__gitcomp "
 			$__git_log_common_options
 			$__git_log_shortlog_options
-			--numbered --summary
+			--numbered --summary --email
 			"
 		return
 		;;
@@ -2778,8 +2789,8 @@ _git_tag ()
 	--*)
 		__gitcomp "
 			--list --delete --verify --annotate --message --file
-			--sign --cleanup --local-user --force --column --sort
-			--contains --points-at
+			--sign --cleanup --local-user --force --column --sort=
+			--contains --points-at --merged --no-merged --create-reflog
 			"
 		;;
 	esac
-- 
2.10.2

Re: [PATCH 7/7] completion: recognize more long-options

[Johannes Sixt]

If at all possible, please use your real email address as the From 
address. It is pointless to hide behind a fake address because as Git 
contributor you will have to reveal your identity anyway.

Please study item (5) "Sign your work" in 
Documentation/SubmittingPatches and sign off your work.

I'm by no means a bash completion expert, but I have a comment on this 
patch.

Am 22.01.2017 um 23:57 schrieb bitte.keine.werbung.einwerfen@googlemail.com:
> Recognize several new long-options for bash completion in the following
> commands:

AFAIR, it was a deliberate decision that potentially destructive command 
line options are not included in command completions. In the list given, 
I find these:

>  - apply: --unsafe-paths
>  - reset: --merge --mixed --hard --soft --patch --keep
>  - rm: --force

Additionally, these options are only for internal purposes, but I may be 
wrong:

>  - archive: --remote= --exec=

-- Hannes

Re: [PATCH 7/7] completion: recognize more long-options

[Cornelius Weig]

On 01/24/2017 08:15 AM, Johannes Sixt wrote:
> If at all possible, please use your real email address as the From
> address. It is pointless to hide behind a fake address because as Git
> contributor you will have to reveal your identity anyway.

These are both real addresses, but for send-mail I would not want to use
my work account. I hope this is not a problem.

> Please study item (5) "Sign your work" in
> Documentation/SubmittingPatches and sign off your work.

I followed the recommendations to submitting work, and in the first
round signing is discouraged.

> AFAIR, it was a deliberate decision that potentially destructive command
> line options are not included in command completions. In the list given,
> I find these:
>
>>  - reset: --merge --mixed --hard --soft --patch --keep

My bad, I only added --keep, which should be fine. As to these options

>>  - apply: --unsafe-paths
>>  - rm: --force

let's wait for further comments, but I won't cling to it.

> Additionally, these options are only for internal purposes, but I may be
> wrong:
>
>>  - archive: --remote= --exec=

These may in fact be too exotic and just clutter the command line. Best
they are removed.

-- Cornelius

Re: [PATCH 7/7] completion: recognize more long-options

[Junio C Hamano]

Cornelius Weig <cornelius.weig@tngtech.com> writes:

>> Please study item (5) "Sign your work" in
>> Documentation/SubmittingPatches and sign off your work.
>
> I followed the recommendations to submitting work, and in the first
> round signing is discouraged.

Just this point.  You found a bug in our documentation if that is
the case; it should not be giving that impression to you.  

Re: [PATCH 7/7] completion: recognize more long-options

[Cornelius Weig]

On 01/25/2017 12:24 AM, Junio C Hamano wrote:
> Cornelius Weig <cornelius.weig@tngtech.com> writes:
> 
>>> Please study item (5) "Sign your work" in
>>> Documentation/SubmittingPatches and sign off your work.
>>
>> I followed the recommendations to submitting work, and in the first
>> round signing is discouraged.
> 
> Just this point.  You found a bug in our documentation if that is
> the case; it should not be giving that impression to you.  
> 

Well, I am referring to par. (4) of Documentation/SubmittingPatches
(emphasis mine):

<<<<<<<<<<<<<<
*Do not PGP sign your patch, at least for now*.  Most likely, your
maintainer or other people on the list would not have your PGP
key and would not bother obtaining it anyway.  Your patch is not
judged by who you are; a good patch from an unknown origin has a
far better chance of being accepted than a patch from a known,
respected origin that is done poorly or does incorrect things.
>>>>>>>>>>>>>>

If first submissions should be signed as well, then I find this quite
misleading.

Re: [PATCH 7/7] completion: recognize more long-options

[Stefan Beller]

On Tue, Jan 24, 2017 at 3:33 PM, Cornelius Weig
<cornelius.weig@tngtech.com> wrote:
> On 01/25/2017 12:24 AM, Junio C Hamano wrote:
>> Cornelius Weig <cornelius.weig@tngtech.com> writes:
>>
>>>> Please study item (5) "Sign your work" in
>>>> Documentation/SubmittingPatches and sign off your work.
>>>
>>> I followed the recommendations to submitting work, and in the first
>>> round signing is discouraged.
>>
>> Just this point.  You found a bug in our documentation if that is
>> the case; it should not be giving that impression to you.
>>
>
> Well, I am referring to par. (4) of Documentation/SubmittingPatches
> (emphasis mine):
>
> <<<<<<<<<<<<<<
> *Do not PGP sign your patch, at least for now*.  Most likely, your
> maintainer or other people on the list would not have your PGP
> key and would not bother obtaining it anyway.  Your patch is not
> judged by who you are; a good patch from an unknown origin has a
> far better chance of being accepted than a patch from a known,
> respected origin that is done poorly or does incorrect things.
>>>>>>>>>>>>>>>
>
> If first submissions should be signed as well, then I find this quite
> misleading.
>

Please read on; While this part addresses PGP signing,
which is discouraged at any round,
later on we talk about another type of signing.
(not cryptographic strong signing, but signing the intent;)
the DCO in the commit message.

So no PGP signing (in any round of the patch).

But DCO signed (in anything that you deem useful for the
project and that you are allowed to contribute)

Re: [PATCH 7/7] completion: recognize more long-options

[Cornelius Weig]

On 01/25/2017 12:43 AM, Stefan Beller wrote:
> On Tue, Jan 24, 2017 at 3:33 PM, Cornelius Weig
> <cornelius.weig@tngtech.com> wrote:
>> On 01/25/2017 12:24 AM, Junio C Hamano wrote:
>>> Cornelius Weig <cornelius.weig@tngtech.com> writes:
>>>
>>>>> Please study item (5) "Sign your work" in
>>>>> Documentation/SubmittingPatches and sign off your work.
>>>>
>>>> I followed the recommendations to submitting work, and in the first
>>>> round signing is discouraged.
>>>
>>> Just this point.  You found a bug in our documentation if that is
>>> the case; it should not be giving that impression to you.
>>>
>>
>> Well, I am referring to par. (4) of Documentation/SubmittingPatches
>> (emphasis mine):
>>
>> <<<<<<<<<<<<<<
>> *Do not PGP sign your patch, at least for now*.  Most likely, your
>> maintainer or other people on the list would not have your PGP
>> key and would not bother obtaining it anyway.  Your patch is not
>> judged by who you are; a good patch from an unknown origin has a
>> far better chance of being accepted than a patch from a known,
>> respected origin that is done poorly or does incorrect things.
>> <<<<<<<<<<<<<<
>>
>> If first submissions should be signed as well, then I find this quite
>> misleading.
>>
> 
> Please read on; While this part addresses PGP signing,
> which is discouraged at any round,
> later on we talk about another type of signing.
> (not cryptographic strong signing, but signing the intent;)
> the DCO in the commit message.
> 
> So no PGP signing (in any round of the patch).
> 
> But DCO signed (in anything that you deem useful for the
> project and that you are allowed to contribute)
> 

Right, it's crystal clear now. What confused me was the combination of

> Do not PGP sign your patch, at least *for now*. (...)

and then the section with heading

> (5) *Sign* your work

So I didn't even bother to read (5) because I deemed it irrelevant. I
think if it had said `(5) *Certify* your work` this would not have happened.

(no subject)

[Stefan Beller]

>
>> Do not PGP sign your patch, at least *for now*. (...)
>

And maybe these 2 small words are the bug in the documentation?
Shall we drop the "at least for now" part, like so:

---8<---
From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001
From: Stefan Beller <sbeller@google.com>
Date: Tue, 24 Jan 2017 16:19:13 -0800
Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP signing

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/SubmittingPatches | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352deaae..28da4ad2d4 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -216,12 +216,12 @@ that it will be postponed.
 Exception:  If your mailer is mangling patches then someone may ask
 you to re-send them using MIME, that is OK.
 
-Do not PGP sign your patch, at least for now.  Most likely, your
-maintainer or other people on the list would not have your PGP
-key and would not bother obtaining it anyway.  Your patch is not
-judged by who you are; a good patch from an unknown origin has a
-far better chance of being accepted than a patch from a known,
-respected origin that is done poorly or does incorrect things.
+Do not PGP sign your patch. Most likely, your maintainer or other
+people on the list would not have your PGP key and would not bother
+obtaining it anyway. Your patch is not judged by who you are; a good
+patch from an unknown origin has a far better chance of being accepted
+than a patch from a known, respected origin that is done poorly or
+does incorrect things.
 
 If you really really really really want to do a PGP signed
 patch, format it as "multipart/signed", not a text/plain message
-- 
2.11.0.495.g04f60290a0.dirty

Re:

[Cornelius Weig]

On 01/25/2017 01:21 AM, Stefan Beller wrote:
>>
>>> Do not PGP sign your patch, at least *for now*. (...)
>>
> 
> And maybe these 2 small words are the bug in the documentation?
> Shall we drop the "at least for now" part, like so:
> 
> ---8<---
> From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001
> From: Stefan Beller <sbeller@google.com>
> Date: Tue, 24 Jan 2017 16:19:13 -0800
> Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP signing
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  Documentation/SubmittingPatches | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 08352deaae..28da4ad2d4 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,12 @@ that it will be postponed.
>  Exception:  If your mailer is mangling patches then someone may ask
>  you to re-send them using MIME, that is OK.
>  
> -Do not PGP sign your patch, at least for now.  Most likely, your
> -maintainer or other people on the list would not have your PGP
> -key and would not bother obtaining it anyway.  Your patch is not
> -judged by who you are; a good patch from an unknown origin has a
> -far better chance of being accepted than a patch from a known,
> -respected origin that is done poorly or does incorrect things.
> +Do not PGP sign your patch. Most likely, your maintainer or other
> +people on the list would not have your PGP key and would not bother
> +obtaining it anyway. Your patch is not judged by who you are; a good
> +patch from an unknown origin has a far better chance of being accepted
> +than a patch from a known, respected origin that is done poorly or
> +does incorrect things.
>  
>  If you really really really really want to do a PGP signed
>  patch, format it as "multipart/signed", not a text/plain message
> 

It definitely is an improvement. Though it would still leave me puzzled
when finding a section about signing just below.

Is changing heading (5) too big a change? Like so:

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352de..71898dc 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -246,7 +246,7 @@ patch.
      *2* The mailing list: git@vger.kernel.org
 
 
-(5) Sign your work
+(5) Certify your work by signing off.
 
 To improve tracking of who did what, we've borrowed the
 "sign-off" procedure from the Linux kernel project on patches

Re:

[Stefan Beller]

On Tue, Jan 24, 2017 at 4:43 PM, Cornelius Weig
<cornelius.weig@tngtech.com> wrote:

> -(5) Sign your work
> +(5) Certify your work by signing off.

That sounds better than what I proposed.

Thanks,
Stefan

Re:

[Linus Torvalds]

On Tue, Jan 24, 2017 at 4:21 PM, Stefan Beller <sbeller@google.com> wrote:
>
> +Do not PGP sign your patch. Most likely, your maintainer or other
> +people on the list would not have your PGP key and would not bother
> +obtaining it anyway.

I think even that could be further simplified - by just removing all
comments about pgp email

Because it's not that the PGP keys would be hard to get, it's that
PGP-signed email is an abject failure, and nobody sane does it.

Google for "phil zimmerman doesn't use pgp email".

It's dead. So I'm not sure it's worth mentioning at all.

You might as well talk about how you shouldn't use EBCDIC encoding for
your patches, or about why git assumes that an email address has an
'@' sign in it, instead of being an UUCP bang path address.

              Linus

Re:

[Eric Wong]

Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Tue, Jan 24, 2017 at 4:21 PM, Stefan Beller <sbeller@google.com> wrote:
> >
> > +Do not PGP sign your patch. Most likely, your maintainer or other
> > +people on the list would not have your PGP key and would not bother
> > +obtaining it anyway.
> 
> I think even that could be further simplified - by just removing all
> comments about pgp email
> 
> Because it's not that the PGP keys would be hard to get, it's that
> PGP-signed email is an abject failure, and nobody sane does it.
> 
> Google for "phil zimmerman doesn't use pgp email".
> 
> It's dead. So I'm not sure it's worth mentioning at all.

I disagree, we still see it, and Debian still advocates it.
In fact, we may also want to mention S/MIME in the same breath:

  https://public-inbox.org/git/20170110004031.57985-2-hansenr@google.com/

Richard's more recent mails seem to indicate he's reformed :)

Re: SubmittingPatches: drop temporal reference for PGP signing

[Philip Oakley]

From: "Stefan Beller" <sbeller@google.com>
>>
>>> Do not PGP sign your patch, at least *for now*. (...)
>>
>
> And maybe these 2 small words are the bug in the documentation?
> Shall we drop the "at least for now" part, like so:
>
> ---8<---
> From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001
> From: Stefan Beller <sbeller@google.com>
> Date: Tue, 24 Jan 2017 16:19:13 -0800
> Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP 
> signing
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> Documentation/SubmittingPatches | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/SubmittingPatches 
> b/Documentation/SubmittingPatches
> index 08352deaae..28da4ad2d4 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,12 @@ that it will be postponed.
> Exception:  If your mailer is mangling patches then someone may ask
> you to re-send them using MIME, that is OK.
>
> -Do not PGP sign your patch, at least for now.  Most likely, your
> -maintainer or other people on the list would not have your PGP
> -key and would not bother obtaining it anyway.  Your patch is not
> -judged by who you are; a good patch from an unknown origin has a
> -far better chance of being accepted than a patch from a known,
> -respected origin that is done poorly or does incorrect things.
> +Do not PGP sign your patch. Most likely, your maintainer or other
> +people on the list would not have your PGP key and would not bother
> +obtaining it anyway. Your patch is not judged by who you are; a good
> +patch from an unknown origin has a far better chance of being accepted
> +than a patch from a known, respected origin that is done poorly or
> +does incorrect things.

Wouldn't this also benefit from a forward reference to the section 5 on the 
DOC signining? This would avoid Cornelius's case where he felt that section 
5 no longer applied.

> If you really really really really want to do a PGP signed
> patch, format it as "multipart/signed", not a text/plain message
> -- 
> 2.11.0.495.g04f60290a0.dirty
--
Philip 

Re: SubmittingPatches: drop temporal reference for PGP signing

[Stefan Beller]

On Tue, Jan 24, 2017 at 10:54 PM, Philip Oakley <philipoakley@iee.org> wrote:
>> -Do not PGP sign your patch, at least for now.  Most likely, your
>> -maintainer or other people on the list would not have your PGP
>> -key and would not bother obtaining it anyway.  Your patch is not
>> -judged by who you are; a good patch from an unknown origin has a
>> -far better chance of being accepted than a patch from a known,
>> -respected origin that is done poorly or does incorrect things.
>> +Do not PGP sign your patch. Most likely, your maintainer or other
>> +people on the list would not have your PGP key and would not bother
>> +obtaining it anyway. Your patch is not judged by who you are; a good
>> +patch from an unknown origin has a far better chance of being accepted
>> +than a patch from a known, respected origin that is done poorly or
>> +does incorrect things.
>
>
> Wouldn't this also benefit from a forward reference to the section 5 on the
> DOC signining? This would avoid Cornelius's case where he felt that section
> 5 no longer applied.

Yeah I agree. My patch was not the best shot by far.

Re: SubmittingPatches: drop temporal reference for PGP signing

[Cornelius Weig]

> 
> Yeah I agree. My patch was not the best shot by far.
> 

How about something along these lines? Does the forward reference
break the main line of thought too severly?

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352de..c2b0cbe 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -216,12 +216,12 @@ that it will be postponed.
 Exception:  If your mailer is mangling patches then someone may ask
 you to re-send them using MIME, that is OK.
 
-Do not PGP sign your patch, at least for now.  Most likely, your
-maintainer or other people on the list would not have your PGP
-key and would not bother obtaining it anyway.  Your patch is not
-judged by who you are; a good patch from an unknown origin has a
-far better chance of being accepted than a patch from a known,
-respected origin that is done poorly or does incorrect things.
+Do not PGP sign your patch, but do sign-off your work as explained in (5).
+Most likely, your maintainer or other people on the list would not have your
+PGP key and would not bother obtaining it anyway. Your patch is not judged by
+who you are; a good patch from an unknown origin has a far better chance of
+being accepted than a patch from a known, respected origin that is done poorly
+or does incorrect things.
 
 If you really really really really want to do a PGP signed
 patch, format it as "multipart/signed", not a text/plain message
@@ -246,7 +246,7 @@ patch.
      *2* The mailing list: git@vger.kernel.org
 
 
-(5) Sign your work
+(5) Certify your work by signing off
 
 To improve tracking of who did what, we've borrowed the
 "sign-off" procedure from the Linux kernel project on patches

Re: SubmittingPatches: drop temporal reference for PGP signing

[Stefan Beller]

On Thu, Jan 26, 2017 at 5:30 AM, Cornelius Weig
<cornelius.weig@tngtech.com> wrote:
>
>>
>> Yeah I agree. My patch was not the best shot by far.
>>
>
> How about something along these lines? Does the forward reference
> break the main line of thought too severly?
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 08352de..c2b0cbe 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,12 @@ that it will be postponed.
>  Exception:  If your mailer is mangling patches then someone may ask
>  you to re-send them using MIME, that is OK.
>
> -Do not PGP sign your patch, at least for now.  Most likely, your
> -maintainer or other people on the list would not have your PGP
> -key and would not bother obtaining it anyway.  Your patch is not
> -judged by who you are; a good patch from an unknown origin has a
> -far better chance of being accepted than a patch from a known,
> -respected origin that is done poorly or does incorrect things.
> +Do not PGP sign your patch, but do sign-off your work as explained in (5).
> +Most likely, your maintainer or other people on the list would not have your
> +PGP key and would not bother obtaining it anyway. Your patch is not judged by
> +who you are; a good patch from an unknown origin has a far better chance of
> +being accepted than a patch from a known, respected origin that is done poorly
> +or does incorrect things.
>
>  If you really really really really want to do a PGP signed
>  patch, format it as "multipart/signed", not a text/plain message
> @@ -246,7 +246,7 @@ patch.
>       *2* The mailing list: git@vger.kernel.org
>
>
> -(5) Sign your work
> +(5) Certify your work by signing off
>
>  To improve tracking of who did what, we've borrowed the
>  "sign-off" procedure from the Linux kernel project on patches

I like it.

Thanks,
Stefan

Re: SubmittingPatches: drop temporal reference for PGP signing

[Junio C Hamano]

Cornelius Weig <cornelius.weig@tngtech.com> writes:

> How about something along these lines? Does the forward reference
> break the main line of thought too severly?

I find it a bit distracting for those who know PGP signing has
nothing to do with signing off your patch, but I think that is OK
because they are not the primary target audience of this part of the
document.

I however am more worried that it may be misleading to mention these
two in the same sentence.  Those who skim these paragraphs without
knowing the difference between the two may get a false impression
that these two may somehow be related because they are mentioned in
the same sentence.

The retitling of section (5) you did, without any other change,
might be sufficient.  It may also help to be even more explicit in
the updated title, i.e. s/by signing off/by adding Signed-off-by:/

Thanks.

> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 08352de..c2b0cbe 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,12 @@ that it will be postponed.
>  Exception:  If your mailer is mangling patches then someone may ask
>  you to re-send them using MIME, that is OK.
>  
> -Do not PGP sign your patch, at least for now.  Most likely, your
> -maintainer or other people on the list would not have your PGP
> -key and would not bother obtaining it anyway.  Your patch is not
> -judged by who you are; a good patch from an unknown origin has a
> -far better chance of being accepted than a patch from a known,
> -respected origin that is done poorly or does incorrect things.
> +Do not PGP sign your patch, but do sign-off your work as explained in (5).
> +Most likely, your maintainer or other people on the list would not have your
> +PGP key and would not bother obtaining it anyway. Your patch is not judged by
> +who you are; a good patch from an unknown origin has a far better chance of
> +being accepted than a patch from a known, respected origin that is done poorly
> +or does incorrect things.
>  
>  If you really really really really want to do a PGP signed
>  patch, format it as "multipart/signed", not a text/plain message
> @@ -246,7 +246,7 @@ patch.
>       *2* The mailing list: git@vger.kernel.org
>  
>  
> -(5) Sign your work
> +(5) Certify your work by signing off
>  
>  To improve tracking of who did what, we've borrowed the
>  "sign-off" procedure from the Linux kernel project on patches

Re: SubmittingPatches: drop temporal reference for PGP signing

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> Cornelius Weig <cornelius.weig@tngtech.com> writes:
>
>> How about something along these lines? Does the forward reference
>> break the main line of thought too severly?
>
> I find it a bit distracting for those who know PGP signing has
> nothing to do with signing off your patch, but I think that is OK
> because they are not the primary target audience of this part of the
> document.

Agreed. I this case the target audience was those weren't aware of that.
>
> I however am more worried that it may be misleading to mention these
> two in the same sentence.  Those who skim these paragraphs without
> knowing the difference between the two may get a false impression
> that these two may somehow be related because they are mentioned in
> the same sentence.
>
> The retitling of section (5) you did, without any other change,
> might be sufficient.  It may also help to be even more explicit in
> the updated title, i.e. s/by signing off/by adding Signed-off-by:/

Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure that 
the reader knows that it is _their certification_ that is being sought. Even 
if it does double up on the 'your'.

>
> Thanks.
>
>> diff --git a/Documentation/SubmittingPatches 
>> b/Documentation/SubmittingPatches
>> index 08352de..c2b0cbe 100644
>> --- a/Documentation/SubmittingPatches
>> +++ b/Documentation/SubmittingPatches
>> @@ -216,12 +216,12 @@ that it will be postponed.
>>  Exception:  If your mailer is mangling patches then someone may ask
>>  you to re-send them using MIME, that is OK.
>>
>> -Do not PGP sign your patch, at least for now.  Most likely, your
>> -maintainer or other people on the list would not have your PGP
>> -key and would not bother obtaining it anyway.  Your patch is not
>> -judged by who you are; a good patch from an unknown origin has a
>> -far better chance of being accepted than a patch from a known,
>> -respected origin that is done poorly or does incorrect things.
>> +Do not PGP sign your patch, but do sign-off your work as explained in 
>> (5).
>> +Most likely, your maintainer or other people on the list would not have 
>> your
>> +PGP key and would not bother obtaining it anyway. Your patch is not 
>> judged by
>> +who you are; a good patch from an unknown origin has a far better chance 
>> of
>> +being accepted than a patch from a known, respected origin that is done 
>> poorly
>> +or does incorrect things.
>>
>>  If you really really really really want to do a PGP signed
>>  patch, format it as "multipart/signed", not a text/plain message
>> @@ -246,7 +246,7 @@ patch.
>>       *2* The mailing list: git@vger.kernel.org
>>
>>
>> -(5) Sign your work
>> +(5) Certify your work by signing off
>>
>>  To improve tracking of who did what, we've borrowed the
>>  "sign-off" procedure from the Linux kernel project on patches
> 

Re: SubmittingPatches: drop temporal reference for PGP signing

[Cornelius Weig]

On 01/26/2017 09:58 PM, Philip Oakley wrote:
> From: "Junio C Hamano" <gitster@pobox.com>
>> Cornelius Weig <cornelius.weig@tngtech.com> writes:
>>
>>> How about something along these lines? Does the forward reference
>>> break the main line of thought too severly?
>>
>> I find it a bit distracting for those who know PGP signing has
>> nothing to do with signing off your patch, but I think that is OK
>> because they are not the primary target audience of this part of the
>> document.
> 
> Agreed. I this case the target audience was those weren't aware of that.

Yes, maybe the forward reference does give a wrong hint about a
connection between sign-off and pgp-signing. However, I would still vote
for the following change suggested by sbeller@google.com:

-Do not PGP sign your patch, -at least for now-. Most likely, your (...)
+Do not PGP sign your patch. Most likely, your maintainer or other (...)


> 
> Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure
> that the reader knows that it is _their certification_ that is being
> sought. Even if it does double up on the 'your'.
> 

I don't think doubling on the 'your' will make the heading clearer. The
main intention of this change is to avoid mixups with pgp-signing and
that would IMHO not improve by that.
Besides, I find the colon in the heading a bit awkward. Is the following
version as expressive as with the colon?

-(5) Sign your work
+(5) Certify your work by adding Signed-off-by

Re: SubmittingPatches: drop temporal reference for PGP signing

[Junio C Hamano]

Cornelius Weig <cornelius.weig@tngtech.com> writes:

> -Do not PGP sign your patch, -at least for now-. Most likely, your (...)
> +Do not PGP sign your patch. Most likely, your maintainer or other (...)

It has been quite a while since we wrote that "at least for now", so
it probably makes sense to drop it.

>> Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure
>> that the reader knows that it is _their certification_ that is being
>> sought. Even if it does double up on the 'your'.
>
> I don't think doubling on the 'your' will make the heading clearer. The
> main intention of this change is to avoid mixups with pgp-signing and
> that would IMHO not improve by that.
> Besides, I find the colon in the heading a bit awkward. Is the following
> version as expressive as with the colon?
>
> -(5) Sign your work
> +(5) Certify your work by adding Signed-off-by

I am leaning towards agreeing with Philip, and my gut feeling says
the presense of the colon, would help a lot.  It would visually
click to readers what we are talking about.

At the same time, I think ending a section header with a colon would
be seen as "awkward".  I would have written it more like this to
avoid it:

    ... by adding "Signed-off-by: " line

I personally do not mind having "your" there, either, but I can see
that a section header wants to be kept shorter.

[PATCH v2 0/7] completion: recognize more long-options

[cornelius.weig]

From: Cornelius Weig <cornelius.weig@tngtech.com>

This revision addresses Johannes' concerns. Changes wrt v1:

 - fixed the commit message: two of the "dangerous" options erroneously ended
   up in the commit message. These options were already in the list of
   auto-completable options.
 - removed the possibly dangerous option '--unsafe-paths' from git-apply.
 - added my sign-off

Patches 1-6 are not resent, because they have not changed (other than my added sign-off).

Also, I added further people to CC, because nobody actually has looked at the code yet.


Cornelius Weig (7):
  completion: recognize more long-options

 contrib/completion/git-completion.bash | 132 +++++++++++++++++++++++++++------
 1 file changed, 110 insertions(+), 22 deletions(-)

-- 
2.10.2

[PATCH v2 7/7] completion: recognize more long-options

[cornelius.weig]

From: Cornelius Weig <cornelius.weig@tngtech.com>

Recognize several new long-options for bash completion in the following
commands:

 - apply: --recount --directory=
 - archive: --output
 - branch: --column --no-column --sort= --points-at
 - clone: --no-single-branch --shallow-submodules
 - commit: --patch --short --date --allow-empty
 - describe: --first-parent
 - fetch, pull: --unshallow --update-shallow
 - fsck: --name-objects
 - grep: --break --heading --show-function --function-context
         --untracked --no-index
 - mergetool: --prompt --no-prompt
 - reset: --keep
 - revert: --strategy= --strategy-option=
 - rm: --force
 - shortlog: --email
 - tag: --merged --no-merged --create-reflog

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
Helped-by: Johannes Sixt <j6t@kdbg.org>
---
 contrib/completion/git-completion.bash | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0e09519..933bb6e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -936,6 +936,7 @@ _git_apply ()
 			--apply --no-add --exclude=
 			--ignore-whitespace --ignore-space-change
 			--whitespace= --inaccurate-eof --verbose
+			--recount --directory=
 			"
 		return
 	esac
@@ -974,7 +975,7 @@ _git_archive ()
 	--*)
 		__gitcomp "
 			--format= --list --verbose
-			--prefix= --remote= --exec=
+			--prefix= --remote= --exec= --output
 			"
 		return
 		;;
@@ -1029,6 +1030,7 @@ _git_branch ()
 			--track --no-track --contains --merged --no-merged
 			--set-upstream-to= --edit-description --list
 			--unset-upstream --delete --move --remotes
+			--column --no-column --sort= --points-at
 			"
 		;;
 	*)
@@ -1142,6 +1144,8 @@ _git_clone ()
 			--single-branch
 			--branch
 			--recurse-submodules
+			--no-single-branch
+			--shallow-submodules
 			"
 		return
 		;;
@@ -1183,6 +1187,7 @@ _git_commit ()
 			--reset-author --file= --message= --template=
 			--cleanup= --untracked-files --untracked-files=
 			--verbose --quiet --fixup= --squash=
+			--patch --short --date --allow-empty
 			"
 		return
 	esac
@@ -1201,7 +1206,7 @@ _git_describe ()
 	--*)
 		__gitcomp "
 			--all --tags --contains --abbrev= --candidates=
-			--exact-match --debug --long --match --always
+			--exact-match --debug --long --match --always --first-parent
 			"
 		return
 	esac
@@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no"
 __git_fetch_options="
 	--quiet --verbose --append --upload-pack --force --keep --depth=
 	--tags --no-tags --all --prune --dry-run --recurse-submodules=
+	--unshallow --update-shallow
 "
 
 _git_fetch ()
@@ -1333,7 +1339,7 @@ _git_fsck ()
 	--*)
 		__gitcomp "
 			--tags --root --unreachable --cache --no-reflogs --full
-			--strict --verbose --lost-found
+			--strict --verbose --lost-found --name-objects
 			"
 		return
 		;;
@@ -1377,6 +1383,8 @@ _git_grep ()
 			--max-depth
 			--count
 			--and --or --not --all-match
+			--break --heading --show-function --function-context
+			--untracked --no-index
 			"
 		return
 		;;
@@ -1576,7 +1584,7 @@ _git_mergetool ()
 		return
 		;;
 	--*)
-		__gitcomp "--tool="
+		__gitcomp "--tool= --prompt --no-prompt"
 		return
 		;;
 	esac
@@ -2456,7 +2464,7 @@ _git_reset ()
 
 	case "$cur" in
 	--*)
-		__gitcomp "--merge --mixed --hard --soft --patch"
+		__gitcomp "--merge --mixed --hard --soft --patch --keep"
 		return
 		;;
 	esac
@@ -2472,7 +2480,10 @@ _git_revert ()
 	fi
 	case "$cur" in
 	--*)
-		__gitcomp "--edit --mainline --no-edit --no-commit --signoff"
+		__gitcomp "
+			--edit --mainline --no-edit --no-commit --signoff
+			--strategy= --strategy-option=
+			"
 		return
 		;;
 	esac
@@ -2483,7 +2494,7 @@ _git_rm ()
 {
 	case "$cur" in
 	--*)
-		__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
+		__gitcomp "--cached --dry-run --ignore-unmatch --quiet --force"
 		return
 		;;
 	esac
@@ -2500,7 +2511,7 @@ _git_shortlog ()
 		__gitcomp "
 			$__git_log_common_options
 			$__git_log_shortlog_options
-			--numbered --summary
+			--numbered --summary --email
 			"
 		return
 		;;
@@ -2778,8 +2789,8 @@ _git_tag ()
 	--*)
 		__gitcomp "
 			--list --delete --verify --annotate --message --file
-			--sign --cleanup --local-user --force --column --sort
-			--contains --points-at
+			--sign --cleanup --local-user --force --column --sort=
+			--contains --points-at --merged --no-merged --create-reflog
 			"
 		;;
 	esac
-- 
2.10.2

Re: [PATCH v2 7/7] completion: recognize more long-options

[SZEDER Gábor]

On Fri, Jan 27, 2017 at 10:17 PM,  <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> Recognize several new long-options for bash completion in the following
> commands:

Adding more long options that git commands learn along the way is
always an improvement.  However, seeing "_several_ new long options"
(or "some long options" in one of the other patches in the series)
makes the reader wonder: are these the only new long options missing
or are there more?  If there are more, why only these are added?  If
there aren't any more missing long options left, then please say so,
e.g. "Add all missing long options, except the potentially
desctructive ones, for the following commands: ...."


>  - apply: --recount --directory=
>  - archive: --output
>  - branch: --column --no-column --sort= --points-at
>  - clone: --no-single-branch --shallow-submodules
>  - commit: --patch --short --date --allow-empty
>  - describe: --first-parent
>  - fetch, pull: --unshallow --update-shallow
>  - fsck: --name-objects
>  - grep: --break --heading --show-function --function-context
>          --untracked --no-index
>  - mergetool: --prompt --no-prompt
>  - reset: --keep
>  - revert: --strategy= --strategy-option=
>  - rm: --force

'--force' is a potentially destructive option, too.

>  - shortlog: --email
>  - tag: --merged --no-merged --create-reflog
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> Helped-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  contrib/completion/git-completion.bash | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 0e09519..933bb6e 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -936,6 +936,7 @@ _git_apply ()
>                         --apply --no-add --exclude=
>                         --ignore-whitespace --ignore-space-change
>                         --whitespace= --inaccurate-eof --verbose
> +                       --recount --directory=
>                         "
>                 return
>         esac
> @@ -974,7 +975,7 @@ _git_archive ()
>         --*)
>                 __gitcomp "
>                         --format= --list --verbose
> -                       --prefix= --remote= --exec=
> +                       --prefix= --remote= --exec= --output
>                         "
>                 return
>                 ;;
> @@ -1029,6 +1030,7 @@ _git_branch ()
>                         --track --no-track --contains --merged --no-merged
>                         --set-upstream-to= --edit-description --list
>                         --unset-upstream --delete --move --remotes
> +                       --column --no-column --sort= --points-at
>                         "
>                 ;;
>         *)
> @@ -1142,6 +1144,8 @@ _git_clone ()
>                         --single-branch
>                         --branch
>                         --recurse-submodules
> +                       --no-single-branch
> +                       --shallow-submodules
>                         "
>                 return
>                 ;;
> @@ -1183,6 +1187,7 @@ _git_commit ()
>                         --reset-author --file= --message= --template=
>                         --cleanup= --untracked-files --untracked-files=
>                         --verbose --quiet --fixup= --squash=
> +                       --patch --short --date --allow-empty
>                         "
>                 return
>         esac
> @@ -1201,7 +1206,7 @@ _git_describe ()
>         --*)
>                 __gitcomp "
>                         --all --tags --contains --abbrev= --candidates=
> -                       --exact-match --debug --long --match --always
> +                       --exact-match --debug --long --match --always --first-parent
>                         "
>                 return
>         esac
> @@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no"
>  __git_fetch_options="
>         --quiet --verbose --append --upload-pack --force --keep --depth=
>         --tags --no-tags --all --prune --dry-run --recurse-submodules=
> +       --unshallow --update-shallow
>  "
>
>  _git_fetch ()
> @@ -1333,7 +1339,7 @@ _git_fsck ()
>         --*)
>                 __gitcomp "
>                         --tags --root --unreachable --cache --no-reflogs --full
> -                       --strict --verbose --lost-found
> +                       --strict --verbose --lost-found --name-objects
>                         "
>                 return
>                 ;;
> @@ -1377,6 +1383,8 @@ _git_grep ()
>                         --max-depth
>                         --count
>                         --and --or --not --all-match
> +                       --break --heading --show-function --function-context
> +                       --untracked --no-index
>                         "
>                 return
>                 ;;
> @@ -1576,7 +1584,7 @@ _git_mergetool ()
>                 return
>                 ;;
>         --*)
> -               __gitcomp "--tool="
> +               __gitcomp "--tool= --prompt --no-prompt"
>                 return
>                 ;;
>         esac
> @@ -2456,7 +2464,7 @@ _git_reset ()
>
>         case "$cur" in
>         --*)
> -               __gitcomp "--merge --mixed --hard --soft --patch"
> +               __gitcomp "--merge --mixed --hard --soft --patch --keep"
>                 return
>                 ;;
>         esac
> @@ -2472,7 +2480,10 @@ _git_revert ()
>         fi
>         case "$cur" in
>         --*)
> -               __gitcomp "--edit --mainline --no-edit --no-commit --signoff"
> +               __gitcomp "
> +                       --edit --mainline --no-edit --no-commit --signoff
> +                       --strategy= --strategy-option=
> +                       "
>                 return
>                 ;;
>         esac
> @@ -2483,7 +2494,7 @@ _git_rm ()
>  {
>         case "$cur" in
>         --*)
> -               __gitcomp "--cached --dry-run --ignore-unmatch --quiet"
> +               __gitcomp "--cached --dry-run --ignore-unmatch --quiet --force"
>                 return
>                 ;;
>         esac
> @@ -2500,7 +2511,7 @@ _git_shortlog ()
>                 __gitcomp "
>                         $__git_log_common_options
>                         $__git_log_shortlog_options
> -                       --numbered --summary
> +                       --numbered --summary --email
>                         "
>                 return
>                 ;;
> @@ -2778,8 +2789,8 @@ _git_tag ()
>         --*)
>                 __gitcomp "
>                         --list --delete --verify --annotate --message --file
> -                       --sign --cleanup --local-user --force --column --sort
> -                       --contains --points-at
> +                       --sign --cleanup --local-user --force --column --sort=
> +                       --contains --points-at --merged --no-merged --create-reflog
>                         "
>                 ;;
>         esac
> --
> 2.10.2
>

Re: [PATCH v2 7/7] completion: recognize more long-options

[Cornelius Weig]

Hi Gabor,

 thanks for taking a look at these commits.

On 01/31/2017 11:17 PM, SZEDER Gábor wrote:
> On Fri, Jan 27, 2017 at 10:17 PM,  <cornelius.weig@tngtech.com> wrote:
>> From: Cornelius Weig <cornelius.weig@tngtech.com>
>>
>> Recognize several new long-options for bash completion in the following
>> commands:
> 
> Adding more long options that git commands learn along the way is
> always an improvement.  However, seeing "_several_ new long options"
> (or "some long options" in one of the other patches in the series)
> makes the reader wonder: are these the only new long options missing
> or are there more?  If there are more, why only these are added?  If
> there aren't any more missing long options left, then please say so,
> e.g. "Add all missing long options, except the potentially
> desctructive ones, for the following commands: ...."

Personally, I agree with you that
> Adding more long options that git commands learn along the way is
> always an improvement.
However, people may start complaining that their terminal becomes too
cluttered when doing a double-Tab. In my cover letter, I go to length
about this. My assumption was that all options that are mentioned in the
introduction of the command man-page should be important enough to have
them in the completion list. I'll change my commit message accordingly.

>>  - rm: --force
> 
> '--force' is a potentially destructive option, too.

Thanks for spotting this.

Btw, I haven't found that non-destructive options should not be eligible
for completion. To avoid confusion about this in the future, I suggest
to also change the documentation:

index 933bb6e..96f1c7f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -13,7 +13,7 @@
 #    *) git email aliases for git-send-email
 #    *) tree paths within 'ref:path/to/file' expressions
 #    *) file paths within current working directory and index
-#    *) common --long-options
+#    *) common non-destructive --long-options
 #
 # To use these routines:
 #


I take it you have also looked at the code itself? Then I would gladly
mention you as reviewer in my sign-off.

Re: [PATCH v2 7/7] completion: recognize more long-options

[SZEDER Gábor]

On Wed, Feb 1, 2017 at 5:49 PM, Cornelius Weig
<cornelius.weig@tngtech.com> wrote:
> Hi Gabor,
>
>  thanks for taking a look at these commits.
>
> On 01/31/2017 11:17 PM, SZEDER Gábor wrote:
>> On Fri, Jan 27, 2017 at 10:17 PM,  <cornelius.weig@tngtech.com> wrote:
>>> From: Cornelius Weig <cornelius.weig@tngtech.com>
>>>
>>> Recognize several new long-options for bash completion in the following
>>> commands:
>>
>> Adding more long options that git commands learn along the way is
>> always an improvement.  However, seeing "_several_ new long options"
>> (or "some long options" in one of the other patches in the series)
>> makes the reader wonder: are these the only new long options missing
>> or are there more?  If there are more, why only these are added?  If
>> there aren't any more missing long options left, then please say so,
>> e.g. "Add all missing long options, except the potentially
>> desctructive ones, for the following commands: ...."
>
> Personally, I agree with you that
>> Adding more long options that git commands learn along the way is
>> always an improvement.
> However, people may start complaining that their terminal becomes too
> cluttered when doing a double-Tab. In my cover letter, I go to length
> about this. My assumption was that all options that are mentioned in the
> introduction of the command man-page should be important enough to have
> them in the completion list.

But that doesn't mean that the ones not mentioned in the synopsis
section are not worth completing.

The list of options listed by the completion script for several of
these commands fits on a single line.  The command with the most
options among these is 'git pull', and even its options don't fill
more than half of a 80x25 screen.  I see no danger of people coming
complaining.

> I'll change my commit message accordingly.
>
>>>  - rm: --force
>>
>> '--force' is a potentially destructive option, too.
>
> Thanks for spotting this.
>
> Btw, I haven't found that non-destructive options should not be eligible
> for completion. To avoid confusion about this in the future, I suggest
> to also change the documentation:
>
> index 933bb6e..96f1c7f 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -13,7 +13,7 @@
>  #    *) git email aliases for git-send-email
>  #    *) tree paths within 'ref:path/to/file' expressions
>  #    *) file paths within current working directory and index
> -#    *) common --long-options
> +#    *) common non-destructive --long-options

I don't mind such a change, but I don't think that list was ever meant
to be comprehensive or decisive.  It is definitely not the former, as
it's missing several things that the completion script does support.
OTOH, it talks about .git/remotes, which has been considered legacy
for quite some years (though it's right, because the completion script
still supports it).

> I take it you have also looked at the code itself? Then I would gladly
> mention you as reviewer in my sign-off.

Yeah, most of the changes was rather straightforward, except the
completion of 'git remote's subcommands' options, but that looks
good, too.

Gábor

Re: [PATCH v2 7/7] completion: recognize more long-options

[Cornelius Weig]

On 02/02/2017 03:00 AM, SZEDER Gábor wrote:
>> Personally, I agree with you that
>>> Adding more long options that git commands learn along the way is
>>> always an improvement.
>> However, people may start complaining that their terminal becomes too
>> cluttered when doing a double-Tab. In my cover letter, I go to length
>> about this. My assumption was that all options that are mentioned in the
>> introduction of the command man-page should be important enough to have
>> them in the completion list.
> 
> But that doesn't mean that the ones not mentioned in the synopsis
> section are not worth completing.

Absolutely. What I meant is that at least the options from the synopsis
should be contained in the set of completable options.

>> Btw, I haven't found that non-destructive options should not be eligible
>> for completion. To avoid confusion about this in the future, I suggest
>> to also change the documentation:
>>
>> index 933bb6e..96f1c7f 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -13,7 +13,7 @@
>>  #    *) git email aliases for git-send-email
>>  #    *) tree paths within 'ref:path/to/file' expressions
>>  #    *) file paths within current working directory and index
>> -#    *) common --long-options
>> +#    *) common non-destructive --long-options
> 
> I don't mind such a change, but I don't think that list was ever meant
> to be comprehensive or decisive.  It is definitely not the former, as
> it's missing several things that the completion script does support.
> OTOH, it talks about .git/remotes, which has been considered legacy
> for quite some years (though it's right, because the completion script
> still supports it).

Then let's not do that change, because for some commands destructive
long-options have been in the list of completed options for quite a
while. Given that, the above change of the documentation, might stir up
more confusion than it settles.

Re: [PATCH 3/7] completion: improve bash completion for git-add

[SZEDER Gábor]

> Add some long-options for git-add and improve path completion when the
> --update flag is given.

Please tell us in the commit message _how_ this improves path
completion.

> ---
>  contrib/completion/git-completion.bash | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 8329f09..652c7e2 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -947,13 +947,17 @@ _git_add ()
>  	--*)
>  		__gitcomp "
>  			--interactive --refresh --patch --update --dry-run
> -			--ignore-errors --intent-to-add
> +			--ignore-errors --intent-to-add --force --edit --chmod=

I almost started complaining that '--force' should be used with care,
but then realized that for 'git add' it only means adding ignored
files.  So in this particular case '--force' is not destructive and we
can offer it among other long options.  Good.

>  			"
>  		return
>  	esac
>  
> -	# XXX should we check for --update and --all options ?
> -	__git_complete_index_file "--others --modified --directory --no-empty-directory"
> +	local complete_opt="--others --modified --directory --no-empty-directory"
> +	if test -n "$(__git_find_on_cmdline "-u --update")"
> +	then
> +		complete_opt="--modified"
> +	fi
> +	__git_complete_index_file "$complete_opt"
>  }
>  
>  _git_archive ()
> -- 
> 2.10.2