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

--no-ansi is not stripping colours #34

Closed
kylef opened this issue May 20, 2014 · 14 comments
Closed

--no-ansi is not stripping colours #34

kylef opened this issue May 20, 2014 · 14 comments

Comments

@kylef
Copy link
Contributor

kylef commented May 20, 2014

$ pod --version
[!] The specification of arguments as a string has been deprecated Pod::Command::Lib::Docstats: `NAME`
0.33.0

screen shot 2014-05-20 at 14 52 22

$ env
Apple_PubSub_Socket_Render=/tmp/launch-en5dW9/Render
CMD_DURATION=4.40s
COLORFGBG=7;0
DESTINATION=/var/folders/x3/rxbktkzx4xlf98p4bsn60p040000gn/T/iTerm 1.0.0.20140421 Update
EDITOR=vim
GEM_HOME=/Users/kylef/gems
HOME=/Users/kylef
ITERM_PROFILE=Default
ITERM_SESSION_ID=w2t0p0
LANG=en_GB.UTF-8
LOGNAME=kylef
LSCOLORS=Gxfxcxdxbxegedabagacad
PATH=/Users/kylef/gems/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin
PIP_REQUIRE_VIRTUALENV=true
PWD=/Users/kylef/Projects/cocode/palaver
SHELL=/usr/local/bin/fish
SHLVL=1
SSH_AUTH_SOCK=/tmp/launch-2JXa9u/Listeners
TERM=xterm-256color
TERM_PROGRAM=iTerm.app
TMPDIR=/var/folders/x3/rxbktkzx4xlf98p4bsn60p040000gn/T/
USER=kylef
__CF_USER_TEXT_ENCODING=0x1F5:0:0
__CHECKFIX1436934=1
__fish_bin_dir=/usr/local/Cellar/fish/2.1.0/bin
__fish_datadir=/usr/local/Cellar/fish/2.1.0/share/fish
__fish_help_dir=/usr/local/Cellar/fish/2.1.0/share/doc/fish
__fish_sysconfdir=/usr/local/Cellar/fish/2.1.0/etc/fish
@fabiopelosin
Copy link
Member

Closed by 6c7f6b3

@AliSoftware
Copy link
Contributor

👍

@kylef
Copy link
Contributor Author

kylef commented May 20, 2014

Sorry guys, missed something with the yellow warning:

screen shot 2014-05-20 at 15 44 26

@kylef kylef reopened this May 20, 2014
@fabiopelosin
Copy link
Member

That happens during class evaluation much earlier than CLAide::Command::run

@fabiopelosin
Copy link
Member

I don't have a well formed opinion about this yet... the options that I see are:

  • Remove the colour form the warning
  • Do nothing as it is a temporary problem
  • Refactor to print the warnings in CLAide::Command::run

None of those look optimal to me.

@fabiopelosin
Copy link
Member

I think that we shouldn't fix this as it is a temporary problem

@fabiopelosin
Copy link
Member

Closing until someone weight-ins expressing a reason about why we should fix.

@alloy
Copy link
Member

alloy commented Aug 11, 2014

It’s temporary now, but other warnings will be introduced at some point, so it’s a recurring problem.

@fabiopelosin
Copy link
Member

This problem doesn't affect all warnings but specifically this one because the warning is presented during the initialization of the concrete CLAide sub-classes: https://github.com/CocoaPods/CLAide/blob/master/lib/claide/command.rb#L502-L503

This happens before the machinery to handle the ANSI flag is setup. It is very unlikely that we encounter a similar warning. Moreover, handling it properly would require extensive changes to the architecture of CLAide (i.e. to parse and handle the --no-ansi flag before loading the classes of the concrete CLAide commands).

@fabiopelosin
Copy link
Member

Another important thing to keep in mind is that the --no-ansi has been introduced to not pollute the standard output of the executables when used by a script (not for visual preference). The warnings are printed to the standard error stream and thus it is less relevant to strip the ANSI codes from them, although still desiderable for visual consistency.

@alloy
Copy link
Member

alloy commented Aug 11, 2014

I was actually thinking more of buffering such early warnings in a simple array and displaying them as soon as the options were parsed and evaluated. That might still be too much, but I think that an app like AppCode does not like ANSI, ever?

@fabiopelosin
Copy link
Member

Good alternative. A bit annoying that we already do that in CocoaPods (to show warnings at the end of the output so the are more visible to the user). If we decided to go that route I would be tempted to move that logic here.

I think that an app like AppCode does not like ANSI, ever?

Why? If you don't know hot to interpret them they just look like any other stream of characters... no?

@alloy
Copy link
Member

alloy commented Aug 11, 2014

Why? If you don't know hot to interpret them they just look like any other stream of characters... no?

Yeah I have actually no clue, so never mind that :D

I’m on the fence about all of this. @kylef is right from a purist point of view of course.

@fabiopelosin
Copy link
Member

I'm generally on the purist side on this things, but is really too and edge case for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants