Skip to content

Use git 'plumbing' command to avoid color codes. (Fixes #26)#27

Open
brunnre8 wants to merge 1 commit intodzhou121:masterfrom
brunnre8:gitPorcelain
Open

Use git 'plumbing' command to avoid color codes. (Fixes #26)#27
brunnre8 wants to merge 1 commit intodzhou121:masterfrom
brunnre8:gitPorcelain

Conversation

@brunnre8
Copy link

@brunnre8 brunnre8 commented Jun 5, 2017

This fixes the issue with the color codes it color.ui=always is set.

There would be a arguably bettter option here recommended by a git maintainer although that is git > 1.8

The solution here works with older versions of git and should not lead to compatibility issues.

Currently the displayed name in a detached state is HEAD. Is this ok or would you then prefer the commit hash?

@brunnre8 brunnre8 changed the title Use git 'plumbing' command to avoid with color codes. (Fixes #26) Use git 'plumbing' command to avoid color codes. (Fixes #26) Jun 5, 2017
@dzhou121
Copy link
Owner

dzhou121 commented Jun 6, 2017

Thanks for the PR. I would prefer the commit hash personally. I think it gives you more info than just HEAD. Is it possible to get commit hash using this way? If not, we could probably use -c color.ui=no to turn off color for the command?

@brunnre8
Copy link
Author

brunnre8 commented Jun 6, 2017

If not, we could probably use -c color.ui=no to turn off color for the command?

We probably could yes, although one really shouldn't be relying on the string returned by the porcelain script meant for human consumption as thismay change without warning between versions according to the git devs

If I implement the commit hash lookup "the proper way" would you merge it?

@dzhou121
Copy link
Owner

dzhou121 commented Jun 6, 2017

Yeah sure. Really appreciate that.

@brunnre8
Copy link
Author

brunnre8 commented Jun 6, 2017

What's the minimum git version you want to support?

They added improved command options in 1.8 or so but this then breaks for the debian wheezy guys unless they use wheezy-backports

All other distros of note including Ubuntu are above that.
What's your opinion about that?

Supporting 1.8+ would make the code cleaner

@dzhou121
Copy link
Owner

dzhou121 commented Jun 6, 2017

That's fine for me.

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.

2 participants