On Mon, 2013-02-18 at 21:31 +0100, Simon vanaf Telefoon wrote: > Hi all, sorry for top posting :-( blame the phone and k9 > > I have a small issue with the use of test instead of [ > If that only applies to this section of the entire file. > Coding style has some value. > > Combining nested ifs with && seems harmless enough, though should be > well tested. > > Cheers > Simon > Ah, indeed, I looked around a bit more, and as per http://mywiki.wooledge.org/BashPitfalls it seems like 'test' is bad to use with multiple &&'s anyways. I've changed to using [] && [] and rerolled the patch. > Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > Hi Martin, > > Martin Erik Werner wrote: > > Minor clean up of if-then nesting in checks for environment variables > and config options. No functional changes. > > Yeah, the nesting was getting a little deep. Thanks for the cleanup. > May we have your sign-off? > > Once this is signed off, > Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx> Meh, I keep on missing that :/ Old (and new) patch is: Signed-off-by: Martin Erik Werner <martinerikwerner@xxxxxxxxx> > > Patch left unsnipped for reference. > > --- > contrib/completion/git-prompt.sh | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git > a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh > index 9b2eec2..e29694d 100644 > --- a/contrib/completion/git-prompt.sh > +++ b/contrib/completion/git-prompt.sh > @@ -320,26 +320,25 @@ __git_ps1 () > b="GIT_DIR!" > fi > elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then > - if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then > - if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then > - git diff --no-ext-diff --quiet --exit-code || w="*" > - if git rev-parse --quiet --verify HEAD >/dev/null; then > - git diff-index --cached --quiet HEAD -- || i="+" > - else > - i="#" > - fi > + if test -n "${GIT_PS1_SHOWDIRTYSTATE-}" && > + test "$(git config --bool bash.showDirtyState)" != > "false" > + then > + git diff --no-ext-diff --quiet --exit-code || w="*" > + if git rev-parse --quiet --verify HEAD >/dev/null; then > + git diff-index --cached --quiet HEAD -- || i="+" > + else > + i="#" > fi > fi > if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then > git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$" > fi > > - if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then > - if [ "$(git config --bool bash.showUntrackedFiles)" != "false" ]; then > - if [ -n "$(git ls-files --others --exclude-standard)" ]; then > - u="%" > - fi > - fi > + if test -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" && > + test "$(git config --bool bash.showUntrackedFiles)" != "false" && > + test -n "$(git ls-files --others --exclude-standard)" > + then > + u="%" > fi > > if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ];! > then > -- > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html