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

Modularize the git status to support customization #391

Closed
wants to merge 6 commits into from

Conversation

olets
Copy link
Contributor

@olets olets commented Mar 11, 2018

Modularizes the Git status section! This is discussed in #375.

Until now Spaceship has been wonderfully flexible except in the opinionated Git prompt. This PR breaks the Git prompt section into subsections analogous to the top-level sections in the full prompt, making it easy for users to customize their Git prompt and to build, share, plug in new Git prompt pieces.

Changes:

  • Reworks git.zsh to run build the Git section from an array of subsections
  • Moves the Git status options to their own file, so that they can be easily used in other user subsections
  • Documentation

Example usages:

Default:

SPACESHIP_GIT_ORDER=(
  branch
  status
)

Spaceship < v3

SPACESHIP_GIT_ORDER=(
  branch
  status_oh_my_zsh
)

Custom

SPACESHIP_GIT_ORDER=(
  my_custom_git_prompt
)

@salmanulfarzy salmanulfarzy added the new-feature A PR that implement feature (section, specific behavior, etc). label Mar 13, 2018
Copy link
Member

@salmanulfarzy salmanulfarzy left a comment

Choose a reason for hiding this comment

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

This is great @olets and sorry this look long to review.

docs/Options.md Outdated

#### Git subsections

The Git section consists of several subsections. You can specify the order of the subsections using `SPACESHIP_PROMPT_ORDER` option. Use Zsh array syntax to define your own prompt order.
Copy link
Member

Choose a reason for hiding this comment

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

git subsections using SPACESHIP_GIT_ORDER option ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

add "git"? 👍

docs/Options.md Outdated

The Git section consists of several subsections. You can specify the order of the subsections using `SPACESHIP_PROMPT_ORDER` option. Use Zsh array syntax to define your own prompt order.

The order also defines which sections that Spaceship loads. If you're struggling with slow prompt, you can just omit the sections that you don't use, and they won't be loaded.
Copy link
Member

Choose a reason for hiding this comment

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

It's already noted at Troubleshooting.md, So I don't think this necessary here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in ab40f38

sections/git.zsh Outdated

local git_branch="$(spaceship_git_branch)" git_status="$(spaceship_git_status)"
for el in ${(Oa)SPACESHIP_GIT_ORDER}; do
Copy link
Member

Choose a reason for hiding this comment

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

If I'm not mistaken this gives revers array order, right ? Would be great with a comment here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right. But now that I look at it again, I'll remove the (Oa) and flip line 36 to $git_prompt$(spaceship_git_${el})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in c973316

SPACESHIP_GIT_STATUS_UNTRACKED="${SPACESHIP_GIT_STATUS_UNTRACKED="?"}"
SPACESHIP_GIT_STATUS_ADDED="${SPACESHIP_GIT_STATUS_ADDED="+"}"
SPACESHIP_GIT_STATUS_MODIFIED="${SPACESHIP_GIT_STATUS_MODIFIED="!"}"
SPACESHIP_GIT_STATUS_RENAMED="${SPACESHIP_GIT_STATUS_RENAMED="»"}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see much sense in sharing configuration options between different git_status sections, not only the options will likely be different (e.g. in #359 I don't have the RENAMED option), but also different sections might choose to provide different default values (again, example is #359).

Copy link
Contributor Author

@olets olets Mar 31, 2018

Choose a reason for hiding this comment

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

I was actually thinking this would be useful for supporting things like #359 — you'd have something like

CUSTOM_GIT_SECTION_ADDED="${CUSTOM_GIT_SECTION_ADDED="$SPACESHIP_GIT_STATUS_ADDED"}"

git_status_options.zsh would be have the basics, and any section might use all or just some of them. That way your .zshrc could just have

SPACESHIP_GIT_STATUS_ADDED="my 'added' indicator"

instead of

SPACESHIP_GIT_STATUS_ADDED="my 'added' indicator"
CUSTOM_GIT_SECTION_ADDED="my 'added' indicator"

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, but you wouldn't use two sections at the same time, would you? You'd choose one of the sections, then look up its available options and what they do, and configure to your liking 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha true. My thought was this would make it easier for people to try out other git prompts — they could simply change the line in the git prompt array, without changing a bunch of variable names. It also gives hypothetical git prompt developers a starting place

That said I don't strong feelings about it :) maybe we can get a third opinion

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, my final note would be that different modules could (and probably will) reuse the same variable names anyway, for example in my custom module I did use SPACESHIP_GIT_STATUS_ADDED variable name 🙂

@maximbaz
Copy link
Contributor

While you are improving git sections, could you perhaps address this discovery as well and move the options to the git_branch subsection where they belong?

SPACESHIP_GIT_PREFIX and SPACESHIP_GIT_SYMBOL are really branch subsection options.

sections/git.zsh Outdated

local git_branch="$(spaceship_git_branch)" git_status="$(spaceship_git_status)"
for el in ${(Oa)SPACESHIP_GIT_ORDER}; do
source "$SPACESHIP_ROOT/sections/git_${el}.zsh" # load dependency
Copy link
Contributor

Choose a reason for hiding this comment

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

this will source every time a prompt is refreshed, in contrast the rest of the sections are sourced only once - I'd suggest moving the "source" part to spaceship.zsh to the same place where other sections are sourced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch thanks. Addressed in 2b91ca2. I'm just sourcing all git sections since that's what Spaceship does for everything else. (Maybe not ideal, but that can be discussed in #407)

@olets
Copy link
Contributor Author

olets commented Mar 31, 2018

@salmanulfarzy @maximbaz I've pushed some commits addressing your feedback

@maximbaz I actually think I was wrong about SPACESHIP_GIT_PREFIX and SPACESHIP_GIT_SYMBOL

@maximbaz
Copy link
Contributor

@maximbaz I actually think I was wrong about SPACESHIP_GIT_PREFIX and SPACESHIP_GIT_SYMBOL

Could you elaborate? Because currently I also think those two belong to the branch section 🙂

@olets
Copy link
Contributor Author

olets commented Mar 31, 2018

@maximbaz it's been long enough that I forgot exactly how it's set up :)

SPACESHIP_GIT_SYMBOL should be in the branch subsection!

In this PR, the Git section has a prefix, and the branch subsection has one too, so SPACESHIP_GIT_PREFIX is good as is. Unless I'm misreading again :)

@maximbaz
Copy link
Contributor

maximbaz commented Mar 31, 2018

I thought we could cleanup the sections/git.zsh from all configuration options except SPACESHIP_GIT_SHOW, for example by renaming SPACESHIP_GIT_SYMBOL to SPACESHIP_GIT_BRANCH_SYMBOL and moving it into the sections/git_branch.zsh. But if that's not trivial and/or doesn't fit into your changes, better leave out for another PR.

@@ -8,7 +8,7 @@
# ------------------------------------------------------------------------------

SPACESHIP_GIT_BRANCH_SHOW="${SPACESHIP_GIT_BRANCH_SHOW=true}"
SPACESHIP_GIT_BRANCH_PREFIX="${SPACESHIP_GIT_BRANCH_PREFIX="$SPACESHIP_GIT_SYMBOL"}"
SPACESHIP_GIT_BRANCH_PREFIX="${SPACESHIP_GIT_BRANCH_PREFIX="$SPACESHIP_GIT_BRANCH_PREFIX"}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm that can't be right! I'm sure I meant to say . Will fix

@olets
Copy link
Contributor Author

olets commented May 24, 2018

Looking at this again to see where it stands.

I moved the git symbol into the branch file, as requested — thanks for sticking to your guns on that idea @maximbaz, you were right.

I personally still like the idea of a git_status_options.zsh — that way any contributor has the option of pulling in an existing Spaceship default. In fact, it could make sense to encourage using reusing variables from git_status_options.zsh, so that if a user wanted to switch from one git prompt to another they'd see familiar symbols in familiar parts of the prompt. (I didn't go back and read the full inline discussion from earlier, but I bet I'm just repeating myself 😉)

@maximbaz maximbaz mentioned this pull request May 29, 2018
@salmanulfarzy salmanulfarzy added the frozen Progress is temporarily frozen until resolving the specific issue. label Sep 20, 2019
github-actions bot pushed a commit that referenced this pull request Sep 1, 2022
# [4.0.0](v3.16.7...v4.0.0) (2022-09-01)

### Bug Fixes

