current_branch contains color escape codes

[mhoyer]

current_branch contains color escape codes

Similar to #30 I just found an issue with ruby-git when trying to match the string returned from current_branch with e.g. "master". I just recently found this issue and I guess it's related to a recent update of my git on my machine which came in just recently.

Your environment

• git@2.24.0.windows.2
• ruby-git@1.5.0 (also had 1.3.0 before - same issue)
• ruby@2.3.1p112 (2016-04-26 revision 54768) [i386-mingw32]

Steps to reproduce

$ irb
irb(main):002:0> require 'git'; git = Git.open '.'; git.current_branch
master
=> "\e[32mmaster\e[m"

Hence, "\e[32mmaster\e[m" is not equal to "master".

Expected behaviour

git.current_branch should return the string of the current branch without coloring.

In fact, the underlying run of git branch should include --no-color option (which is a nested call inside branches_all).

Actual behaviour

git.current_branch contains "\e[32m" color escape codes.

Your checklist for this pull request

🚨Please review the guidelines for contributing to this repository.

• Ensure all commits include DCO sign-off.
• Ensure that your contributions pass unit testing.
• Ensure that your contributions contain documentation if applicable.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[mhoyer]

Sad, that there was no reaction at all. Do you need further support here?

[jcouball]

Sorry, Marcel, we are working to get more attention on this project starting with making the stale[bot] much less aggressive.

[jcouball]

Shouldn't we ALWAYS call git in no color mode for all commands?

I think the answer is yes. If so, then we should look for a way to turn it off for all commands.

https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration#_colors_in_git

[jcouball]

Marcel, what do you think of this?

My hypothesis is that adding -c color.ui=false into the global_opts array here:

https://github.com/ruby-git/ruby-git/blob/master/lib/git/lib.rb#L945-L947

will turn off color for all commands.

If I understand it correctly, this will override the color setting for all commands that support it no matter what is in the user configuration.

https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration#_code_color_ui_code

If you agree, will you test this hypothesis and, if it works, submit a update with that implementation? Please include at least one test that asserts that this option is added to commands.

[mhoyer]

@jcouball, sadly this is not working with the global_opts array. What I tried:

• global_opts << "-c color.ui=false"
• global_opts << [ "-c", "color.ui=false" ]

