{"thread":{"id":"38286","subject":"[PATCH] test-lib.sh: do tests for color support after changing HOME","startedAt":"2015-01-05T18:54:14Z","lastAt":"2015-01-06T22:57:51Z","messageCount":5,"participants":["Richard Hansen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"254299","messageId":"1420484054-15948-1-git-send-email-rhansen@bbn.com","threadId":"38286","inReplyTo":null,"subject":"[PATCH] test-lib.sh: do tests for color support after changing HOME","fromName":"Richard Hansen","fromEmail":"rhansen@bbn.com","sentAt":"2015-01-05T18:54:14Z","receivedAt":"2015-01-05T18:54:14Z","isPatch":true,"sender":{"key":"rhansen@rhansen.org","avatar":null},"body":"If ncurses needs ~/.terminfo for the current $TERM, then tput will\nsucceed before changing HOME to $TRASH_DIRECTORY but fail afterward.\nMove the tests that determine whether there is color support after\nchanging HOME so that color=t is set if and only if tput would succeed\nwhen say_color() is run.\n\nThis disables color support for those that need ~/.terminfo for their\nTERM, but it's better than filling the screen with:\n\n    tput: unknown terminal \"custom-terminal-name-here\"\n\nAn alternative would be to symlink or copy the user's terminfo\ndatabase into $TRASH_DIRECTORY, but this is tricky due to the lack of\na standard name for the terminfo database (for example, instead of a\n~/.terminfo directory, NetBSD uses a ~/.terminfo.cdb database file).\n\nSigned-off-by: Richard Hansen <rhansen@bbn.com>\n---\n t/test-lib.sh | 90 +++++++++++++++++++++++++++++++----------------------------\n 1 file changed, 47 insertions(+), 43 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 9acdc88..65ecbed 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -184,16 +184,8 @@ export _x05 _x40 _z40 LF u200c\n # This test checks if command xyzzy does the right thing...\n # '\n # . ./test-lib.sh\n-[ \"x$ORIGINAL_TERM\" != \"xdumb\" ] && (\n-\t\tTERM=$ORIGINAL_TERM &&\n-\t\texport TERM &&\n-\t\t[ -t 1 ] &&\n-\t\ttput bold >/dev/null 2>&1 &&\n-\t\ttput setaf 1 >/dev/null 2>&1 &&\n-\t\ttput sgr0 >/dev/null 2>&1\n-\t) &&\n-\tcolor=t\n \n+unset color\n while test \"$#\" -ne 0\n do\n \tcase \"$1\" in\n@@ -258,40 +250,6 @@ then\n \tverbose=t\n fi\n \n-if test -n \"$color\"\n-then\n-\tsay_color () {\n-\t\t(\n-\t\tTERM=$ORIGINAL_TERM\n-\t\texport TERM\n-\t\tcase \"$1\" in\n-\t\terror)\n-\t\t\ttput bold; tput setaf 1;; # bold red\n-\t\tskip)\n-\t\t\ttput setaf 4;; # blue\n-\t\twarn)\n-\t\t\ttput setaf 3;; # brown/yellow\n-\t\tpass)\n-\t\t\ttput setaf 2;; # green\n-\t\tinfo)\n-\t\t\ttput setaf 6;; # cyan\n-\t\t*)\n-\t\t\ttest -n \"$quiet\" && return;;\n-\t\tesac\n-\t\tshift\n-\t\tprintf \"%s\" \"$*\"\n-\t\ttput sgr0\n-\t\techo\n-\t\t)\n-\t}\n-else\n-\tsay_color() {\n-\t\ttest -z \"$1\" && test -n \"$quiet\" && return\n-\t\tshift\n-\t\tprintf \"%s\\n\" \"$*\"\n-\t}\n-fi\n-\n error () {\n \tsay_color error \"error: $*\"\n \tGIT_EXIT_OK=t\n@@ -857,6 +815,52 @@ HOME=\"$TRASH_DIRECTORY\"\n GNUPGHOME=\"$HOME/gnupg-home-not-used\"\n export HOME GNUPGHOME\n \n+# run the tput tests *after* changing HOME (in case ncurses needs\n+# ~/.terminfo for $TERM)\n+test -n \"${color+set}\" || [ \"x$ORIGINAL_TERM\" != \"xdumb\" ] && (\n+\t\tTERM=$ORIGINAL_TERM &&\n+\t\texport TERM &&\n+\t\t[ -t 1 ] &&\n+\t\ttput bold >/dev/null 2>&1 &&\n+\t\ttput setaf 1 >/dev/null 2>&1 &&\n+\t\ttput sgr0 >/dev/null 2>&1\n+\t) &&\n+\tcolor=t\n+\n+if test -n \"$color\"\n+then\n+\tsay_color () {\n+\t\t(\n+\t\tTERM=$ORIGINAL_TERM\n+\t\texport TERM\n+\t\tcase \"$1\" in\n+\t\terror)\n+\t\t\ttput bold; tput setaf 1;; # bold red\n+\t\tskip)\n+\t\t\ttput setaf 4;; # blue\n+\t\twarn)\n+\t\t\ttput setaf 3;; # brown/yellow\n+\t\tpass)\n+\t\t\ttput setaf 2;; # green\n+\t\tinfo)\n+\t\t\ttput setaf 6;; # cyan\n+\t\t*)\n+\t\t\ttest -n \"$quiet\" && return;;\n+\t\tesac\n+\t\tshift\n+\t\tprintf \"%s\" \"$*\"\n+\t\ttput sgr0\n+\t\techo\n+\t\t)\n+\t}\n+else\n+\tsay_color() {\n+\t\ttest -z \"$1\" && test -n \"$quiet\" && return\n+\t\tshift\n+\t\tprintf \"%s\\n\" \"$*\"\n+\t}\n+fi\n+\n if test -z \"$TEST_NO_CREATE_REPO\"\n then\n \ttest_create_repo \"$TRASH_DIRECTORY\"\n-- \n2.2.1\n"},{"id":"254357","messageId":"xmqq8uhfpw3s.fsf@gitster.dls.corp.google.com","threadId":"38286","inReplyTo":"1420484054-15948-1-git-send-email-rhansen@bbn.com","subject":"Re: [PATCH] test-lib.sh: do tests for color support after changing HOME","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-06T19:06:31Z","receivedAt":"2015-01-06T19:06:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Richard Hansen <rhansen@bbn.com> writes:\n\n> If ncurses needs ~/.terminfo for the current $TERM, then tput will\n> succeed before changing HOME to $TRASH_DIRECTORY but fail afterward.\n> Move the tests that determine whether there is color support after\n> changing HOME so that color=t is set if and only if tput would succeed\n> when say_color() is run.\n>\n> This disables color support for those that need ~/.terminfo for their\n> TERM, but it's better than filling the screen with:\n>\n>     tput: unknown terminal \"custom-terminal-name-here\"\n>\n> An alternative would be to symlink or copy the user's terminfo\n> database into $TRASH_DIRECTORY, but this is tricky due to the lack of\n> a standard name for the terminfo database (for example, instead of a\n> ~/.terminfo directory, NetBSD uses a ~/.terminfo.cdb database file).\n\nSounds like a very sensible design trade-off.\n\n> +unset color\n>  while test \"$#\" -ne 0\n>  do\n>  \tcase \"$1\" in\n> @@ -258,40 +250,6 @@ then\n>  \tverbose=t\n>  fi\n>  \n> -if test -n \"$color\"\n> ...\n> @@ -857,6 +815,52 @@ HOME=\"$TRASH_DIRECTORY\"\n>  GNUPGHOME=\"$HOME/gnupg-home-not-used\"\n>  export HOME GNUPGHOME\n>  \n> +# run the tput tests *after* changing HOME (in case ncurses needs\n> +# ~/.terminfo for $TERM)\n> +test -n \"${color+set}\" || [ \"x$ORIGINAL_TERM\" != \"xdumb\" ] && (\n\nOK, $color used to be boolean between '' (unset included) and 't',\nbut now we do this after possibly processing the --no-color\nargument, so this is guarded slightly differently from the original.\n\nMakes sense.\n\n> +\t\tTERM=$ORIGINAL_TERM &&\n> +\t\texport TERM &&\n> +\t\t[ -t 1 ] &&\n> +\t\ttput bold >/dev/null 2>&1 &&\n> +\t\ttput setaf 1 >/dev/null 2>&1 &&\n> +\t\ttput sgr0 >/dev/null 2>&1\n> +\t) &&\n\nThanks.\n\nThis is a tangent but this patch shows 2 places out of the only\nthree places we use [ ... ] construct (as opposed to a more\ntraditionalist \"test\").  Perhaps we may want to fix them with a\nfollow-up patch?\n\n> +\tcolor=t\n> +\n> +if test -n \"$color\"\n> +then\n> +\tsay_color () {\n> +\t\t(\n> +\t\tTERM=$ORIGINAL_TERM\n> +\t\texport TERM\n> +\t\tcase \"$1\" in\n> +\t\terror)\n> +\t\t\ttput bold; tput setaf 1;; # bold red\n> +\t\tskip)\n> +\t\t\ttput setaf 4;; # blue\n> +\t\twarn)\n> +\t\t\ttput setaf 3;; # brown/yellow\n> +\t\tpass)\n> +\t\t\ttput setaf 2;; # green\n> +\t\tinfo)\n> +\t\t\ttput setaf 6;; # cyan\n> +\t\t*)\n> +\t\t\ttest -n \"$quiet\" && return;;\n> +\t\tesac\n> +\t\tshift\n> +\t\tprintf \"%s\" \"$*\"\n> +\t\ttput sgr0\n> +\t\techo\n> +\t\t)\n> +\t}\n> +else\n> +\tsay_color() {\n> +\t\ttest -z \"$1\" && test -n \"$quiet\" && return\n> +\t\tshift\n> +\t\tprintf \"%s\\n\" \"$*\"\n> +\t}\n> +fi\n> +\n>  if test -z \"$TEST_NO_CREATE_REPO\"\n>  then\n>  \ttest_create_repo \"$TRASH_DIRECTORY\"\n"},{"id":"254375","messageId":"1420585071-28973-1-git-send-email-rhansen@bbn.com","threadId":"38286","inReplyTo":"xmqq8uhfpw3s.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2 0/2] test-lib.sh: do tests for color support after changing HOME","fromName":"Richard Hansen","fromEmail":"rhansen@bbn.com","sentAt":"2015-01-06T22:57:49Z","receivedAt":"2015-01-06T22:57:49Z","isPatch":true,"sender":{"key":"rhansen@rhansen.org","avatar":null},"body":"On 2015-01-06T11:06-08:00, Junio C Hamano wrote:\n>> +unset color\n>>  while test \"$#\" -ne 0\n>>  do\n>>  \tcase \"$1\" in\n>> @@ -258,40 +250,6 @@ then\n>>  \tverbose=t\n>>  fi\n>>  \n>> -if test -n \"$color\"\n>> ...\n>> @@ -857,6 +815,52 @@ HOME=\"$TRASH_DIRECTORY\"\n>>  GNUPGHOME=\"$HOME/gnupg-home-not-used\"\n>>  export HOME GNUPGHOME\n>>  \n>> +# run the tput tests *after* changing HOME (in case ncurses needs\n>> +# ~/.terminfo for $TERM)\n>> +test -n \"${color+set}\" || [ \"x$ORIGINAL_TERM\" != \"xdumb\" ] && (\n>\n> OK, $color used to be boolean between '' (unset included) and 't',\n> but now we do this after possibly processing the --no-color\n> argument, so this is guarded slightly differently from the original.\n>\n> Makes sense.\n\nI updated the commit message to make this change more obvious.\n\n> This is a tangent but this patch shows 2 places out of the only\n> three places we use [ ... ] construct (as opposed to a more\n> traditionalist \"test\").  Perhaps we may want to fix them with a\n> follow-up patch?\n\nI added a prequel patch to address this.\n\nThank you for taking a look,\nRichard\n\n\nRichard Hansen (2):\n  use 'test ...' instead of '[ ... ]'\n  test-lib.sh: do tests for color support after changing HOME\n\n t/test-lib.sh | 92 +++++++++++++++++++++++++++++++----------------------------\n 1 file changed, 48 insertions(+), 44 deletions(-)\n\n-- \n2.2.1\n"},{"id":"254376","messageId":"1420585071-28973-2-git-send-email-rhansen@bbn.com","threadId":"38286","inReplyTo":"1420585071-28973-1-git-send-email-rhansen@bbn.com","subject":"[PATCH v2 1/2] use 'test ...' instead of '[ ... ]'","fromName":"Richard Hansen","fromEmail":"rhansen@bbn.com","sentAt":"2015-01-06T22:57:50Z","receivedAt":"2015-01-06T22:57:50Z","isPatch":true,"sender":{"key":"rhansen@rhansen.org","avatar":null},"body":"(see Documentation/CodingGuidelines)\n\nSigned-off-by: Richard Hansen <rhansen@bbn.com>\n---\n t/test-lib.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 9acdc88..3670eed 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -184,10 +184,10 @@ export _x05 _x40 _z40 LF u200c\n # This test checks if command xyzzy does the right thing...\n # '\n # . ./test-lib.sh\n-[ \"x$ORIGINAL_TERM\" != \"xdumb\" ] && (\n+test \"x$ORIGINAL_TERM\" != \"xdumb\" && (\n \t\tTERM=$ORIGINAL_TERM &&\n \t\texport TERM &&\n-\t\t[ -t 1 ] &&\n+\t\ttest -t 1 &&\n \t\ttput bold >/dev/null 2>&1 &&\n \t\ttput setaf 1 >/dev/null 2>&1 &&\n \t\ttput sgr0 >/dev/null 2>&1\n@@ -684,7 +684,7 @@ test_done () {\n \t\tthen\n \t\t\terror \"Can't use skip_all after running some tests\"\n \t\tfi\n-\t\t[ -z \"$skip_all\" ] || skip_all=\" # SKIP $skip_all\"\n+\t\ttest -z \"$skip_all\" || skip_all=\" # SKIP $skip_all\"\n \n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n-- \n2.2.1\n"},{"id":"254377","messageId":"1420585071-28973-3-git-send-email-rhansen@bbn.com","threadId":"38286","inReplyTo":"1420585071-28973-1-git-send-email-rhansen@bbn.com","subject":"[PATCH v2 2/2] test-lib.sh: do tests for color support after changing HOME","fromName":"Richard Hansen","fromEmail":"rhansen@bbn.com","sentAt":"2015-01-06T22:57:51Z","receivedAt":"2015-01-06T22:57:51Z","isPatch":true,"sender":{"key":"rhansen@rhansen.org","avatar":null},"body":"If ncurses needs ~/.terminfo for the current $TERM, then tput will\nsucceed before changing HOME to $TRASH_DIRECTORY but fail afterward.\nMove the tests that determine whether there is color support after\nchanging HOME so that color=t is set if and only if tput would succeed\nwhen say_color() is run.\n\nNote that color=t is now set after --no-color is processed, so the\ncondition to set color=t has changed:  it is now set only if\ncolor has not already been set to the empty string by --no-color.\n\nThis commit disables color support for those that need ~/.terminfo for\ntheir TERM, but it's better than filling the screen with:\n\n    tput: unknown terminal \"custom-terminal-name-here\"\n\nAn alternative would be to symlink or copy the user's terminfo\ndatabase into $TRASH_DIRECTORY, but this is tricky due to the lack of\na standard name for the terminfo database (for example, instead of a\n~/.terminfo directory, NetBSD uses a ~/.terminfo.cdb database file).\n\nSigned-off-by: Richard Hansen <rhansen@bbn.com>\n---\n t/test-lib.sh | 90 +++++++++++++++++++++++++++++++----------------------------\n 1 file changed, 47 insertions(+), 43 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 3670eed..bb1402d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -184,16 +184,8 @@ export _x05 _x40 _z40 LF u200c\n # This test checks if command xyzzy does the right thing...\n # '\n # . ./test-lib.sh\n-test \"x$ORIGINAL_TERM\" != \"xdumb\" && (\n-\t\tTERM=$ORIGINAL_TERM &&\n-\t\texport TERM &&\n-\t\ttest -t 1 &&\n-\t\ttput bold >/dev/null 2>&1 &&\n-\t\ttput setaf 1 >/dev/null 2>&1 &&\n-\t\ttput sgr0 >/dev/null 2>&1\n-\t) &&\n-\tcolor=t\n \n+unset color\n while test \"$#\" -ne 0\n do\n \tcase \"$1\" in\n@@ -258,40 +250,6 @@ then\n \tverbose=t\n fi\n \n-if test -n \"$color\"\n-then\n-\tsay_color () {\n-\t\t(\n-\t\tTERM=$ORIGINAL_TERM\n-\t\texport TERM\n-\t\tcase \"$1\" in\n-\t\terror)\n-\t\t\ttput bold; tput setaf 1;; # bold red\n-\t\tskip)\n-\t\t\ttput setaf 4;; # blue\n-\t\twarn)\n-\t\t\ttput setaf 3;; # brown/yellow\n-\t\tpass)\n-\t\t\ttput setaf 2;; # green\n-\t\tinfo)\n-\t\t\ttput setaf 6;; # cyan\n-\t\t*)\n-\t\t\ttest -n \"$quiet\" && return;;\n-\t\tesac\n-\t\tshift\n-\t\tprintf \"%s\" \"$*\"\n-\t\ttput sgr0\n-\t\techo\n-\t\t)\n-\t}\n-else\n-\tsay_color() {\n-\t\ttest -z \"$1\" && test -n \"$quiet\" && return\n-\t\tshift\n-\t\tprintf \"%s\\n\" \"$*\"\n-\t}\n-fi\n-\n error () {\n \tsay_color error \"error: $*\"\n \tGIT_EXIT_OK=t\n@@ -857,6 +815,52 @@ HOME=\"$TRASH_DIRECTORY\"\n GNUPGHOME=\"$HOME/gnupg-home-not-used\"\n export HOME GNUPGHOME\n \n+# run the tput tests *after* changing HOME (in case ncurses needs\n+# ~/.terminfo for $TERM)\n+test -n \"${color+set}\" || test \"x$ORIGINAL_TERM\" != \"xdumb\" && (\n+\t\tTERM=$ORIGINAL_TERM &&\n+\t\texport TERM &&\n+\t\ttest -t 1 &&\n+\t\ttput bold >/dev/null 2>&1 &&\n+\t\ttput setaf 1 >/dev/null 2>&1 &&\n+\t\ttput sgr0 >/dev/null 2>&1\n+\t) &&\n+\tcolor=t\n+\n+if test -n \"$color\"\n+then\n+\tsay_color () {\n+\t\t(\n+\t\tTERM=$ORIGINAL_TERM\n+\t\texport TERM\n+\t\tcase \"$1\" in\n+\t\terror)\n+\t\t\ttput bold; tput setaf 1;; # bold red\n+\t\tskip)\n+\t\t\ttput setaf 4;; # blue\n+\t\twarn)\n+\t\t\ttput setaf 3;; # brown/yellow\n+\t\tpass)\n+\t\t\ttput setaf 2;; # green\n+\t\tinfo)\n+\t\t\ttput setaf 6;; # cyan\n+\t\t*)\n+\t\t\ttest -n \"$quiet\" && return;;\n+\t\tesac\n+\t\tshift\n+\t\tprintf \"%s\" \"$*\"\n+\t\ttput sgr0\n+\t\techo\n+\t\t)\n+\t}\n+else\n+\tsay_color() {\n+\t\ttest -z \"$1\" && test -n \"$quiet\" && return\n+\t\tshift\n+\t\tprintf \"%s\\n\" \"$*\"\n+\t}\n+fi\n+\n if test -z \"$TEST_NO_CREATE_REPO\"\n then\n \ttest_create_repo \"$TRASH_DIRECTORY\"\n-- \n2.2.1\n"}]}