* add permissions to scripts ([7c3e427](7c3e427))
* add x permissions to files ([9a34149](9a34149))
* **async:** remove obsolete runtime variables ([2128ab0](2128ab0))
* cherry pick [#1089](#1089) ([52448c1](52448c1))
* **cli:** Add missing commands in help message ([c2eb3e3](c2eb3e3))
* compdef only when compdef is available ([c309af6](c309af6))
* **docker:** add additional utils inside docker image ([8813eb0](8813eb0))
* fix command-line completion leving one letter on the beginning of prompt ([6a4abd5](6a4abd5)), closes [#1064](#1064)
* **git:** Add explicit variable for async rendering ([cfa95e6](cfa95e6))
* **git:** fix ahead/behind indicators ([a067c4d](a067c4d))
* **git:** Fix typo in a section signature ([f8ed612](f8ed612))
* **hg:** Add options for async rendering ([d5bdfa4](d5bdfa4))
* **kubectl:** fix kubectl section ([1b71e09](1b71e09))
* make core variables readonly ([1cd4756](1cd4756))
* NEWLINE variable doesn't work ([28b37f5](28b37f5))
* **ps2:** print PS2 properly ([0d9766c](0d9766c))
* put user config loading in a proper place ([5c68702](5c68702))
* **python:** use python3 command ([0466bb8](0466bb8))
* **renderer:** fix missing options for globes ([107429c](107429c))
* rendering docker context ([b6a49f8](b6a49f8))
* Resolve newline issue with command expansion ([bcfce70](bcfce70))
* **scripts:** Stop on errors ([ccee8e9](ccee8e9))
* set proper message if no account ([#1099](#1099)) ([69f906c](69f906c))
* support CLOUDSDK_CONFIG env ([#1122](#1122)) ([ae348f9](ae348f9)), closes [#1117](#1117)
* **tests:** render prefixes and suffixes only when they are not empty ([73c2c71](73c2c71))
* **upsearch:** Check repo properly ([4764ed9](4764ed9))

### Code Refactoring

* **ember:** remove ember from built-in sections ([05beb8b](05beb8b))
* **gradle:** migrate Gradle to a separate repo ([984bc01](984bc01))
* **install:** simplify the installation process ([824f876](824f876))
* **maven:** migrate Maven to a separate repo ([3efd48b](3efd48b))
* **vi_mode:** move vi_mode to external section ([9ce611b](9ce611b)), closes [#586](#586)

### Features

* add ability to forcefully render a section synchronously ([216f0f2](216f0f2))
* add IBM Cloud CLI section ([#912](#912)) ([30b4b60](30b4b60))
* Add is_async util ([d539ee5](d539ee5))
* Add java section ([713f406](713f406))
* **async:** add an option to globaly disable/enable async rendering ([0ca1463](0ca1463))
* **async:** introduce async jobs indicator ([f0a75ce](f0a75ce))
* **cli:** add a print command for CLI ([6843105](6843105))
* **cli:** add CLI ([205cb16](205cb16))
* **cli:** add command completions for cli ([2ae45a4](2ae45a4))
* **cli:** Add edit command ([b029587](b029587))
* **cli:** CLI for inserting and removing sections ([5dcce40](5dcce40)), closes [#318](#318)
* **config:** Add ability to store config in ~/.config/spaceship ([b5dee37](b5dee37))
* **config:** introduce config file resolution ([e087459](e087459)), closes [#508](#508)
* **docker:** add docker image and publishing it to ghcr.io ([77d7457](77d7457))
* **exec_time:** display exec time with precision ([7b04aed](7b04aed))
* **git:** add ability to customize git and hg orders ([8a54cb3](8a54cb3)), closes [#391](#391)
* **haskell:** Look for .hs files ([b2eb8a3](b2eb8a3))
* **ibmcloud:** Enable async rendering for ibmcloud ([8391054](8391054))
* **package:** Add composer and julia support ([3f8bb1e](3f8bb1e))
* **perf:** Compile to zwc ([3549bbf](3549bbf))
* **registry:** Add custom sections registry ([86d64dd](86d64dd))
* **renderer:** render sync section on every render ([4e93ae0](4e93ae0))
* **section:** migrate default section to v4 ([0684171](0684171))
* **section:** Migrate sections to new signature ([b3249fb](b3249fb))
* **section:** Named arguments for section ([201496b](201496b))
* **testkit:** add a simple testkit ([1ceca85](1ceca85))
* Upsearch for project files up the tree ([08f6d70](08f6d70))
* **vsc:** Make git and hg async ([9ddb9be](9ddb9be))

### Performance Improvements

* **zwc:** Precompile root file and async ([ee5afc5](ee5afc5))
* **zwc:** spaceship::precompile compiles sources to zwc ([58758e7](58758e7))

### BREAKING CHANGES

* **section:** spaceship::section:v4 uses flags instead of arguments order to pass section params.
* **vi_mode:** vi_mode section is not include in core anymore
* **gradle:** Gradle section is not included by default. It can be installed additionally from:
https://github.com/spaceship-prompt/spaceship-gradle
* **maven:** Maven section is no longer available by default. It can be additionally installed
from: https://github.com/spaceship-prompt/spaceship-maven
* **install:** Now users have to manually enable prompt system and choose spaceship
* **ember:** ember section is no longer available out of the box. It needs to be installed
additionally.
@denysdovhan
Copy link
Member

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@denysdovhan denysdovhan added the released Released issues and PRs label Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frozen Progress is temporarily frozen until resolving the specific issue. new-feature A PR that implement feature (section, specific behavior, etc). released Released issues and PRs
Development

Successfully merging this pull request may close these issues.

None yet

4 participants