`current_branch` contains color escape codes #427

mhoyer · 2019-11-08T16:07:12Z

`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 · 2020-01-10T08:53:35Z

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 · 2020-01-20T07:31:43Z

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

jcouball · 2020-01-20T10:16:04Z

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

jcouball · 2020-01-22T21:16:12Z

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 · 2020-01-29T17:17:17Z

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 · 2020-02-07T22:31:25Z

@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 · 2020-02-07T23:12:04Z

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 · 2020-02-07T23:13:09Z

What is rake sv:prepare ?

mhoyer · 2020-02-10T10:07:04Z

@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 · 2020-02-10T20:46:06Z

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.

Signed-off-by: Marcel Hoyer <m.hoyer@cid.com>

mhoyer · 2020-02-10T21:03:00Z

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!

…es (ruby-git#427) Signed-off-by: Marcel Hoyer <m.hoyer@cid.com>Signed-off-by: Agora Security <github@agora-security.com>

mhoyer force-pushed the patch-1 branch from 1df6013 to ba84ab7 Compare November 11, 2019 08:34

stale bot added the wontfix label Jan 10, 2020

stale bot closed this Jan 17, 2020

jcouball reopened this Jan 20, 2020

jcouball removed the wontfix label Jan 22, 2020

jcouball mentioned this pull request Jan 29, 2020

Diff parsing fails when filenames contain unicode characters #418

Closed

mhoyer force-pushed the patch-1 branch 2 times, most recently from 2deb521 to b13537c Compare February 10, 2020 11:25

fix issue with color escape codes after recent update of git binaries

455b0b7

Signed-off-by: Marcel Hoyer <m.hoyer@cid.com>

mhoyer force-pushed the patch-1 branch from b13537c to 455b0b7 Compare February 10, 2020 21:01

jcouball approved these changes Feb 10, 2020

View reviewed changes

jcouball merged commit c10ca28 into ruby-git:master Feb 10, 2020

jcouball mentioned this pull request Apr 19, 2020

Release v1.7.0 #468

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`current_branch` contains color escape codes #427

`current_branch` contains color escape codes #427

Uh oh!

mhoyer commented Nov 8, 2019

Uh oh!

stale bot commented Jan 10, 2020

Uh oh!

mhoyer commented Jan 20, 2020

Uh oh!

jcouball commented Jan 20, 2020

Uh oh!

jcouball commented Jan 22, 2020

Uh oh!

jcouball commented Jan 29, 2020

Uh oh!

mhoyer commented Feb 7, 2020 •

edited

Loading

Uh oh!

jcouball commented Feb 7, 2020

Uh oh!

jcouball commented Feb 7, 2020

Uh oh!

mhoyer commented Feb 10, 2020

Uh oh!

jcouball commented Feb 10, 2020

Uh oh!

mhoyer commented Feb 10, 2020

Uh oh!

jcouball left a comment

Uh oh!

Uh oh!

current_branch contains color escape codes #427

current_branch contains color escape codes #427

Uh oh!

Conversation

mhoyer commented Nov 8, 2019