Skip to content

Improve instructions around how to set git-prompt preference variables. #255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Improve instructions around how to set git-prompt preference variables. #255

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions contrib/completion/git-prompt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@
#
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, SZEDER Gábor wrote (reply to this):

On Wed, Jun 05, 2019 at 08:06:07AM -0700, Edward D'Souza via GitGitGadget wrote:
> From: Edward D'Souza <edsouza@gmail.com>
> 
> Clarify the need to set variables like GIT_PS1_SHOWDIRTYSTATE before
> "source ~/.git-prompt.sh" is executed in your shell init process.
> 
> If you set these preferences too late i.e. after .git-prompt.sh executes,
> they will silently fail to take effect.

I can't reproduce this.  It doesn't matter when these variables are
set, because __git_ps1() checks them each time it is invoked, it
always has.

  $ echo $GIT_PS1_SHOWSTASHSTATE $GIT_PS1_SHOWDIRTYSTATE $GIT_PS1_SHOWUNTRACKEDFILES
  
  /tmp/repo$ git init
  Initialized empty Git repository in /tmp/repo/.git/
  /tmp/repo (master)$ echo 1 >file
  /tmp/repo (master)$ git add file
  /tmp/repo (master)$ git commit -q -m initial
  /tmp/repo (master)$ echo 2 >file
  /tmp/repo (master)$ git stash 
  Saved working directory and index state WIP on master: 5ae0413 initial
  /tmp/repo (master)$ echo 3 >file
  /tmp/repo (master)$ git add file
  /tmp/repo (master)$ echo 4 >file
  /tmp/repo (master)$ >untracked
  /tmp/repo (master)$ GIT_PS1_SHOWSTASHSTATE=y
  /tmp/repo (master $)$ GIT_PS1_SHOWDIRTYSTATE=y
  /tmp/repo (master *+$)$ GIT_PS1_SHOWUNTRACKEDFILES=y
  /tmp/repo (master *+$%)$ unset GIT_PS1_SHOWSTASHSTATE GIT_PS1_SHOWDIRTYSTATE GIT_PS1_SHOWUNTRACKEDFILES
  /tmp/repo (master)$ 

Note that some of these status indicators are controlled not only by
environment variables but by corresponding 'bash.<indicator>' config
variables as well.  Even if the env var is set to enable the status
indicator globally, the config setting can still override that to
allow disabling potentially expensive indicators on a per-repo basis.
Is it possible that you had e.g. 'bash.showDirtyState = false' in your
config somewhere?

Anyway, even if the issue were real, this patch goes in the wrong
direction: instead of requiring a workaround from users, we should
rather fix the issue.

> Signed-off-by: Edward D'Souza <edsouza@gmail.com>
> ---
>  contrib/completion/git-prompt.sh | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index c6cbef38c2..ab5bcc0fec 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -35,6 +35,11 @@
>  #
>  # The prompt status always includes the current branch name.
>  #
> +# The prompt can be customized by setting various shell variables
> +# (GIT_PS1_SHOWDIRTYSTATE, GIT_PS1_SHOWSTASHSTATE, etc.), which are described
> +# below. Make sure that these variables get set *before* the
> +# "source ~/.git-prompt.sh" line from step 2 (above) runs.
> +#
>  # In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty value,
>  # unstaged (*) and staged (+) changes will be shown next to the branch
>  # name.  You can configure this per-repository with the
> -- 
> gitgitgadget

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't reproduce this.

@ghedsouza can you? If so, maybe you can reply to the mail (you can follow this advice to reply even if you are not subscribed to the mailing list).

And even if you cannot reproduce this anymore, it would be nice to close the circle and reply to the reviewer comment.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, "Edward D'Souza" wrote (reply to this):

Confirming that I can't reproduce this either. Should have done that
first before continuing with a PR from two years ago!

On Wed, Jun 5, 2019 at 12:07 PM SZEDER G=C3=A1bor <szeder.dev@gmail.com> wr=
ote:
>
> On Wed, Jun 05, 2019 at 08:06:07AM -0700, Edward D'Souza via GitGitGadget=
 wrote:
> > From: Edward D'Souza <edsouza@gmail.com>
> >
> > Clarify the need to set variables like GIT_PS1_SHOWDIRTYSTATE before
> > "source ~/.git-prompt.sh" is executed in your shell init process.
> >
> > If you set these preferences too late i.e. after .git-prompt.sh execute=
s,
> > they will silently fail to take effect.
>
> I can't reproduce this.  It doesn't matter when these variables are
> set, because __git_ps1() checks them each time it is invoked, it
> always has.
>
>   $ echo $GIT_PS1_SHOWSTASHSTATE $GIT_PS1_SHOWDIRTYSTATE $GIT_PS1_SHOWUNT=
RACKEDFILES
>
>   /tmp/repo$ git init
>   Initialized empty Git repository in /tmp/repo/.git/
>   /tmp/repo (master)$ echo 1 >file
>   /tmp/repo (master)$ git add file
>   /tmp/repo (master)$ git commit -q -m initial
>   /tmp/repo (master)$ echo 2 >file
>   /tmp/repo (master)$ git stash
>   Saved working directory and index state WIP on master: 5ae0413 initial
>   /tmp/repo (master)$ echo 3 >file
>   /tmp/repo (master)$ git add file
>   /tmp/repo (master)$ echo 4 >file
>   /tmp/repo (master)$ >untracked
>   /tmp/repo (master)$ GIT_PS1_SHOWSTASHSTATE=3Dy
>   /tmp/repo (master $)$ GIT_PS1_SHOWDIRTYSTATE=3Dy
>   /tmp/repo (master *+$)$ GIT_PS1_SHOWUNTRACKEDFILES=3Dy
>   /tmp/repo (master *+$%)$ unset GIT_PS1_SHOWSTASHSTATE GIT_PS1_SHOWDIRTY=
STATE GIT_PS1_SHOWUNTRACKEDFILES
>   /tmp/repo (master)$
>
> Note that some of these status indicators are controlled not only by
> environment variables but by corresponding 'bash.<indicator>' config
> variables as well.  Even if the env var is set to enable the status
> indicator globally, the config setting can still override that to
> allow disabling potentially expensive indicators on a per-repo basis.
> Is it possible that you had e.g. 'bash.showDirtyState =3D false' in your
> config somewhere?
>
> Anyway, even if the issue were real, this patch goes in the wrong
> direction: instead of requiring a workaround from users, we should
> rather fix the issue.
>
> > Signed-off-by: Edward D'Souza <edsouza@gmail.com>
> > ---
> >  contrib/completion/git-prompt.sh | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-=
prompt.sh
> > index c6cbef38c2..ab5bcc0fec 100644
> > --- a/contrib/completion/git-prompt.sh
> > +++ b/contrib/completion/git-prompt.sh
> > @@ -35,6 +35,11 @@
> >  #
> >  # The prompt status always includes the current branch name.
> >  #
> > +# The prompt can be customized by setting various shell variables
> > +# (GIT_PS1_SHOWDIRTYSTATE, GIT_PS1_SHOWSTASHSTATE, etc.), which are de=
scribed
> > +# below. Make sure that these variables get set *before* the
> > +# "source ~/.git-prompt.sh" line from step 2 (above) runs.
> > +#
> >  # In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty value,
> >  # unstaged (*) and staged (+) changes will be shown next to the branch
> >  # name.  You can configure this per-repository with the
> > --
> > gitgitgadget
>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dscho No, I can't reproduce it. I tried to reply to the email through gmail, but it got rejected from git@vger.kernel.org because it had HTML content in it.

I just tried again in plain-text mode. It looks like it worked because I see it on the mailing list mirror.

Hopefully my next patch request is little more well thought out :p

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it got rejected from git@vger.kernel.org because it had HTML content in it.

Right, I tried to make sure that this is mentioned in https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis. Maybe you can improve that page? It's a wiki.

I just tried again in plain-text mode. It looks like it worked because I see it on the mailing list mirror.

Yep, I got it, too (I'm subscribed).

Hopefully my next patch request is little more well thought out :p

No worries. I'd rather err on the side of having one contribution too many than one too few.

# The prompt status always includes the current branch name.
#
# The prompt can be customized by setting various shell variables
# (GIT_PS1_SHOWDIRTYSTATE, GIT_PS1_SHOWSTASHSTATE, etc.), which are described
# below. Make sure that these variables get set *before* the
# "source ~/.git-prompt.sh" line from step 2 (above) runs.
#
# In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty value,
# unstaged (*) and staged (+) changes will be shown next to the branch
# name. You can configure this per-repository with the
Expand Down