Skip to content
This repository has been archived by the owner on May 6, 2021. It is now read-only.

New symbol to show that there is no upstream branch #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

malfter
Copy link

@malfter malfter commented Mar 18, 2018

Thanks for your work, here are some small extensions.

Copy link
Owner

@bric3 bric3 left a comment

Choose a reason for hiding this comment

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

Thank you, I'm ok to add the flag, however I'd like a few changes (explained in the comment). Feel free to challenge my points.

README.md Outdated
@@ -120,6 +120,7 @@ needs_to_merge_symbol | `ᄉ`
needs_to_merge_color | `$yellow`
has_upstream_symbol | `⇅`
has_upstream_color | `$on`
has_no_upstream_symbol | ` ⃢`
Copy link
Owner

Choose a reason for hiding this comment

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

I don't know this symbol, it probably comes from a patched font, however I'd rather have a standard unicode symbol as default. Because not everyone may have this symbol. Also the space may not be necessary.

Copy link
Author

Choose a reason for hiding this comment

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

The symbol is the unicode character SCREEN

no-upstream_unicode-screen

However, it does not seem to be supported by many fonts. I replaced it with a character from your code.

no-upstream_unicode

An alternative would be to leave it empty and it can then be adjusted by the user via the variable.

I use a font from 'Awesome-Terminal-Fonts' with the symbol 'omg_not_tracked_branch_symbol' from 'oh-my-git'.
no-upstream_awesome_omg-not-tracked

@@ -230,7 +231,7 @@ function oh_my_git_info {

else
oh_my_git_string+="
${has_no_upstream_color}(${current_branch_color}${current_branch}${reset}${has_no_upstream_color})${reset}
${has_no_upstream_color}(${current_branch_color}${current_branch}${reset}${has_no_upstream_color}${has_no_upstream_symbol})${reset}
Copy link
Owner

Choose a reason for hiding this comment

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

Wouldn't it be better if it was placed at the same position as the upstream indicator ([line 190-192]](0350b07#diff-d65f4cbd76d7f4fbc68f321e1ae82c3fR190)). Also I think it should be displayed only if the same (display_has_upstream) is enabled, or a similar one.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, you're right, I didn't think of that. I've changed it.

I think the existing flag 'display_has_upstream' is sufficient for this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants