Improve instructions around how to set git-prompt preference variables. #255
Conversation
Welcome to GitGitGadgetHi @ghedsouza, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests. Please make sure that this Pull Request has a good description, as it will be used as cover letter. Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:
It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code. Contributing the patchesBefore you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a PR comment of the form Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions. If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox ("raw") file corresponding to the mail you want to reply to from the Git mailing list. If you use GMail, you can upload that raw mbox file via: curl -g --user "<EMailAddress>:<Password>" --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt |
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. Signed-off-by: Edward D'Souza <edsouza@gmail.com>
|
/allow ghedsouza |
|
User ghedsouza is now allowed to use GitGitGadget. |
|
May I suggest to use a broader PR description? This will be sent as cover letter, and it would make sense to describe more of the background of this contribution (e.g. that it was a PR on git/git first, how you ran into this, that you're a new contributor ;-)) instead of repeating the commit message. |
ghedsouza
changed the title
|
@dscho Thank you. I've updated the PR body and linked back to the original PR. |
|
/submit |
|
Error: User ghedsouza is not permitted to use GitGitGadget |
|
@dscho Hmm, looks like I should be allowed to use the submit command (based on the first few comments in this thread where you used the allow command), but the bot says I'm not. Would you have any idea what's wrong? |
|
/allow ghedsouza |
|
User ghedsouza is now allowed to use GitGitGadget. |
@ghedsouza my mistake: I added a space by mistake, and GitGitGadget does not (yet) validate the user name... |
|
/submit |
|
Submitted as pull.255.git.gitgitgadget@gmail.com |
|
@ghedsouza your |
|
@dscho Cool, thanks again! |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -35,6 +35,11 @@ | |||
| # | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
>
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
it got rejected from
git@vger.kernel.orgbecause 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.
|
Closing as per reply above. |
When I first tried to use the
git-prompt.shscript, I followed the instructions at the top of the file and everything worked, except for the "GIT_PS1_SHOWDIRTYSTATE" preference. Even though I had it set to true, I wasn't seeing the "(*)" and "(+)" in my shell prompt. After a few hours of poking around, I finally discovered what I had done wrong: I was setting the variable after the "source ~/.git-prompt.sh" line, and it has to be set before.I moved the lines around my .bashrc script and got everything working, but I thought the instructions could be a bit more explicit about how to set these preference variables, especially because this is a silent failure situation; the preference doesn't work if you get the order wrong and there are no warnings or errors to guide you.
This patch started as a PR on the official Github project two years ago (git#425), back when I didn't know the official process for submitting patches. Luckily, a kind user (@dscho) saw it and pointed me in the right direction to get it submitted.
This will be my first time submitting a patch, so hopefully I've figured out the process.