Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parse_git_dirty() when status.branch is set. #2928

Merged
merged 2 commits into from Nov 6, 2014

Conversation

simonbuchan
Copy link
Contributor

Also some cleanups.

@ncanceill
Copy link
Contributor

Seems legit to me, but needs a bit more testing before I pull it into #2790 — by the way, please send your git plugin PRs directly to me instead of upstream, it allows me to cleanly aggregate them.

@mcornella @kipanshi @Kriechi can you please have a look?

@mcornella
Copy link
Member

Basically this PR does 3 things:

  • change git status command from -s (short-format) to --porcelain, which is script-compatible.
  • change git_compare_version(), presumably because it didn't work correctly (I recall seeing and discussing this somewhere else)
  • clean up flags and variables and such

The cleanup is correct and doesn't need testing, but the other 2 do need testing. I'll try to find the issue regarding version comparing: #2386 (specifically LFDM@02f9513); and see if parse_git_dirty works with various cases.

Also, notice that this modifies lib/git.zsh instead of the git plugin.

@simonbuchan
Copy link
Contributor Author

Thanks for comments, guys. Sorry for mixing a bit too much in here (PR title should be "Fixes for parse_git_dirty()"?)

@mcornella : You are saying this is merging to the correct place? I did assume lib/ stuff should be to the base repo, though I was surprised that the git_* GIT_* definitions were not in the git plugin myself.

@simonbuchan
Copy link
Contributor Author

Closing in favor of #2386, which I can start testing with (and get mergable)

@simonbuchan
Copy link
Contributor Author

Reopening on @mcornella's recommendation in #3127 (and this is the last time I open a PR from master 😦)

@simonbuchan simonbuchan reopened this Oct 26, 2014
@mcornella
Copy link
Member

/cc @robbyrussell: works perfectly and solves #3126

@c10l
Copy link

c10l commented Oct 30, 2014

Looks good to me! 👍

@robbyrussell robbyrussell merged commit 2927ce3 into ohmyzsh:master Nov 6, 2014
mcornella added a commit to mcornella/ohmyzsh that referenced this pull request Nov 19, 2014
Commit 81004df reverted the change
in 9b811fb when editing the merge
conflict from ohmyzsh#2928.

This commit fixes that so that we don't make the same mistake again.

First seen in http://git.io/Cdaj5Q
benjaoming pushed a commit to benjaoming/oh-my-zsh that referenced this pull request Nov 28, 2014
Commit 81004df reverted the change
in 9b811fb when editing the merge
conflict from ohmyzsh#2928.

This commit fixes that so that we don't make the same mistake again.

First seen in http://git.io/Cdaj5Q
fhenrysson pushed a commit to fhenrysson/oh-my-zsh that referenced this pull request Dec 17, 2014
Commit 81004df reverted the change
in 9b811fb when editing the merge
conflict from ohmyzsh#2928.

This commit fixes that so that we don't make the same mistake again.

First seen in http://git.io/Cdaj5Q
TonyLovesDevOps pushed a commit to TonyLovesDevOps/oh-my-zsh that referenced this pull request Dec 30, 2014
Commit 81004df reverted the change
in 9b811fb when editing the merge
conflict from ohmyzsh#2928.

This commit fixes that so that we don't make the same mistake again.

First seen in http://git.io/Cdaj5Q
riccardomerolla pushed a commit to riccardomerolla/oh-my-zsh that referenced this pull request Jan 12, 2015
Commit 81004df reverted the change
in 9b811fb when editing the merge
conflict from ohmyzsh#2928.

This commit fixes that so that we don't make the same mistake again.

First seen in http://git.io/Cdaj5Q
forivall pushed a commit to forivall/oh-my-zsh that referenced this pull request Apr 26, 2015
Commit 81004df reverted the change
in 9b811fb when editing the merge
conflict from ohmyzsh#2928.

This commit fixes that so that we don't make the same mistake again.

First seen in http://git.io/Cdaj5Q
hbpoison pushed a commit to hbpoison/oh-my-zsh that referenced this pull request Aug 14, 2015
Commit 81004df reverted the change
in 9b811fb when editing the merge
conflict from ohmyzsh#2928.

This commit fixes that so that we don't make the same mistake again.

First seen in http://git.io/Cdaj5Q
NobbZ pushed a commit to NobbZ/oh-my-zsh that referenced this pull request May 15, 2016
Commit 81004df reverted the change
in 9b811fb when editing the merge
conflict from ohmyzsh#2928.

This commit fixes that so that we don't make the same mistake again.

First seen in http://git.io/Cdaj5Q
fforw pushed a commit to fforw/oh-my-zsh that referenced this pull request Feb 21, 2017
Commit 81004df reverted the change
in 9b811fb when editing the merge
conflict from ohmyzsh#2928.

This commit fixes that so that we don't make the same mistake again.

First seen in http://git.io/Cdaj5Q
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants