Skip to content
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.

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

Add Git stash support #462

Merged
merged 8 commits into from
Feb 14, 2020
Merged

Conversation

carhartl
Copy link
Contributor

This is adding support for git stashes. E.g. if there are any stashes show an icon, otherwise not (similar to how git arrows behave). Said icon is configurable via PURE_GIT_STASH_SYMBOL environment variable.

It's implemented utilizing vcs_info's misc message + hook for tracking information about stashes.

Please take this as a proposal I guess; while #365 is still open, I'm aware of the discussion in #317..

screenshot 2019-02-11 at 09 41 54

@sindresorhus
Copy link
Owner

If we decide to accept this, it should at minimum be opt-in using zstyle as outlined in (someone needs to implement that).

Alternatively, #460 could allow you do add this yourself without having to fork Pure.

@sindresorhus
Copy link
Owner

@mafredri Are you ok with adding this as opt-in through zstyle?

@mafredri
Copy link
Collaborator

@mafredri Are you ok with adding this as opt-in through zstyle?

I think that would be alright. So would we have an env variable for configuring the icon, and a zstyle for enabling the feature? Or should we take this opportunity to start defining icons in zstyle as well?

@sindresorhus
Copy link
Owner

Or should we take this opportunity to start defining icons in zstyle as well?

Yes

@sindresorhus
Copy link
Owner

@carhartl Can you make it use zstyle?

@carhartl
Copy link
Contributor Author

@sindresorhus Need to learn more about that but happy to try...

@carhartl
Copy link
Contributor Author

@sindresorhus So it‘s supposed to be implemented as @mafredri suggested:

zstyle for enabling the feature
zstyle for defining the icon

?

@carhartl
Copy link
Contributor Author

carhartl commented Dec 1, 2019

I think it would be better to migrate all of the icons to zstyle, not just the one for stashes. This seems out of scope of this PR and I'd like to do this in a follow-up PR.

@sindresorhus
Copy link
Owner

So it‘s supposed to be implemented as @mafredri suggested:
zstyle for enabling the feature
zstyle for defining the icon

👍

I think it would be better to migrate all of the icons to zstyle, not just the one for stashes. This seems out of scope of this PR and I'd like to do this in a follow-up PR.

👍

@carhartl
Copy link
Contributor Author

carhartl commented Dec 1, 2019

@sindresorhus

Showing the stash status does now require opt-in via zstyle ":prompt:pure:git:stash:show" enable yes. The stash symbol color is also zstyle'd.

Is there anything else needed for this to be merged?

@carhartl
Copy link
Contributor Author

carhartl commented Dec 1, 2019

Hmmm, might be nicer if we‘re not setting up the hook in the first place unless opted in!

readme.md Outdated

Showing git stash status as part of the prompt is not activated by default. To activate this you'll need to opt in via `zstyle`:

`zstyle ':prompt:pure:git:stash:show' enable yes`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Show and enable sounds a bit repetitive to me. Could it make sense to drop :show here? Or drop show and rename enable => show.

pure.zsh Outdated
# and stash information via misc (%m).
zstyle ':vcs_info:git*' formats '%b' '%R' '%a' '%m'
zstyle ':vcs_info:git*' actionformats '%b' '%R' '%a' '%m'
zstyle ':vcs_info:git*+set-message:*' hooks git-stash
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally I’d like us to avoid “doing the work” for gut stash, unless it’s enabled. But it’s not critical for this PR and we cam change it afterwards.

zstyle ':vcs_info:*' max-exports 4
# Export branch (%b), Git toplevel (%R), action (rebase/cherry-pick) (%a),
# and stash information via misc (%m).
zstyle ':vcs_info:git*' formats '%b' '%R' '%a' '%m'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I’m guessing there are no repercussions from including %a here?

Copy link
Contributor Author

@carhartl carhartl Dec 1, 2019

Choose a reason for hiding this comment

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

For some reason it was producing an error when formats didn't have the same amount as actionformats. I really don't understand why these two seem to interact, maybe I did something wrong elsewhere, still new to zsh internals.

EDIT: not an error, but something that looked like an error, but was just not the expected output.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, it’s been a while since I worked with vcs_info as well so just being cautious 😄. It’s probably fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Took a second look... %m was only required in formats only, but not in actionformats.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting. How about when a git rebase is active? That’s when action formats kick in AFAICR.

Copy link
Contributor Author

@carhartl carhartl Dec 1, 2019

Choose a reason for hiding this comment

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

