Git plugin: Replace calls to git status with porcelain commands

[ColinHebert]

Clean the git plugin to avoid calls to git status.

Right now, most of the git plugin depends on the git-status command which is called like a user would do.
There is plenty of plumbing tools more adapted to get this kind of data and even more information.

A possible issue with git-status is the fact that if colors.ui is set to always, it will screw up colors in the prompt.

Another neat feature if the dirty count. Now we can know how many dirty files are in the current repository.

[sorin-ionescu]

No! I specifically wrote it this way to avoid multiple calls to Git commands to make the prompt fast.

The difference between git status --short --branch and git status --porcelain is that it shows the branch and remote ahead, behind.

[ColinHebert]

I must say I would like to see benchmarks showing that this is slower than git status --short --branch.

Having git to do a formatting job, then trying to extract the content from the formatted data (with a ton of conditions/regex) instead of calling the plumbing methods (which are made to be fast) without having any processing to do seems way slower to me.

Plus as I said, currently, using git status --short --branch break colors in the prompt if colors.ui is set to always.

[sorin-ionescu]

This is not the first iteration of the code. I have not written benchmarks. But, I saw perceived slowness with my eyeballs. Yes, I know how that sounds. Feel free to benchmark it.

[ColinHebert]

Right now I don't really have the time (nor the willingness) to do the benchmarks. Anyway keeping the current version or this version of the code shouldn't change anything to the end-user (except for the dirty-count part, I'll submit another patch for that [#42] and the issue with colors.ui=always), so it won't be a problem to choose either this solution or the current one.

If someone wants to do the benchmarks that would be awesome.

For what it worth, I think this is a bit more maintainable (separation of concerns, 1 command per task), but if indeed it's slower than the current code, and can't be modified in order to be faster it would be better to stick with the current solution.

In the mean time, unless absolutely you want to close this pull request, can we keep this request open until we have more details on the performances?

[sorin-ionescu]

It is more maintainable. Clearly, one command that gives you everything is faster than multiple commands that give you pieces. Now, does parsing said one command slow it down to where the speed gained from running one command is nullified?

We have to be careful. A few milliseconds here, a few milliseconds there will add up to one or two seconds of loading time, which is noticeable when you open new terminal windows or tabs.

I have been aware of the color.ui = always issue. I chose to ignore it because setting it to always breaks many programs who don't strip ANSI escape codes. auto is the best setting.

[ColinHebert]

(Except when you want to use cat as the git pager and want to pipe to another thing...).

On another note (but still performance wise), I mentioned it in #39, but wouldn't it be possible to diable some part of git-info based on the settings of the current theme?

Regarding the overall speed, I'm not even sure that's always true that only one command does it faster, if the implementation only calls plumbing methods to aggregate everything in one big formated result it could be even slower. But I totally agree, even with a few millisecs, with every other script around it could be quickly a nightmare.

[sorin-ionescu]

It's possible, but you would have to add many if statements to check if the zstyles have been defined. Also, style.zsh will have to be turned into a giant comment. It's a complication.

[ColinHebert]

But if it's applied on "groups" of data, for example if we know we won't use the remote name, or the difference between local and remote, or in the case of #39, if we don't use the describe system at all, it would be in the current code one less regex and in this patch one less command called.

I also don't think that style.zsh should be there, it acts mostly like a documentation and not a default minimal implementation (in your theme you've almost overridden every style [and I do so too in the theme I'm currently building]).

[sorin-ionescu]

How would you like style HTML if you had no HTML? Would you like to write abstract CSS? style.zsh acts as a default, which you can change in your theme item by item. There are others in keyboard.zsh.

[ColinHebert]

What I mean is, I think it would be better to have a nice documentation on how to use the git-plugin-prompt and a style.zsh stripped to the bare minimum ( prompt, branch, dirty and clean [+ignore & ignore submodule]).

The idea of having a style.zsh being a gigantic comment file comes from the fact that OMZ needs a documentation in the code.
If a real documentation is made for the plugin (Markdown/wiki/both?) then, there is no need for most of the content of style.zsh

Of course you're right we need a default behaviour, this way someone just using ${git_prompt_info} with nothing else in the theme already have a result, but once the git_prompt starts to be customised, I think that a documentation on how to do would be better than "default values".

[sorin-ionescu]

I prefer for plugins to be documented in a README file inside of the plugin directory. Documentation should always accompany source code. Users should not have to hunt for wikis. However, the newly enabled wiki is useful for keeping the main README.md short.

Another option, which will unfortunately make styling more verbose but more flexible and similar to vcs_info is to have something like bellow.

Turn:

# %a - Indicator to notify of added files.
zstyle ':omz:plugin:git:prompt' added 'added:%a'

Into:

# %a - Indicator to notify of added files.
zstyle ':omz:plugin:git:prompt:added' format 'added:%a'
zstyle ':omz:plugin:git:prompt:added' enable 'yes'

[sorin-ionescu]

This prompt is an example of what porcelain commands need to be run to acquire the same information provided by git status --short --branch.

[ColinHebert]

Yes, it seems they are the same as the one I use.

git rev-parse --verify HEAD@{upstream} --symbolic-full-name  // Mine (The HEAD could be removed)
git rev-parse --verify ${hook_com[branch]}@{upstream} --symbolic-full-name // Their

git rev-list --count --left-right @{upstream}...HEAD //Mine (ahead and behind)
git rev-list ${hook_com[branch]}@{upstream}..HEAD //Their (ahead)
git rev-list HEAD..${hook_com[branch]}@{upstream} //Their (behind)

[sorin-ionescu]

What do you think of the zstyles I wrote two days ago?

[ColinHebert]

I like the idea of enabling/disabling some part of the style, if it's possible to do that on each git command used in git_info instead of each format.

zstyle ':omz:plugin:git:prompt:status' enable 'yes'
zstyle ':omz:plugin:git:prompt:action' enable 'yes'
zstyle ':omz:plugin:git:prompt:branch' enable 'yes'
zstyle ':omz:plugin:git:prompt:remote' enable 'yes'
zstyle ':omz:plugin:git:prompt:synchro' enable 'yes'
zstyle ':omz:plugin:git:prompt:describe' enable 'yes'
zstyle ':omz:plugin:git:prompt:stash' enable 'yes'

Plus it's not that verbose if it's used like this:

 zstyle ':omz:plugin:git:prompt:*' enable 'yes'

[sorin-ionescu]

You'll have to rebase it since it no longer cleanly merges.

[ColinHebert]

That should be okay now.

[ColinHebert]

I found another issue with the git status -s -b, it seems it doesn't print the remote's name if there is no new commits.

See http://i.imgur.com/mh88O.png

[sorin-ionescu]

We're changing to porcelain commands. I'll go through your patch later today. We'll look at enable/disable if performance is a problem later.

You might have to rebase once more when I merge in the #41 patch, which also fixes a remote related bug.

[ColinHebert]

Updated to match #41 changes

[sorin-ionescu]

Are you sure? It still can't be merged cleanly.

[ColinHebert]

Hum, I pushed just before 854c67a

I'll rebase right away

[sorin-ionescu]

The remote should only show when you are on a branch, right?

[ColinHebert]

Exactly, you can't have a remote-branch without a local-branch.

[sorin-ionescu]

Is there a risk that upstream_diff_cmd may fail in future Git versions? Should we split ahead and behind (2 calls)?

[ColinHebert]

I don't think there is such a risk (plumbing methods don't change that radically), and doing two requests here could take more time (depending on the code implementation): if git has to go through the commits-tree two time to get its info, that could be slower.

I think that if upstream_diff_cmd fails at some point, it can easily be fixed, so I don't worry too much about that.

[sorin-ionescu]

Do you know how recent --left-right has been added to git rev-list? I do not want to make it dependent on a too new Git.

for i in {1..5}; time (git rev-list --count --left-right '@{upstream}'...HEAD > /dev/null)
0.00s user 0.01s system 72% cpu 0.014 total
0.00s user 0.01s system 67% cpu 0.014 total
0.00s user 0.01s system 75% cpu 0.013 total
0.00s user 0.01s system 69% cpu 0.014 total
0.00s user 0.01s system 79% cpu 0.013 total

for i in {1..5}; time (git rev-list --count '@{upstream}'..HEAD > /dev/null; git rev-list --count HEAD..'@{upstream}' > /dev/null)
0.01s user 0.01s system 74% cpu 0.026 total
0.01s user 0.01s system 80% cpu 0.024 total
0.01s user 0.01s system 81% cpu 0.023 total
0.01s user 0.01s system 80% cpu 0.023 total
0.01s user 0.01s system 82% cpu 0.022 total

[ColinHebert]

It's here at least since 1.5.1.1