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

Text-only formatter option #120

Merged
merged 1 commit into from
Apr 12, 2014
Merged

Conversation

DejaAugustine
Copy link
Contributor

A text-only (i.e. without color) formatter option that cleans up the output when using SSHKit via GitBash on Windows (or via any other terminal that does not understand ANSI color codes)

Can be used in Capistrano's deploy.rb via:

set :format, :text

@leehambley
Copy link
Member

Thanks, but please don't push your pull request. This topic has come up a few times before, and the answer is always the same. If the implementation is the Pretty formatter, with the ansi codes stripped out, that is not a solution. The problem here lies with the ansi gem that we use, term-ansicolor which needs to be fixed to work on Windows, and respect the environmental variables, and/or especially the TERM variable. The issue on their side is at flori/term-ansicolor#16, which we are dependent upon to avoid code duplication in SSHKit.

Thanks for the pull request in any case, perhaps you could look at fixing term-ansicolor instead, I tried to open a dialog with the maintainer (as you can see in the linked issue), however there was no definitive outcome of action for the next step.

@leehambley leehambley closed this Mar 10, 2014
@leehambley
Copy link
Member

If I'm mistaken about the format, please choose a better name, and post a sample of the output, but it looks way, way too much like the Pretty formatter for my taste right now.

@DejaAugustine
Copy link
Contributor Author

This is not intended as a solution to the issues with term-ansicolor, but instead provides an option where colored output in any form is not desired (one example may be logging / log parsing for Capistrano used in an automated environment).

There are no problems with the information itself, hence using the selectively verbose Pretty formatter as the basis, but rather the presentation of that information includes unnecessary formatting markers.

At the time of writing this patch, there are only three options available: 1. Black Hole (i.e. no output) 2. Dot (i.e. minimal information with formatting markers) 3. Pretty (i.e. full information with formatting markers).

There is, at present, no text-only formatter which can provide the information of Pretty without the color markers. I see "Text" as the complement to "Pretty". Not only does it provide functionality that cannot presently be achieved any other way, but it also defines "Pretty" as a colorized version of "Text". Lastly, it seems to me that the question for colorized vs plain text is genuinely a question of formatting, hence a separate formatter would be the appropriate course of action.

If you do not agree, then I would appreciate some guidance from you on what degree of formatting changes would warrant a separate formatter, or what means would be recommended to provide a clean, text-only output format for SSHKit.

Thank you.

@leehambley
Copy link
Member

This is not intended as a solution to the issues with term-ansicolor, but instead provides an option where colored output in any form is not desired (one example may be logging / log parsing for Capistrano used in an automated environment).

If TERM and $stdout.tty? would be respected by term-ansicolor this is what would happen. The colouring would only be displayed if the output was being sent to a terminal that supported (and was configured for) colours. Streaming to log files term-ansicolor would do nothing, running Capistrano from another script, term-ansicolor would do nothing, using capistrano/sshkit on a platform where POSIX colour codes are not supported (indicated by TERM) term-ansicolor should do nothing.

... This is not intended as a solution to the issues with term-ansicolor, but instead provides an option where colored output in any form is not ....

If a black and white screenshot of two formatters look the same, they shouldn't be two formatters.

I think that's fair? Do you agree? (I'm more than happy to implement the correct behaviour in term-ansicolor but the author over there doesn't seem interested in responding to issue requests)

@DejaAugustine
Copy link
Contributor Author

Ah, thank you for clarifying. I can see where you're coming from.

Would a "Simple" or "Simplified" format be an acceptable alternative as something between the absolute minimalist "Dot" and the verbose "Pretty"? Something that looks like this (assuming a DEBUG level of verbosity):

Running /usr/bin/env ls -x /home/deploy/my_app/releases on my.host
Command: /usr/bin/env ls -x /home/deploy/my_app/releases
    20140310172932  20140310174154  20140310183617  20140310183732  20140310200555
    20140311134614
Finished in 1.642 seconds with exit status 0 (successful).
Keeping 5 of 6 deployed releases on my.host
Running /usr/bin/env rm -rf /home/deploy/my_app/releases/20140310172932 on my.host
Command: /usr/bin/env rm -rf /home/deploy/my_app/releases/20140310172932
Finished in 1.148 seconds with exit status 0 (successful).

Or for Info-only

Running /usr/bin/env ls -x /home/deploy/my_app/releases on my.host
Keeping 5 of 6 deployed releases on my.host
Running /usr/bin/env rm -rf /home/deploy/my_app/releases/20140310172932 on my.host
Finished in 1.148 seconds with exit status 0 (successful).

@leehambley
Copy link
Member

If you don't mind calling it something like SimpleText, I'd be fine with merging it. In the end it's a bit of a shame to add more formatter logic (especially when Pretty is a bit of a mess, and I really wanted to clean it up!) but I can see the case. If you can rebase this commit into a single one, make your changes, and force-push, etc I'll be happy to merge it.

@leehambley leehambley reopened this Mar 11, 2014
@damphyr
Copy link

damphyr commented Mar 11, 2014

A bit late chiming in but I'll add my 2cs: A simple text (maybe PlainTextFormatter ?) is useful when automating. A good example is calling cap tasks within Jenkins. By default Jenkins does not handle ANSI codes and the logs come back a mess. There is a way around it (Jenkins has a plugin) but it is a bigger PITA.
In my case the ANSI codes mess up the logs I want to attach to test protocols - plain text is also better here.
So 👍 for Plain in addition to Pretty

@leehambley
Copy link
Member

A good example is calling cap tasks within Jenkins.

This is the point exactly of term-ansicolor needing to respect $stdout.tty?. Because under Jenkins, it isn't. (and shouldn't be)

@leehambley
Copy link
Member

In my case the ANSI codes mess up the logs I want to attach to test protocols - plain text is also better here.

Ohh, and when you redirect the output through a pipe, the $stdout.tty? should also be false. (But term-ansicolor doesn't bother to check)

@DejaAugustine
Copy link
Contributor Author

Well at least until term-ansicolor works as desired, the SimpleText formatter has been built, rebased, and pushed up for your review.

Thank you.

leehambley added a commit that referenced this pull request Apr 12, 2014
@leehambley leehambley merged commit 3b5c68b into capistrano:master Apr 12, 2014
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.

3 participants