{"thread":{"id":"32717","subject":"[PATCH] t9902: Instruct git-completion.bash about a test mode","startedAt":"2013-01-24T21:20:08Z","lastAt":"2013-01-24T22:56:23Z","messageCount":5,"participants":["Jean-Noël AVILA","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"207721","messageId":"201301242220.09067.jn.avila@free.fr","threadId":"32717","inReplyTo":null,"subject":"[PATCH] t9902: Instruct git-completion.bash about a test mode","fromName":"Jean-Noël AVILA","fromEmail":"jn.avila@free.fr","sentAt":"2013-01-24T21:20:08Z","receivedAt":"2013-01-24T21:20:08Z","isPatch":true,"sender":{"key":"jn.avila@free.fr","avatar":"https://avatars.githubusercontent.com/u/156172?v=4"},"body":"In test mode, git completion should propose commands only if they\nbelong to the list of authorized commands.\n\nSigned-off-by: Jean-Noel Avila <jn.avila@free.fr>\n---\n\nBetter show some code than try to explain. Here is what I mean by\n\"filter the output git help -a\". \n\n contrib/completion/git-completion.bash | 16 +++++++++++++++-\n t/t9902-completion.sh                  |  4 ++--\n 2 files changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 14dd5e7..6490553 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -531,6 +531,20 @@ __git_complete_strategy ()\n \treturn 1\n }\n \n+if test -z \"$AUTHORIZED_CMD_LIST\"; then\n+\t__git_cmdlist ()\n+\t{ \n+\t\techo $1;\n+\t}\n+else\n+\t__git_cmdlist ()\n+\t{ \n+\t\tif [[ \" $AUTHORIZED_CMD_LIST \" =~ \" $1 \" ]] ; then\n+\t\t\t\techo $1\n+\t\tfi\n+\t}\n+fi\n+\n __git_list_all_commands ()\n {\n \tlocal i IFS=\" \"$'\\n'\n@@ -538,7 +552,7 @@ __git_list_all_commands ()\n \tdo\n \t\tcase $i in\n \t\t*--*)             : helper pattern;;\n-\t\t*) echo $i;;\n+\t\t*) __git_cmdlist $i;;\n \t\tesac\n \tdone\n }\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 3cd53f8..5e7d81e 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -12,8 +12,8 @@ complete ()\n \t# do nothing\n \treturn 0\n }\n-\n-. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\"\n+AUTHORIZED_CMD_LIST=\" checkout show add filter-branch ls-files send-email describe\"\n+. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\" \n \n # We don't need this function to actually join words or do anything special.\n # Also, it's cleaner to avoid touching bash's internal completion variables.\n-- \n1.8.1.1.271.g02f55e6\n"},{"id":"207724","messageId":"7vtxq68day.fsf@alter.siamese.dyndns.org","threadId":"32717","inReplyTo":"201301242220.09067.jn.avila@free.fr","subject":"Re: [PATCH] t9902: Instruct git-completion.bash about a test mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-24T21:46:13Z","receivedAt":"2013-01-24T21:46:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jean-Noël AVILA\" <jn.avila@free.fr> writes:\n\n> In test mode, git completion should propose commands only if they\n> belong to the list of authorized commands.\n>\n> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>\n> ---\n>\n> Better show some code than try to explain. Here is what I mean by\n> \"filter the output git help -a\". \n\nWhy do you have to make an extra shell function call for each and\nevery possible Git subcommand to slow down everybody's work when not\nin testing mode?\n\nComparing it with Peff's suggestion, it is fairly clear which one we\nshould pick, I think.\n\n\n\n>  contrib/completion/git-completion.bash | 16 +++++++++++++++-\n>  t/t9902-completion.sh                  |  4 ++--\n>  2 files changed, 17 insertions(+), 3 deletions(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 14dd5e7..6490553 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -531,6 +531,20 @@ __git_complete_strategy ()\n>  \treturn 1\n>  }\n>  \n> +if test -z \"$AUTHORIZED_CMD_LIST\"; then\n> +\t__git_cmdlist ()\n> +\t{ \n> +\t\techo $1;\n> +\t}\n> +else\n> +\t__git_cmdlist ()\n> +\t{ \n> +\t\tif [[ \" $AUTHORIZED_CMD_LIST \" =~ \" $1 \" ]] ; then\n> +\t\t\t\techo $1\n> +\t\tfi\n> +\t}\n> +fi\n> +\n>  __git_list_all_commands ()\n>  {\n>  \tlocal i IFS=\" \"$'\\n'\n> @@ -538,7 +552,7 @@ __git_list_all_commands ()\n>  \tdo\n>  \t\tcase $i in\n>  \t\t*--*)             : helper pattern;;\n> -\t\t*) echo $i;;\n> +\t\t*) __git_cmdlist $i;;\n>  \t\tesac\n>  \tdone\n>  }\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 3cd53f8..5e7d81e 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -12,8 +12,8 @@ complete ()\n>  \t# do nothing\n>  \treturn 0\n>  }\n> -\n> -. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\"\n> +AUTHORIZED_CMD_LIST=\" checkout show add filter-branch ls-files send-email describe\"\n> +. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\" \n>  \n>  # We don't need this function to actually join words or do anything special.\n>  # Also, it's cleaner to avoid touching bash's internal completion variables.\n"},{"id":"207725","messageId":"201301242304.16078.jn.avila@free.fr","threadId":"32717","inReplyTo":"7vtxq68day.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t9902: Instruct git-completion.bash about a test mode","fromName":"Jean-Noël AVILA","fromEmail":"jn.avila@free.fr","sentAt":"2013-01-24T22:04:15Z","receivedAt":"2013-01-24T22:04:15Z","isPatch":true,"sender":{"key":"jn.avila@free.fr","avatar":"https://avatars.githubusercontent.com/u/156172?v=4"},"body":"Le jeudi 24 janvier 2013 22:46:13, Junio C Hamano a écrit :\n> \"Jean-Noël AVILA\" <jn.avila@free.fr> writes:\n> \n> > In test mode, git completion should propose commands only if they\n> > belong to the list of authorized commands.\n> >\n> > Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>\n> > ---\n> >\n> > Better show some code than try to explain. Here is what I mean by\n> > \"filter the output git help -a\". \n> \n> Why do you have to make an extra shell function call for each and\n> every possible Git subcommand to slow down everybody's work when not\n> in testing mode?\n> \n\nMy rational was to be sure to put the environment variable out of the \nway once the script has been sourced. I can make two alternative \ndefinitions of __git_list_all_commands () depending on the presence of \n$AUTHORIZED_CMD_LIST if you are worried about performance.\n\n> Comparing it with Peff's suggestion, it is fairly clear which one we\n> should pick, I think.\n> \n> \n> \n> >  contrib/completion/git-completion.bash | 16 +++++++++++++++-\n> >  t/t9902-completion.sh                  |  4 ++--\n> >  2 files changed, 17 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> > index 14dd5e7..6490553 100644\n> > --- a/contrib/completion/git-completion.bash\n> > +++ b/contrib/completion/git-completion.bash\n> > @@ -531,6 +531,20 @@ __git_complete_strategy ()\n> >  \treturn 1\n> >  }\n> >  \n> > +if test -z \"$AUTHORIZED_CMD_LIST\"; then\n> > +\t__git_cmdlist ()\n> > +\t{ \n> > +\t\techo $1;\n> > +\t}\n> > +else\n> > +\t__git_cmdlist ()\n> > +\t{ \n> > +\t\tif [[ \" $AUTHORIZED_CMD_LIST \" =~ \" $1 \" ]] ; then\n> > +\t\t\t\techo $1\n> > +\t\tfi\n> > +\t}\n> > +fi\n> > +\n> >  __git_list_all_commands ()\n> >  {\n> >  \tlocal i IFS=\" \"$'\\n'\n> > @@ -538,7 +552,7 @@ __git_list_all_commands ()\n> >  \tdo\n> >  \t\tcase $i in\n> >  \t\t*--*)             : helper pattern;;\n> > -\t\t*) echo $i;;\n> > +\t\t*) __git_cmdlist $i;;\n> >  \t\tesac\n> >  \tdone\n> >  }\n> > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> > index 3cd53f8..5e7d81e 100755\n> > --- a/t/t9902-completion.sh\n> > +++ b/t/t9902-completion.sh\n> > @@ -12,8 +12,8 @@ complete ()\n> >  \t# do nothing\n> >  \treturn 0\n> >  }\n> > -\n> > -. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\"\n> > +AUTHORIZED_CMD_LIST=\" checkout show add filter-branch ls-files send-email describe\"\n> > +. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\" \n> >  \n> >  # We don't need this function to actually join words or do anything special.\n> >  # Also, it's cleaner to avoid touching bash's internal completion variables.\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"207728","messageId":"7vlibi8baq.fsf@alter.siamese.dyndns.org","threadId":"32717","inReplyTo":"201301242304.16078.jn.avila@free.fr","subject":"Re: [PATCH] t9902: Instruct git-completion.bash about a test mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-24T22:29:33Z","receivedAt":"2013-01-24T22:29:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jean-Noël AVILA\" <jn.avila@free.fr> writes:\n\n> My rational was to be sure to put the environment variable out of the \n> way once the script has been sourced. I can make two alternative \n> definitions of __git_list_all_commands () depending on the presence of \n> $AUTHORIZED_CMD_LIST if you are worried about performance.\n\nActually, it does not matter, as once __git_list_all_commands is\nrun, its result is kept in a variable.\n\nSo Peff's\n\n  if test -z \"$FAKE_COMMAND_LIST\"; then\n          __git_cmdlist() {\n                  git help -a | egrep '^  [a-zA-Z0-9]'\n          }\n  else\n          __git_cmdlist() {\n                  printf '%s' \"$FAKE_COMMAND_LIST\"\n          }\n  fi\n\nwould be just as simple if not simpler, does the same thing, and is\nsufficient, I think.\n\nThe t9902 test is only interested in making sure that the completion\nworks, and we do not want \"git help -a\" that omits a subcommand from\nits output that is not built in your particular environment to get\nin the way, which will not be an issue with this approach.\n"},{"id":"207730","messageId":"201301242356.23512.jn.avila@free.fr","threadId":"32717","inReplyTo":"7vlibi8baq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t9902: Instruct git-completion.bash about a test mode","fromName":"Jean-Noël AVILA","fromEmail":"jn.avila@free.fr","sentAt":"2013-01-24T22:56:23Z","receivedAt":"2013-01-24T22:56:23Z","isPatch":true,"sender":{"key":"jn.avila@free.fr","avatar":"https://avatars.githubusercontent.com/u/156172?v=4"},"body":"Le jeudi 24 janvier 2013 23:29:33, Junio C Hamano a écrit :\n> \"Jean-Noël AVILA\" <jn.avila@free.fr> writes:\n> \n> > My rational was to be sure to put the environment variable out of the \n> > way once the script has been sourced. I can make two alternative \n> > definitions of __git_list_all_commands () depending on the presence of \n> > $AUTHORIZED_CMD_LIST if you are worried about performance.\n> \n> Actually, it does not matter, as once __git_list_all_commands is\n> run, its result is kept in a variable.\n> \n> So Peff's\n> \n>   if test -z \"$FAKE_COMMAND_LIST\"; then\n>           __git_cmdlist() {\n>                   git help -a | egrep '^  [a-zA-Z0-9]'\n>           }\n>   else\n>           __git_cmdlist() {\n>                   printf '%s' \"$FAKE_COMMAND_LIST\"\n>           }\n>   fi\n> \n> would be just as simple if not simpler, does the same thing, and is\n> sufficient, I think.\n> \n> The t9902 test is only interested in making sure that the completion\n> works, and we do not want \"git help -a\" that omits a subcommand from\n> its output that is not built in your particular environment to get\n> in the way, which will not be an issue with this approach.\n> \n\n\nAh. I totally missed the point. I though that the requested change was\nto intersect the list needed for the test with the one provided by the \nstandard completion.\n"}]}