More customization #22

bric3 · 2014-01-04T20:56:18Z

This new base script (git-info.sh)

is not related to a prompt shell anymore
is using configurable color codes
doesn't leak variables
does not depend on external configuration
offers configurable color codes per symbol
has a more understandable name, that can be easily used in another context (I'm using it in my own crafted oh-my-zsh theme, though it's named oh-my-git.plugin.zsh)
adds suffix and prefix config
can toggle the display of the git repository symbol (display_git_symbol)
can toggle if empty spaces for off flags will be displayed (use_color_off=false and flag is off, toggable with print_unactive_flags_space)
can be toggled per git repository config git config --get oh-my-zsh.hide-status
displays current git action (REBASE-i, REBASE-m, REBASE, AM/REBASE, MERGING, BISECTING, CHERRY-PICKING)

Note default color codes might not work well on ZSH, when this method is echoed on `RPROMPT``, use the following ZSH compliant color codes.

For example:

# redefine zsh compliant color codes for oh-my-git
omg_on="%B";
omg_off="%b";
omg_red="%F{red}";
omg_green="%F{green}";
omg_yellow="%F{yellow}";
omg_violet="%F{magenta}";
omg_reset="%{%f%k%b%}";
omg_prefix=" %{%B%F{green}%}[%{%f%k%b%}"
omg_suffix="%{%B%F{green}%}]%{%f%k%b%}"

This new script - is not related to a prompt shell anymore, except for color codes - is using configurable color codes - doesn't leak variables - offers configurable color codes per symbol

arialdomartini · 2014-01-04T21:29:29Z

Amazing job, pal! Really impressive!
I'm going to review it as soon as back home (a 3'' phone is all I have right now).

Hopefully I'll merge it on Monday.

arialdomartini · 2014-01-08T09:25:21Z

Hi Brice.
I reviewed your (impressive) work. I'm looking forward to merging it. You made a really great job!

Before merging your pull request, I need to make it compatible with the current zsh themes.
The current themes use something like

PROMPT='$(build_prompt)%{$fg_bold[green]%}%~
%{$fg_bold[white]%}∙ '

so they would break right after the merge.
I wonder if we should rename oh_my_git_info to build_prompt or, vice-versa, update the themes.
What do you think?

I also noticed that the prompt.sh file still has

 # Colors
    : ${on='\033[0;37m'}
    : ${off='\033[1;30m'}
    : ${red='\033[0;31m'}
    : ${green='\033[0;32m'}
    : ${yellow='\033[0;33m'}
    : ${violet='\033[0;35m'}
    : ${branch_color='\033[0;34m'}
    : ${reset='\033[0m'}

which uses the old names.
While I agree that the new names are much, much better, I think we should update the prompt.sh as well. Don't you agree?

The last two issues I noticed:

in bash, on my Mac, your version produces a weird prompt: I'm pretty sure that it depends on the old variable names used in the prompt.sh script;
zsh the prompt is slightly different than the original: when the current path is displayed inline with the prompt (like in the arialdo-pathinline.zsh-theme) the space between the prompt and the path is missing.

I think most of these problems could be fixed updating the variables names in prompt.sh and syncing the themes with function name.

What do you think?

bric3 · 2014-01-08T10:47:43Z

Oh you're right I forgot to update the vars and I removed the finally variable appending. I definitely got too hasty in the commit b015459

So this might be possible to reintroduce a build_prompt function that will build the PS1 using the oh_my_git_info and the other forgotten variables.

Also I prefer to keep the oh_my_git_info name because it is not anymore related to a shell prompt. And on a engineering point of view it may even be a good thing to keep oh_my_git_info and build_prompt in separate files as well.

I'll fix the issues you raised later this week ;)

Also it's not in the pipeline yet, but I wondered if oh-my-git could have a place in the oh-my-git repo (in this case the variables must be uppercase to conform the coding norm of oh-my-zsh). Or if it is enough to put the script in the oh-my-zsh custom plugin folder

arialdomartini · 2014-01-08T10:54:53Z

I'll fix the issues you raised later this week ;)

Take your time! In the meanwhile I'm going on reviewing your code.

Also it's not in the pipeline yet, but I wondered if oh-my-git could have a place in the (oh-my-zsh?) repo

I'd love to! How could I achieve this?

bric3 · 2014-01-08T11:49:39Z

For oh-my-zsh, I think you can just make a simple PR there. For the moment until I created the following file plugins/oh-my-git/oh-my-git.plugin.zsh and load the plugin in the zshrc.

Here's a screenshot of how it looks

Also to be accepted the code needs to follow the coding style https://github.com/robbyrussell/oh-my-zsh/wiki/Coding-style-guide

bric3 · 2014-01-08T22:22:12Z

Hello I've fixed the issues, now it should work fine on bash or on zsh. I added a new prompt_builder.sh script that declares a build_prompt function.

Also I fixed the color codes for bash in the example, as they must be wrapped by \[ and \].

arialdomartini · 2014-01-09T07:39:55Z

Great!
I'm testing your last commits. I think we are close to the final merge ;)
I started the tests with bash, and I noticed that, strangely, the current prompt at master and the one at 98d7a66 are a bit different: in the original prompt a missing symbols produces a space (they are blank), so that symbols are always represented at the same position; on the contrary, in your last prompt, active symbols collapse, since inactive symbols have null width (probably they are not printed at all).

In this screenshot, the first line after cd prg/oh-my-git uses your prompt; the next one uses the prompt at master.

I'm trying to understand why checking your code.
Cheers!

arialdomartini · 2014-01-09T08:13:50Z

My fault! I just understood that there's the option print_unactive_flags_space for this.
In order to make the switch to the new version transparent, I think it would be better if print_unactive_flags_space is true so that the new prompt is not changed.
Do you agree?

arialdomartini · 2014-01-09T08:35:35Z

(By the way: I hate to be such a perfectionist, but, you know, we must ensure that the new version won't introduce any new bugs or unexpected behaviours. I'm really sorry that I'm forcing you to discuss every single line of your great contribution: I hope you won't mind ;)

arialdomartini · 2014-01-09T08:38:14Z

prompt.sh

@@ -28,20 +28,26 @@ if [ -n "${BASH_VERSION}" ]; then
    : ${two_lines:=true}
    : ${finally:='\w ∙ '}
    : ${use_color_off:=false}
+	: ${print_unactive_flags_space:=false}


I like this flag: it's a big improvement of the previous implementation.
In order to make the new prompt perfectly compatible with the previous one, I'd use true as the default value.

: ${print_unactive_flags_space:=true}

bric3 · 2014-01-09T09:35:41Z

No problem, I understand that ;)

I set the print_unactive_flags_space to true by default now.

bric3 · 2014-01-09T09:52:37Z

I'm eager to merge that one ;P
Because I've something else coming up ; I've been meddling with the current action of git and the __git_ps1 script, as it's not displayed yet on any ZSH (including oh-my-zsh) git theme, and now I can display the current action (REBASING, CHERRY-PICKING, etc.). It works very well, but the placement of this information could be polished as well (right now it's only on the left).

See : https://github.com/bric3/oh-my-git/blob/git_current_action/git-info.sh

arialdomartini · 2014-01-09T09:55:32Z

Yes, I know! I've seen your next commits (your repo is one of my remotes ;)
This evening I will continue the tests with zsh, then merge!

arialdomartini · 2014-01-09T09:58:26Z

(I also think that the "current action" feature is really really cool!)

arialdomartini · 2014-01-09T18:12:24Z

Ok, we are almost done.
I tested it with zsh and I figured out which changes need to be made to the themes (the new flags must be added and the variables names must be updated).

One of the flags that I should add, in order to make the theme render like before, is omg_suffix: it should contain a bunch of spaces if one wishes to print the current path inline with the git prompt.
Unfortunately, the line

# collapse contiguous spaces including new lines
echo $(echo "${oh_my_git_string}")

would strip those spaces away.
I think the contiguous spaces should be stripped before the omg_suffix is concatenated to the prompt.
Do you agree?

I have prepared the fixes for the 2 themes, so I'll be able to merge this pull request and publish the new version for the theme, as soon as this issue has been fixed. So far, this was the last problem I was able to spot.

Cheers!

bric3 · 2014-01-09T19:05:01Z

During my tests I didn't have to use any spaces, I think this is probably the same issue as with bash prompt with unprintable characters. (When the prompt is redrawn the terminal thinks it is wider that it actually is, and depending on the configuration the actual prompt location may vary.)

In my setup I don't use $RED or escape codes directly but rather the following color codes documented in the prompt expansion section of the zsh documentation. So color codes must be wrapped in %{ and %} or use the documented prompt expansion directives.

Also I don't know where these variables ($RED, $WHITE, etc.) come from. I don't have them in my shell.

Instead I defined the colors this way on ZSH :

omg_on="%B";
omg_off="%b";
omg_red="%F{red}";
omg_green="%F{green}";
omg_yellow="%F{yellow}";
omg_violet="%F{magenta}";
omg_reset="%{%f%k%b%}";
omg_prefix=" %{%B%F{green}%}[%{%f%k%b%}"
omg_suffix="%{%B%F{green}%}]%{%f%k%b%}"

arialdomartini · 2014-01-09T22:37:00Z

I'm also very eager to merge this!
Brice, thanks a lot for all the time you are giving to this project. I really really appreciate it.

Unfortunately, I am not able to get rid of some weird problems with your code. I'm very very sorry, because I'm looking forward to including all your new features in oh-my-git repository.

This is the current state: I created 2 forks for us to test

https://github.com/code4nothing/oh-my-git which is a fork of your oh-my-git fork. In this repository master has been merged with more_customization. That is: this repository is equivalent of mine, as if I had merged your pull request.
https://github.com/code4nothing/oh-my-git-themes is a fork of https://github.com/arialdomartini/oh-my-git-themes: in this repository I changed the arialdo-granzestyle.zsh-theme so that it contains

omg_on="%B";
omg_off="%b";
omg_red="%F{red}";
omg_green="%F{green}";
omg_yellow="%F{yellow}";
omg_violet="%F{magenta}";
omg_reset="%{%f%k%b%}";
omg_prefix="%{%B%F{green}%}%{%f%k%b%}"
omg_suffix="%{%B%F{green}%} %{%f%k%b%}"
print_unactive_flags_space=true

rather than

on=$WHITE
off=$WHITE
red=$RED
green=$GREEN
yellow=$YELLOW
violet=$CYAN
branch_color=$BLUE
reset=$RESET

Those 2 repository are exactly what I'd expect to obtain after the merge of your pull request.

I'm using them to perform a last test, and so you could do (in the case you want the rights to push on them, just ask me, I'll be happy to include you)

Using antigen, I configured my zsh with this .zshrc

source "$HOME/.antigen/antigen.zsh"

antigen-use oh-my-zsh
antigen-bundle git
antigen-bundle zsh-users/zsh-syntax-highlighting
antigen-bundle zsh-users/zsh-history-substring-search

#antigen-bundle arialdomartini/oh-my-git
#antigen theme arialdomartini/oh-my-git-themes arialdo-granzestyle
antigen-bundle code4nothing/oh-my-git
antigen theme code4nothing/oh-my-git-themes arialdo-granzestyle

antigen-apply

This is what I obtain:

Can you spot the two issues?

The first is a whitespace before the ❤ symbol. This is really weird, and I spent 2 hours trying to understand why this is printed in zsh only. Bash does not displays it. I wonder if this can depends on

# collapse contiguous spaces including new lines
    echo $(echo "${oh_my_git_string}")

Anyway: this is just a minor problem.

Unlucky, there's a second issue: whenever you checkout a commit detaching from branches, a / is displayed rather that the commit hash.

See the comments inline for this issue

Cheers!

arialdomartini · 2014-01-09T22:37:04Z

git-info.sh

+			if [[ ${just_init} == true ]]; then
+				oh_my_git_string="${oh_my_git_string}${detached_color}detached${reset}";
+			else
+				oh_my_git_string="${oh_my_git_string}${detached_current_commit_color}(${current_commit_hash_abbrev}\)${reset}";


It turns out that ${current_commit_hash_abbrev} is never assigned.
I think the line should be

oh_my_git_string="${oh_my_git_string}${detached_current_commit_color}(${current_commit_hash:0:7})${reset}";

(with the :0:7 and without the extra \)

Alternatively, the ${current_commit_hash_abbrev} must be defined.
Since you introduced a new variable, I suppose you just missed a line.

bric3 · 2014-01-09T23:42:30Z

You're welcome ;)

Well spotted, I totally missed that one.

I think I see where is the issue with the leading space. And I believe this is due to string concatenation. the double echo trick, only collapse contiguous spaces in to one, it doesn't remove them. I wonder of using printf instead of echo could solve this issue, I've already used it for an svn-fast-plugin I wrote for oh-my-zsh

bric3 · 2014-01-10T01:03:05Z

By the way, I'm not using antigen at all, just the bunch of scripts available on oh-my-zsh

It's probably the one of the most intensive work I have ever done in shell, shell scripting is kinda booby trapped ;)

About the new repos they seem to look good. The powerline theme needs the new color code though.

arialdomartini · 2014-01-10T11:05:45Z

It's probably the one of the most intensive work I have ever done in shell, shell scripting is kinda booby trapped ;

I'd love to see it! Is it public?

As for the current state of the pull request, I'm going to update the other theme as well.
Could I be of any help for the fix of the last bugs we found in the pull request?

bric3 · 2014-01-10T12:58:44Z

It's probably the one of the most intensive work I have ever done in shell, shell scripting is kinda booby trapped ;

I'd love to see it! Is it public?

I've made some PRs on the oh-my-zsh repo, but they are not yet integrated. As for the theme I didn't updated it yet on my fork ; it still uses the git plugin of oh-my-zsh and non submitted works. I think once my PRs are merged there I will probably push the updated modification. But I may create a new project that install easily in the not yet documented $ZSH_CUSTOM folder setting.

Could I be of any help for the fix of the last bugs we found in the pull request?

I think the last two are now fixed ; ie the leading space issue and the commit hash. I haven't looked at every combination, but I don't think there's any other (significant) bug ?

bric3 · 2014-01-13T09:36:10Z

I'm using ZSH 5.0.5 so that shouldn't be an issue (well to be sure the changelog has to be checked).

For example the following lines remove the leading and trailing blank spaces just fine in ZSH

SOME_VAR="  a b  "
echo "|${SOME_VAR##+([[:space:]])}|"    # removes leading whitespace characters
echo "|${SOME_VAR%%+([[:space:]])}|"    # removes trailing whitespace characters

However checking again I've discovered that these expansions don't work well in bash (that could mean there's some option to set for bash). It seems the correct way in bash is the following :

SOME_VAR="  a b  "
echo "|${SOME_VAR#"${SOME_VAR%%[![:space:]]*}"}|"   # removes leading whitespace characters
echo "|${SOME_VAR%"${SOME_VAR##*[![:space:]]}"}|"   # removes trailing whitespace characters

A trim function should be declared per type of shell, I'll do that sometime later.

bric3 · 2014-01-13T09:56:42Z

Using your theme, without any modifications in the theme or the script here's what I got :

bric3 · 2014-01-13T12:45:31Z

I've externalized the trimming function, I didn't change the logic, but it may help to narrow down the issue on your box.

Will now display curent git action (REBASE-i, REBASE-m, REBASE, MERGING, CHERRY-PICKING, etc...) Also adds `oh-my-zsh.hide-status` check before doing anything

bric3 · 2014-07-04T17:28:38Z

Hey,

I'd like to inform you that I've created a separate project from this pull request, I've reworked the install to be tailored for oh-my-zsh. Of course most credit must go to you. Let me know if you want to change the credits in a way or another.

⇒ https://github.com/bric3/oh-my-zsh-git/

Cheers,
brice

arialdomartini · 2014-07-04T17:55:11Z

Man, this is an amazing work!
My congratulation!

My fault that I wasn't able to fix the only, little issue that prevented me to merge your PR: I'd say it has been a luck, since it took the the birth of your project!

bric3 · 2014-07-04T19:08:53Z

Well bash stuff have quirks ;)
And still I wasn't able to locate your issue as well

If you see issues you can fix don't hesitate as well to make a PR then :)

bric3 · 2015-01-18T09:27:23Z

@arialdomartini looks very good !

arialdomartini · 2015-01-18T09:38:13Z

Man, I included your git_current_action() in 5a38646
It's an amazing idea of yours! Thank you very much

bric3 · 2015-01-18T10:25:10Z

:)

bric3 added 2 commits January 4, 2014 21:44

extracted the git information code

642d2f3

This new script - is not related to a prompt shell anymore, except for color codes - is using configurable color codes - doesn't leak variables - offers configurable color codes per symbol

Replaces the base.sh by the reworked git-info.sh

b015459

forgot one more 'tr' after wc

50384e7

bric3 mentioned this pull request Jan 6, 2014

Adds a method that print a battery gauge ohmyzsh/ohmyzsh#2402

Merged

brings back compatibility with existing themes

98d7a66

arialdomartini reviewed Jan 9, 2014
View reviewed changes

sets display unactive flags to true by default

52f05eb

arialdomartini reviewed Jan 9, 2014
View reviewed changes

fixes bad initialization of short commit hash when detached

d1d88b5

bric3 added 2 commits January 10, 2014 00:52

spaces

554113e

fixes leading and trailing blank spaces that might lurk around

3920223

Removes git current action code

6ff823c

Added a trim function declaration for ZSH and BASH

4882bc6

Displays current git action

0a0d3d2

Will now display curent git action (REBASE-i, REBASE-m, REBASE, MERGING, CHERRY-PICKING, etc...) Also adds `oh-my-zsh.hide-status` check before doing anything

arialdomartini mentioned this pull request May 28, 2014

Disable oh-my-git on a per-repository basis #25

Closed

arialdomartini closed this Jul 4, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More customization #22

More customization #22

bric3 commented Jan 4, 2014

arialdomartini commented Jan 4, 2014

arialdomartini commented Jan 8, 2014

bric3 commented Jan 8, 2014

arialdomartini commented Jan 8, 2014

bric3 commented Jan 8, 2014

bric3 commented Jan 8, 2014

arialdomartini commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini Jan 9, 2014

bric3 commented Jan 9, 2014

bric3 commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

bric3 commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini Jan 9, 2014

bric3 commented Jan 9, 2014

bric3 commented Jan 10, 2014

arialdomartini commented Jan 10, 2014

bric3 commented Jan 10, 2014

bric3 commented Jan 13, 2014

bric3 commented Jan 13, 2014

bric3 commented Jan 13, 2014

bric3 commented Jul 4, 2014

arialdomartini commented Jul 4, 2014

bric3 commented Jul 4, 2014

bric3 commented Jan 18, 2015

arialdomartini commented Jan 18, 2015

bric3 commented Jan 18, 2015

More customization #22

More customization #22

Conversation

bric3 commented Jan 4, 2014

arialdomartini commented Jan 4, 2014

arialdomartini commented Jan 8, 2014

bric3 commented Jan 8, 2014

arialdomartini commented Jan 8, 2014

bric3 commented Jan 8, 2014

bric3 commented Jan 8, 2014

arialdomartini commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini Jan 9, 2014

Choose a reason for hiding this comment

bric3 commented Jan 9, 2014

bric3 commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

bric3 commented Jan 9, 2014

arialdomartini commented Jan 9, 2014

arialdomartini Jan 9, 2014

Choose a reason for hiding this comment

bric3 commented Jan 9, 2014

bric3 commented Jan 10, 2014

arialdomartini commented Jan 10, 2014

bric3 commented Jan 10, 2014

bric3 commented Jan 13, 2014

bric3 commented Jan 13, 2014

bric3 commented Jan 13, 2014

bric3 commented Jul 4, 2014

arialdomartini commented Jul 4, 2014

bric3 commented Jul 4, 2014

bric3 commented Jan 18, 2015

arialdomartini commented Jan 18, 2015

bric3 commented Jan 18, 2015