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

Show AheadyBy/BehindBy counts based on configuration #256

Merged
merged 5 commits into from
Dec 30, 2016

Conversation

drawfour
Copy link
Contributor

Add prompt setting BranchBehindAndAheadDisplay, which operates as follows:

If only behind or ahead:
    If set to "Full" or "Compact", display count along with appropriate arrow, otherwise just show the arrow

If both behind and ahead:
    If set to "Full", then display count along with arrow indicating direction of behind/ahead
    If set to "Compact", display the behind count immediately next to double-arrow, then ahead count
    If set to "Minimal" (or not recognized), display the old behavior of just the double-arrow.

Add prompt setting BranchBehindAndAheadDisplay:
    If only behind or ahead,:
        If set to "Full" or "Compact", display count along with appropriate arrow, otherwise just show the arrow
    If both behind and ahead:
        If set to "Full", then display count along with arrow indicating direction of behind/ahead
        If set to "Compact", display the behind count immediately next to double-arrow, then ahead count
        If set to "Minimal" (or not recognized), display the old behavior of just the double-arrow.
@paulmarsy
Copy link
Contributor

Would there be interest in a "ColourCoded" display option, where the behind by arrow's colour changes based upon how many commits it is behind by?

Say magenta if it's behind by 1, dark magenta 2-4, red 5-10, dark red 10+?

That would convey an indication of how behind the branch is without making the prompt any bigger if the exact count isn't of interest but more the general severity how how much it is behind by.

@drawfour
Copy link
Contributor Author

Interesting idea. Each arrow (for Full mode) can be color coded as well, but Compact/Minimal mode only have one symbol. That can be changed according to the overall delta (i.e. sum of Ahead/Behind is the deciding factor). Or we could have a mode between "Compact" and "Minimal" that will show individual arrows for Ahead/Behind, with a color coded option - just no numbers.

@bazzilic
Copy link

It is better not to rely on colors to convey information as people are likely to have custom color schemes.

Copy link
Owner

@dahlbyk dahlbyk left a comment

Choose a reason for hiding this comment

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

Sorry to have ignored this for so long - looks really good! Left a few comments for your consideration. Thanks for bring it up-to-date!

@@ -82,6 +82,9 @@ $global:GitPromptSettings = New-Object PSObject -Property @{

AutoRefreshIndex = $true

# Valid values are "Full", "Compact", and "Minimal"
Copy link
Owner

Choose a reason for hiding this comment

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

Thoughts on defining an enum to represent the valid states?

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 thought about it, but seemed like overkill to me, since nothing else uses enums yet, and the syntax for enums seems less Powershell-y than some weird hack. If enums were already used, I would have defined them that way. But I can add.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to use an enum but once we drop support on master for PS v2, you can use:

[ValidateSet('Compact','Full','Minimal')]
$BranchBehindAndAheadDisplay = "Full"

PS v2 support can't be dropped soon enough. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, that doesn't work because it's a set of properties being initialized, not a variable. We could move it outside the GitPromptSettings declaration as a special case, but that's kind of ugly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Doh. Yeah, sometimes the narrow GitHub diff window bites me. :-)

Copy link
Owner

Choose a reason for hiding this comment

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

For 1.0 we could consider converting $GitPromptSettings to a giant config function of doom, as @Jaykul proposed long ago.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's an interesting approach. It does have the benefit of providing tab-completion/validation on each parameter.

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 change that was proposed has a side effect of having to pass the same parameters that you've already changed to the config method, since the default value for the parameter is a hard-coded defined value. You could still save the settings in a variable/property bag for storing, and perhaps have the default value for the parameter be the value already in the property bag, so that once changed, it will be used for subsequent sets.

Or you can do the Add-Type -TypeDefinition method to create enums you need and a class that has a bunch of public properties. They won't have tab-completion, but they will have parameter validation.

Copy link
Owner

Choose a reason for hiding this comment

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

Opened #340 for discussion, targeting 1.0.

@@ -82,6 +82,9 @@ $global:GitPromptSettings = New-Object PSObject -Property @{

AutoRefreshIndex = $true

# Valid values are "Full", "Compact", and "Minimal"
BranchBehindAndAheadDisplay = "Full"
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure about defaulting to Full - we're already taking up more space than we used to. Then again, leaving the counts disabled by default means most people will never see them. Anyone else have an opinion?

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 exactly why I left it default to Full and then added the example showing how to make it Compact. I find that the staged files section takes way more space than the Full view, unless you're dealing with seriously out of sync branches.


if ($branchStatusSymbol) {
Write-Prompt (" {0}" -f $branchStatusSymbol) -BackgroundColor $branchStatusBackgroundColor -ForegroundColor $branchStatusForegroundColor

Copy link
Owner

Choose a reason for hiding this comment

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

Trailing whitespace! :trollface:

@dahlbyk dahlbyk merged commit 7bb0f5e into dahlbyk:master Dec 30, 2016
@dahlbyk
Copy link
Owner

dahlbyk commented Dec 30, 2016

Works for me - belated thanks!

@drawfour
Copy link
Contributor Author

Happy to contribute! I use this tool every day.

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.

5 participants