Ah, the actionformats are for when git is performing a merge or rebase. If I understand all this correctly, if we were omitting %m from actionformats, info[stash]=$vcs_info_msg_3_ would remain empty (?) and the stash symbol would never be shown during such an action. Which is what we want?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we need to ensure that $vcs_info_msg_3_ would always point to the expected output...

@sindresorhus
Copy link
Owner

Can we make it zstyle ':prompt:pure:git:stash' enable yes? (Without the show)

@mafredri
Copy link
Collaborator

mafredri commented Dec 1, 2019

Looks like we all had a few of the same ideas 😅

@carhartl
Copy link
Contributor Author

carhartl commented Dec 1, 2019

Can we make it zstyle ':prompt:pure:git:stash' enable yes?

:prompt:pure:git:stash is used for the color already, to prevent that clash I thought I need to introduce the additional :show. So with @mafredri's suggestion, would

zstyle ':prompt:pure:git:stash:show' show yes

be ok? Or maybe zstyle ':prompt:pure:git' stash yes is more in line with the intended usage of zstyle?

@mafredri
Copy link
Collaborator

mafredri commented Dec 1, 2019

:prompt:pure:git:stash is used for the color already, to prevent that clash I thought I need to introduce the additional :show. So with @mafredri's suggestion, would

That’s ok because there can be multiple values under a zstyle. So :prompt:pure:git:stash should be able to store both color=x and show=y.

be ok? Or maybe zstyle ':prompt:pure:git' stash yes is more in line with the intended usage of zstyle?

This could also work. For me the former feels a bit more uniform but this is essentially a matter of enabling / disabling showing of git features so ¯\_(ツ)_/¯. I’m fine with either.

@carhartl
Copy link
Contributor Author

carhartl commented Dec 1, 2019

Ah, please bear with me... I missed to think of :prompt:pure:git:stash as some sort of namespace. Thanks for clarifying.

This changes everything :)

Then I also think we should go for :prompt:pure:git:stash, will allow to configure related things in the same way, as pointed out.

zstyle ':prompt:pure:git:stash' show yes
zstyle ':prompt:pure:git:stash' color yellow
zstyle ':prompt:pure:git:stash' icon S

Updating the commit...

pure.zsh Outdated Show resolved Hide resolved
@sindresorhus sindresorhus changed the title Git stash support Add Git stash support Dec 6, 2019
@carhartl
Copy link
Contributor Author

carhartl commented Dec 7, 2019

Is there anything you need me to add?

@fladson
Copy link

fladson commented Jan 7, 2020

I would love to see this merged, thanks for adding it.

@@ -70,7 +70,11 @@ prompt pure
| **`PURE_PROMPT_VICMD_SYMBOL`** | Defines the prompt symbol used when the `vicmd` keymap is active (VI-mode). | `❮` |
| **`PURE_GIT_DOWN_ARROW`** | Defines the git down arrow symbol. | `⇣` |
| **`PURE_GIT_UP_ARROW`** | Defines the git up arrow symbol. | `⇡` |
| **`PURE_GIT_STASH_SYMBOL`** | Defines the git stash symbol. | `≡` |
Copy link
Owner

Choose a reason for hiding this comment

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

As discussed in the comments, this should be zstyle ':prompt:pure:git:stash' icon S.

@sindresorhus
Copy link
Owner

@carhartl Sorry, this fell off my radar. There are some remaining feedback:

@carhartl
Copy link
Contributor Author

carhartl commented Feb 8, 2020

@sindresorhus

Oh, ok. Regarding the icon configuration I thought you agreed that this would be out of scope and would better be done for all icons instead of just this new one in a separate PR.

See #462 (comment)

@sindresorhus
Copy link
Owner

Forgot about my comment. Never mind the zconfig thing then.

@carhartl
Copy link
Contributor Author

carhartl commented Feb 8, 2020

Regarding the other issue: I had already changed it to

zstyle ':prompt:pure:git:stash' show yes

but it looks like I overlooked to adapt that in one place in the readme.

@carhartl
Copy link
Contributor Author

carhartl commented Feb 9, 2020

@sindresorhus I have fixed the issue in the readme...

@sindresorhus sindresorhus merged commit 70498a0 into sindresorhus:master Feb 14, 2020
@sindresorhus
Copy link
Owner

Looks good. Thanks for working on this 🙌

@sindresorhus sindresorhus mentioned this pull request Feb 14, 2020
@carhartl carhartl deleted the git-stash-support branch February 14, 2020 10:17
kutsan pushed a commit to kutsan/pure that referenced this pull request Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants