Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

Add space between VSC_BRANCH_ICON and branch name. #618

Closed
wants to merge 2 commits into from

Conversation

diraol
Copy link
Contributor

@diraol diraol commented Sep 9, 2017

FIX #617

@bhilburn
Copy link
Member

@diraol - Thanks so much for submitting this PR! I apologize it's taken me so long to respond - the last month has been a bit crazy for me.

@diraol @runmad - This issue has actually come up a few times. When I first released P9k, we actually did not have a space, then I added it a little bit ago and it remained for a few releases before enough people complained that I pulled it back out again.

The primary issue with putting the space in the code is that it makes it difficult for those that don't want it to remove it. Going the opposite direction, though, if there is not a space in the code, it's very easy to add one by modifying the VCS_BRANCH_ICON text by appending a space.

If you look at your changeset, for example, with the space built-in, there, there is no way for someone to remove it other than modifying the P9k code directly.

I personally prefer having a space, to be honest, and I would like to settle on a solution that works well for everyone.

@dritter @rjorgenson @docwhat - Curious about your thoughts on this topic.

@docwhat
Copy link
Contributor

docwhat commented Sep 29, 2017

I think leaving the space out is best. Unicode 9 solves a lot of problems with characters that need double space.

@diraol
Copy link
Contributor Author

diraol commented Sep 29, 2017

@bhilburn and @docwhat what about adding a new variable just like POWERLEVEL9K_SHORTEN_DELIMITER?

Something like POWERLEVEL9K_VCS_BRANCH_ICON_RSPACE. It can be a 'boolean' variable or just a string, which defaults to "" (empty string) and we can set it to " ".

@docwhat
Copy link
Contributor

docwhat commented Sep 30, 2017

I think supporting Unicode 9 is better. It automatically makes wide glyphs take two spaces in mono spaced fonts.

@diraol
Copy link
Contributor Author

diraol commented Oct 3, 2017

@docwhat I do not have any idea what is the necessary effort to support Unicode 9. Any tips on that?

Also, why not supporting the latest release of Unicode (10) instead of (9)?

@docwhat
Copy link
Contributor

docwhat commented Oct 4, 2017

https://github.com/jquast/wcwidth has some info. As well as iterm2 for Mac.

Basically: emoji can be double width in 9. This makes things interesting if you need to know how wide text is.

@bhilburn
Copy link
Member

bhilburn commented Oct 7, 2017

Hm, so it's not really clear to me what actually needs to be done in P9k to support UTF-9.

@diraol - So I just put the space back. I actually started receiving e-mails from folks about this, and they raised the good point that, previously, the default was to have the space. I've returned it back to the old default, and anyone who doesn't want the space can remove it by resetting the icon.

@bhilburn bhilburn closed this Oct 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants