Support for Git default branches other than master

[darrenkidd]

Newly created repos in GitHub now use main as the default branch name. This PR makes main behave the same as master would (i.e. collapsed by default).

I'm not supporting the gazillions of other default branch names that other devs might use; I think that keeping master and main as the core names is the simplest and least-controversial approach.

[bobthecow]

👍

I've been wondering if we should add something like theme_default_git_branches which defaults to both main and master, but could be locally overridden to any branch names you want. If we do this, we should also rename (with backwards compatibility) theme_display_git_master_branch to theme_display_git_default_branch.

[darrenkidd]

Working on these changes; should have something up soon.

Out of interest, where would you set the $theme_default_git_branches variable? It's not the usual case of undefined until overridden with a yes or no, so I'm a bit lost as to where is the best place to put it.

I'm assuming user would do a set -a to append custom branch names to the default list in their config.fish. I'll document all of that once we've confirmed.

Finally, wondering if you have a pattern for warning users that a variable (i.e. $theme_display_git_master_branch) is deprecated if it detects usage?

[bobthecow]

We should never set userland variables ourselves. Instead:

builtin count $theme_default_git_branches
  or set -l theme_default_git_branches master main

… inside the function that'll use it. That way we get the values we want if none are set, but don't leak anything outside our scope. Overriding users will need to specify the defaults if they want to retain them, not just append new ones. This also lets people remove, e.g., main if they want it to not receive "default branch" treatment.

We don't warn. Just treat both options as equivalent (with a preference for the new name if it exists), and update documentation to reference the new one.

tl;dr:

Be conservative in what you do, be liberal in what you accept from others

🙂

[darrenkidd]

We should never set userland variables ourselves.

Awesome - I'm fairly new to fish and this theme, so all good stuff to learn. Out of interest, is this kind of knowledge documented anywhere? Feels like something that should go into a Wiki page or CONTRIBUTING.md.

builtin count $theme_default_git_branches
  or set -l theme_default_git_branches master main

I've used theme_git_default_branches instead of theme_default_git_branches as "default" is such an overloaded term i.e. I wanted to make it clear it was specific to Git internals, not the theme. Also follows the pattern of theme_git_worktree_support.

I decided to go with the non-zero test instead of the builtin count check you used, as it seemed slightly simpler with no downsides (that I know of). But I'm still a fish novice, so I could be wrong. Happy to change (although your code does need to redirect all output to /dev/null as it returns the number of items in your prompt otherwise).

Overriding users will need to specify the defaults if they want to retain them, not just append new ones. This also lets people remove, e.g., main if they want it to not receive "default branch" treatment.

That's a really good point that I hadn't thought about. Have implemented it this way now.

We don't warn. Just treat both options as equivalent (with a preference for the new name if it exists), and update documentation to reference the new one.

This my plan as well - but wasn't sure how you handle deprecation (i.e. support-both-forever vs aggressively-remove-in-next-major). Plenty of other tools out there that politely tell you that you should stop using something, so was wondering if this theme was one of them. Not bothered either way; still learning the lay of the land!

For clarity, here's the current interaction of theme variables: .	theme_display_git_default_branch = 'yes'	theme_display_git_default_branch != 'yes'
theme_display_git_master_branch = 'yes'	don't collapse	don't collapse
theme_display_git_master_branch != 'yes'	don't collapse	collapse

[darrenkidd]

Testing

Before changes:

After changes:

Scripts

test_2_new_display_var_ok.fish

set -g theme_display_git_default_branch yes

test_3_old_display_var_ok.fish

set -g theme_display_git_master_branch yes

test_4_both_display_vars_ok.fish

set -g theme_display_git_default_branch yes
set -g theme_display_git_master_branch yes

test_5_custom_default_branches_ok.fish

set -g theme_git_default_branches trunk

test_9_show_values.fish

echo "\$theme_git_default_branches='$theme_git_default_branches'"
echo "\$theme_display_git_master_branch='$theme_display_git_master_branch'"
echo "\$theme_display_git_default_branch='$theme_display_git_default_branch'"

[darrenkidd]

Testing

Because I messed with the truncation, I wanted to make sure it still worked OK.

Before changes:

After changes:

Scripts

test_6_trunc_branch_ok.fish

set -g theme_use_abbreviated_branch_name yes

[darrenkidd]

~FYI: documentation to follow soon.~

Documentation done.

[bobthecow]

Out of interest, is this kind of knowledge documented anywhere?

Yeah. I've laid out a bunch things like this as of principles of bobthefish over the years, mostly in PR feedback. But it would probably be helpful to write them up so people can find 'em :)

I've used theme_git_default_branches instead of theme_default_git_branches

👍

I decided to go with the non-zero test instead of the builtin count check you used

Yeah, this should be fine. For trivial values it's even a hair faster to check. Maybe at a large scale the string interpolation cost would grow, but we're realistically talking about two or three branch names, tops :)

This my plan as well - but wasn't sure how you handle deprecation

I tend to fall on the "stop telling people to use it, but basically support it forever" side here. Not really worried about migrating existing users, since in general config options are not expensive to support :)

[darrenkidd]

You happy with this one now, @bobthecow?

[bobthecow]

Yep! Thank you :)