Both approaches did not work. I guess, due to the escape() mapping of each array item (see: https://github.com/ruby-git/ruby-git/blob/master/lib/git/lib.rb#L951). This leads to quoting of each global option string, which breaks the invocation of the git command.

$ rake sv:prepare
rake aborted!
Git::GitExecuteError: git "-c color.ui=false" version   2>&1:unknown option: -c color.ui=false
usage: git [--version] [--help] [-C <path>] [-c <name>=<value>]
           [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
           [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--bare]
           [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
           <command> [<args>]

The only working alternative for me was an direct update of https://github.com/ruby-git/ruby-git/blob/master/lib/git/lib.rb#L953:

-      git_cmd = "#{Git::Base.config.binary_path} #{global_opts} #{cmd} #{opts} #{redirect} 2>&1"
+      git_cmd = "#{Git::Base.config.binary_path} #{global_opts} -c color.ui=false #{cmd} #{opts} #{redirect} 2>&1"

Not sure, which approach you would prefer.

Update: this would also work for me:

      global_opts = global_opts.flatten.map {|s| escape(s) }.join(' ')
+     global_opts << " -c color.ui=false"

      git_cmd = "#{Git::Base.config.binary_path} #{global_opts} #{cmd} #{opts} #{redirect} 2>&1"

[jcouball]

Let me know if this works for you.

Adding this line worked for me:

$ git diff lib/git/lib.rb
diff --git a/lib/git/lib.rb b/lib/git/lib.rb
index 6c5dbc9..8aa7d27 100644
--- a/lib/git/lib.rb
+++ b/lib/git/lib.rb
@@ -945,6 +945,7 @@ module Git
       global_opts = []
       global_opts << "--git-dir=#{@git_dir}" if !@git_dir.nil?
       global_opts << "--work-tree=#{@git_work_dir}" if !@git_work_dir.nil?
+      global_opts << ["-c", "color.ui=false"]

       opts = [opts].flatten.map {|s| escape(s) }.join(' ')

$ 

I tested by setting the global color.ui attribute to always:

$ git config --global color.ui always
$ git config --list --show-origin | grep color.ui
file:/Users/couballj/.gitconfig	color.ui=always
$ 

And then ran in a irb session to test:

$ irb -Ilib -rgit
2.6.5 :001 > require 'logger'
 => true
2.6.5 :002 > logger = Logger.new(STDOUT)
 => #<Logger:0x00007f94ec0ed7f0 @level=0, @progname=nil, @default_formatter=#<Logger::Formatter:0x00007f94ec0ed7a0 @datetime_format=nil>, @formatter=nil, @logdev=#<Logger::LogDevice:0x00007f94ec0ed6d8 @shift_period_suffix=nil, @shift_size=nil, @shift_age=nil, @filename=nil, @dev=#<IO:<STDOUT>>, @binmode=false, @mon_mutex=#<Thread::Mutex:0x00007f94ec0ed5e8>, @mon_mutex_owner_object_id=70138796141420, @mon_owner=nil, @mon_count=0>>
2.6.5 :003 > g = Git.open('.', log: logger)
I, [2020-02-07T15:08:20.435338 #32135]  INFO -- : Starting Git
 => #<Git::Base:0x00007f94ec106bb0 @logger=#<Logger:0x00007f94ec0ed7f0 @level=0, @progname=nil, @default_formatter=#<Logger::Formatter:0x00007f94ec0ed7a0 @datetime_format=nil>, @formatter=nil, @logdev=#<Logger::LogDevice:0x00007f94ec0ed6d8 @shift_period_suffix=nil, @shift_size=nil, @shift_age=nil, @filename=nil, @dev=#<IO:<STDOUT>>, @binmode=false, @mon_mutex=#<Thread::Mutex:0x00007f94ec0ed5e8>, @mon_mutex_owner_object_id=70138796141420, @mon_owner=nil, @mon_count=0>>, @working_directory=#<Git::WorkingDirectory:0x00007f94ec106728 @path="/Users/couballj/SynologyDrive/Documents/Projects/ruby-git">, @repository=#<Git::Repository:0x00007f94ec1065c0 @path="/Users/couballj/SynologyDrive/Documents/Projects/ruby-git/.git">, @index=#<Git::Index:0x00007f94ec1064d0 @path="/Users/couballj/SynologyDrive/Documents/Projects/ruby-git/.git/index">>
2.6.5 :004 > g.current_branch
I, [2020-02-07T15:08:30.181286 #32135]  INFO -- : git '--git-dir=/Users/couballj/SynologyDrive/Documents/Projects/ruby-git/.git' '--work-tree=/Users/couballj/SynologyDrive/Documents/Projects/ruby-git' '-c' 'color.ui=false' branch '-a'  2>&1
D, [2020-02-07T15:08:30.181340 #32135] DEBUG -- :   fix_diff
* master
  update_contributing
  update_maintainers
  remotes/origin/HEAD -> origin/master
  remotes/origin/develop
  remotes/origin/diff_name_status
  remotes/origin/fix_diff
  remotes/origin/integrate
  remotes/origin/internals
  remotes/origin/jcouball-patch-1
  remotes/origin/ls-files-default-location
  remotes/origin/master
  remotes/origin/test
  remotes/origin/thread_safety
  remotes/origin/update_contributing
  remotes/origin/update_maintainers
  remotes/upstream/develop
  remotes/upstream/diff_name_status
  remotes/upstream/integrate
  remotes/upstream/internals
  remotes/upstream/ls-files-default-location
  remotes/upstream/master
  remotes/upstream/test
  remotes/upstream/thread_safety
 => "master"
2.6.5 :005 >

You can see the command ran was:

git '--git-dir=/Users/couballj/SynologyDrive/Documents/Projects/ruby-git/.git' '--work-tree=/Users/couballj/SynologyDrive/Documents/Projects/ruby-git' '-c' 'color.ui=false' branch '-a'  2>&1

When I ran bundle exec rake test I did notice that one of the logging tests fail because it wasn't expecting to see the additional bits in the command line. That is an easy fix.

[jcouball]

What is rake sv:prepare ?

[mhoyer]

@jcouball, thank you for the hint. I seems there was another issue on my side which resulted in the error. I now tried your approach with global_opts << ["-c", "color.ui=false"] again and it works. 👏

I will update the PR in a second.

What is rake sv:prepare ?

That's a Rake task we created on our side to build SemanticVersioning into our git workflow. Hence, the sv:prepare task would create a release branch and therefore relies on the ruby-git gem.

[jcouball]

Everything is looking good, but the DCO sign-off isn't correct in your commit. In the DCO section of the "Some checks were not successful" section below, click on the "Details" like for instructions to amend the commit with the right information.

Once both checks complete successfully, I'll approve and merge this PR.

[mhoyer]

Sorry for the inconvenience. I hope it's now okay so far? Thank you for the support, @jcouball.

[jcouball]

Approved. Thank you for your contribution, Marcel!