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

Remove dependency term-ansicolor #43

Closed
wants to merge 1 commit into from
Closed

Remove dependency term-ansicolor #43

wants to merge 1 commit into from

Conversation

joefiorini
Copy link

Per the thread at http://groups.google.com/group/cukes/browse_thread/thread/4ffa442cdfb7ffac this commit embeds term-ansicolor under the Cucumber namespace and removes it from the gemspec.

This dependency has caused problems for many developers who have a class
named "Term", which is a very ambiguous word. This commit brings
Term::ANSIColor into Cucumber and nests it under the Cucumber namespace.
@aslakhellesoy
Copy link
Contributor

I think we should use the more light-weight AnsiEscapes module in Gherkin: https://github.com/cucumber/gherkin/blob/master/lib/gherkin/formatter/ansi_escapes.rb

@joefiorini
Copy link
Author

Matt, the problem we ran into was that we have a model called Term that conflicted with the Term module in Term::AnsiColor. I'm not sure your suggestion would fix that. Wouldn't requiring lib/cucumber/term/ansicolor just pollute the global namespace again?

Aslak, that sounds like a good idea, but Term::AnsiColor has a little more functionality than AnsiEscapes (escaping colored strings, turning coloring on and off). We would need to encapsulate that functionality somewhere else in order to make that work. I think this is the ideal solution. If I get some time I'll look into it further.

Thanks guys!

@srogers
Copy link

srogers commented Sep 16, 2011

Has this gone anywhere lately? I have the same problem with a Term model in my application. I would imagine that's a pretty common model in education and finance. It sure would be great to have this fixed in Cucumber. Glad to help out if there's a way.

@aslakhellesoy
Copy link
Contributor

Term::Ansicolor is GPL, so we should never have used it to begin with. And we cannot accept this pull request because it violates the GPL Fixing this should get rid of GPL code as well.

Fixing this issue doesn't have high priority for me, but if anyone wants to give it a stab, I suggest reusing the ANSI stuff we have in gherkin:

https://github.com/cucumber/gherkin/blob/master/lib/gherkin/formatter/ansi_escapes.rb

@mulderp
Copy link

mulderp commented Sep 22, 2011

Ok... I am taking a look whether I can take the code from Gherkin

@mulderp
Copy link

mulderp commented Sep 22, 2011

Ok, I took the Ansi_escapes module from Gherkin and quickly patched it into Cucumber.
Maybe someone can take a quick look and feedback whether the patch needs some more changes:

https://github.com/poseid/cucumber/commit/e0da6cb20c5dba5cdbf443312b2c7ec533644c4a

(I also removed the dependency in the Gemspec)

@aslakhellesoy
Copy link
Contributor

@Poseid - thanks a lot. Are all the features passing?

@mulderp
Copy link

mulderp commented Sep 22, 2011

no, that is why i am just checking some issues in ast/table and Cucumber::Cli::Configuration

if these are pass, i give you a pull request

@mulderp
Copy link

mulderp commented Sep 22, 2011

Hmm, sorry, my modifications would need some work.
I see 3 problems with the new coloring strategy right now:

  1. the term based cucumber has calls to Term::ANSIColor.coloring?
    From the old cucumber code, it looks like this switch defines that colors are taken from the console, saved, replaced by Term colors, and switch back to original colors

  2. There is a block in AnsiEscapes on line 78 :

    ALIASES[key].split(',').map{|color| COLORS[color]}.join('')

causes an error wrong number of arguments (1 for 0)

  1. And last:

undefined method comment' for classModule' (NameError)
undefined method comment' for classModule' (NameError)
/Users/mulder/.rvm/gems/ruby-1.8.7-p174@foo_business/bundler/gems/cucumber-d0e72127f18d/lib/cucumber/formatter/console.rb:14:in method' /Users/mulder/.rvm/gems/ruby-1.8.7-p174@foo_business/bundler/gems/cucumber-d0e72127f18d/lib/cucumber/formatter/console.rb:14 /Users/mulder/.rvm/gems/ruby-1.8.7-p174@foo_business/bundler/gems/cucumber-d0e72127f18d/lib/cucumber/formatter/console.rb:181:incall'
/Users/mulder/.rvm/gems/ruby-1.8.7-p174@foo_business/bundler/gems/cucumber-d0e72127f18d/lib/cucumber/formatter/console.rb:181:in default' /Users/mulder/.rvm/gems/ruby-1.8.7-p174@foo_business/bundler/gems/cucumber-d0e72127f18d/lib/cucumber/formatter/console.rb:181:in[]'
/Users/mulder/.rvm/gems/ruby-1.8.7-p174@foo_business/bundler/gems/cucumber-d0e72127f18d/lib/cucumber/formatter/console.rb:181:in format_for' /Users/mulder/.rvm/gems/ruby-1.8.7-p174@foo_business/bundler/gems/cucumber-d0e72127f18d/lib/cucumber/formatter/console.rb:30:informat_string'
/Users/mulder/.rvm/gems/ruby-1.8.7-p174@foo_business/bundler/gems/cucumber-d0e72127f18d/lib/cucumber/formatter/pretty.rb:219:in print_feature_element_name' /Users/mulder/.rvm/gems/ruby-1.8.7-p174@foo_business/bundler/gems/cucumber-d0e72127f18d/lib/cucumber/formatter/pretty.rb:94:inbackground_name'

@mattwynne mattwynne closed this in 6040d0a Mar 3, 2012
@mattwynne
Copy link
Member

@aslakhellesoy I agree we should use the Gherkin AnsiEscapes eventually, but @joefiorini's fix is a step in the right direction.

@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants