From: Junio C Hamano <gitster@pobox.com>
To: Richard Hansen <rhansen@bbn.com>
Cc: git@vger.kernel.org, caleb@calebthompson.io
Subject: Re: [PATCH] git-prompt.sh: don't assume the shell expands the value of PS1
Date: Tue, 20 May 2014 11:38:17 -0700 [thread overview]
Message-ID: <xmqqbnusz46e.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1400540137-29994-1-git-send-email-rhansen@bbn.com> (Richard Hansen's message of "Mon, 19 May 2014 18:55:37 -0400")
Richard Hansen <rhansen@bbn.com> writes:
> Not all shells subject the prompt string to parameter expansion. Test
> whether the shell will expand the value of PS1, and use the result to
> control whether raw ref names are included directly in PS1.
>
> This fixes a regression introduced in commit 8976500 ("git-prompt.sh:
> don't put unsanitized branch names in $PS1"): zsh does not expand PS1
> by default, but that commit assumed it did. The bug resulted in
> prompts containing the literal string '${__git_ps1_branch_name}'
> instead of the actual branch name.
>
> Reported-by: Caleb Thompson <caleb@calebthompson.io>
> Signed-off-by: Richard Hansen <rhansen@bbn.com>
> ---
Thanks, applied.
> To prevent a regression like this from happening again, I plan on
> adding new zsh test cases and expanding the bash test cases (to test
> the behavior with 'shopt -u promptvars'). I'd like the zsh tests to
> cover the same stuff as the bash tests. These are the steps I am
> considering:
>
> 1. delete the last test case in t9903 ("prompt - zsh color pc mode")
> 2. add two new functions to t/lib-bash.sh:
> ps1_expansion_enable () { shopt -s promptvars; }
> ps1_expansion_disable () { shopt -u promptvars; }
> 3. loop over the relevant test cases twice: once after calling
> ps1_expansion_enable and once after calling ps1_expansion_disable
> (with appropriate adjustments to the expected output)
> 4. move the test cases in t9903 to a separate library file and
> source it from t9903-bash-prompt.sh
> 5. create two new files:
> * t/lib-zsh.sh (same as t/lib-bash.sh but tweaked for zsh)
> * t/t9904-zsh-prompt.sh (same as t/t9903-bash-prompt.sh but
> tweaked for zsh)
>
> Does this approach sound reasonable?
Sounds like a plan, especially if step 4 does a reasonable job of
factoring out as much common stuff as possible.
next prev parent reply other threads:[~2014-05-20 18:39 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-19 22:55 [PATCH] git-prompt.sh: don't assume the shell expands the value of PS1 Richard Hansen
2014-05-20 18:38 ` Junio C Hamano [this message]
2014-05-27 7:40 ` [PATCH 00/10] Zsh prompt tests Richard Hansen
2014-05-27 7:40 ` [PATCH 01/10] t9903: remove Zsh test from the suite of Bash " Richard Hansen
2014-05-27 7:40 ` [PATCH 02/10] t9903: put the Bash pc mode prompt test cases in a function Richard Hansen
2014-05-27 7:40 ` [PATCH 03/10] t9903: move test name prefix to a separate variable Richard Hansen
2014-05-27 7:40 ` [PATCH 04/10] t9903: run pc mode tests again with PS1 expansion disabled Richard Hansen
2014-05-27 7:40 ` [PATCH 05/10] t9903: include "Bash" in test names via new $shellname var Richard Hansen
2014-05-27 7:40 ` [PATCH 06/10] t9903: move PS1 color code variable definitions to lib-bash.sh Richard Hansen
2014-05-27 7:40 ` [PATCH 07/10] t9903: move prompt tests to a new lib-prompt-tests.sh file Richard Hansen
2014-05-27 7:40 ` [PATCH 08/10] lib-prompt-tests.sh: put all tests inside a function Richard Hansen
2014-05-27 7:40 ` [PATCH 09/10] lib-prompt-tests.sh: add variable for string that encodes percent in PS1 Richard Hansen
2014-05-27 7:41 ` [PATCH 10/10] t9904: new __git_ps1 tests for Zsh Richard Hansen
2014-05-29 19:02 ` Thomas Rast
2014-05-29 22:30 ` [PATCH 11/10] fixup! " Richard Hansen
2014-06-04 21:01 ` [PATCH v2 00/11] Zsh prompt tests Richard Hansen
2014-06-04 21:01 ` [PATCH v2 01/11] t9903: remove Zsh test from the suite of Bash " Richard Hansen
2014-06-04 21:01 ` [PATCH v2 02/11] t9903: put the Bash pc mode prompt test cases in a function Richard Hansen
2014-06-04 21:01 ` [PATCH v2 03/11] t9903: move test name prefix to a separate variable Richard Hansen
2014-06-04 21:01 ` [PATCH v2 04/11] t9903: run pc mode tests again with PS1 expansion disabled Richard Hansen
2014-06-04 21:01 ` [PATCH v2 05/11] t9903: include "Bash" in test names via new $shellname var Richard Hansen
2014-06-04 21:01 ` [PATCH v2 06/11] t9903: move PS1 color code variable definitions to lib-bash.sh Richard Hansen
2014-06-04 21:01 ` [PATCH v2 07/11] t9903: move prompt tests to a new lib-prompt-tests.sh file Richard Hansen
2014-06-04 21:01 ` [PATCH v2 08/11] lib-prompt-tests.sh: put all tests inside a function Richard Hansen
2014-06-04 21:01 ` [PATCH v2 09/11] lib-prompt-tests.sh: add variable for string that encodes percent in PS1 Richard Hansen
2014-06-04 21:01 ` [PATCH v2 10/11] test-lib: make it possible to override how test code is eval'd Richard Hansen
2014-06-05 21:11 ` Junio C Hamano
2014-06-06 1:00 ` Richard Hansen
2014-06-06 16:53 ` Junio C Hamano
2014-06-04 21:01 ` [PATCH v2 11/11] t9904: new __git_ps1 tests for Zsh Richard Hansen
2014-06-10 20:06 ` [PATCH v2 00/11] Zsh prompt tests Torsten Bögershausen
2014-06-10 20:28 ` Richard Hansen
2014-06-11 1:16 ` brian m. carlson
2014-06-11 15:27 ` Richard Hansen
2014-06-11 23:46 ` brian m. carlson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqbnusz46e.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=caleb@calebthompson.io \
--cc=git@vger.kernel.org \
--cc=rhansen@bbn.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.