Fix #3126 - parse_git_dirty() always returns dirty if branch = 1 on .gitconfig

[c10l]

This fixes the issue by grepping out the branch information.

[mcornella]

Hi @cassianoleal, you don't need to open a new issue detailing a problem if you already have an existing solution. That increments the number of open issues unnecessarily and makes it an artificial value with no real meaning. Please close the other issue.

As per this change, just adding the --porcelain flag will make git status output consistent regardless of the user configuration. Please update your change to use that instead of the grep solution.

Thanks for contributing!

[mcornella]

Update: wait, don't do that yet because I need to test the consequences of using the porcelain flag on git submodules.

[c10l]

@mcornella yeah, sorry about the thing with the extra issue. closed.

Didn't know about the --porcelain flag. That does seem to be the best way to fix this. Let me know once you have tested it with submodules and I'll update this PR.

Thanks!

[mcornella]

Hi @cassianoleal, I have checked the docs and it's confirmed that the porcelain flag doesn't affect the behavior (i.e., submodules aren't checked differently with or without the flag). It only affects the output format.

You can go ahead and change it, we'll get more people to test this anyway. I made a commit to test this, you can borrow from it without worry: mcornella@a205c6c

[c10l]

Done. I've tested it on my setup and it works as expected. Cheers!

[mcornella]

/cc @robbyrussell: this seems harmless enough to merge

[mcornella]

Apparently this was already posted #2928, but was closed in favor of #2386. Not sure how to proceed
How is your progress @simonbuchan?
This is good to merge while there are no other options available.
/cc @robbyrussell

[simonbuchan]

Bah, I hate github notifications. I would really like all the fixes in #2386 to go out, but it looks like @LFDM isn't tracking it.

@mcornella, @robbyrussell would you like me to open my re-merge of #2386 as a new PR to try to get that merged? It's what I've been running since and it seems OK, though I've not been exhaustive.

[rhong-fiksu]

Works like a charm for me. Moving all the git parsing stuff to using the porcelain flag seems prudent.

[mcornella]

@simonbuchan yeah #2928 looks like has a deeper cleanup of the file and also was first to solve this one. If you open it we'll merge that one, if @cassianoleal is ok with it ;)

You can also get #2386 rebased and posted as a separate PR, but there's no rush to getting it merged since it's a big refactor.

[c10l]

👍

If it fixes my issue (which it seems to do from reading the code, even though I haven't actually tried it), I'm cool with it. :)

[simonbuchan]

@mcornella, @cassianoleal - done, though note I have been running on my merge of #2386 since August, so it only got a couple months use.

[mcornella]

Ok cool @simonbuchan, we'll do it this way then. I take note of your caution in regards to #2386

@cassianoleal can you please close this PR and go +1 #2928 by @simonbuchan?

[c10l]

Closing in favour of #2928