Skip to content

Make list output flush so that can be piped.#64

Merged
sferik merged 3 commits intosferik:masterfrom
timvdalen:master
Oct 29, 2014
Merged

Make list output flush so that can be piped.#64
sferik merged 3 commits intosferik:masterfrom
timvdalen:master

Conversation

@timvdalen
Copy link
Copy Markdown
Contributor

A very small commit, but made a pull request for #62.

timvdalen and others added 2 commits June 2, 2012 17:22
The other output options already flush.
Merge sferik/t into timvdalen/t
@travisbot
Copy link
Copy Markdown

This pull request passes (merged 62b2f7b into f473c02).

@sferik
Copy link
Copy Markdown
Owner

sferik commented Aug 14, 2012

Are you sure this is the correct (and only) place to call STDOUT.flush? If you look at latest version of stream.rb, it never calls print_table_with_headings. It does, however, call print_table (from thor), print_message, and print_csv_status.

It would be nice to have some tests to go along with this code.

@timvdalen
Copy link
Copy Markdown
Contributor Author

I haven't had time to test this with the latest version. I think I will be able to do that tomorrow. For the master version two months ago, this was the only place it needed.

What I remember from my testing was that the CSV gem/lib flushed the output itself.

@sferik
Copy link
Copy Markdown
Owner

sferik commented Aug 14, 2012

Cool. The streaming stuff isn't very well tested at the moment but @spagalloco is working on a branch for that.

@sferik
Copy link
Copy Markdown
Owner

sferik commented Nov 11, 2012

As far as I can tell, this piping output (both streaming and non-streaming) works in the latest version of the gem. Please re-open this issue with your specific use case if you're still having this problem.

@sferik sferik closed this Nov 11, 2012
@sferik
Copy link
Copy Markdown
Owner

sferik commented Nov 11, 2012

I just re-read #62, which appears to be a valid use case for this patch.

My only remaining concern is that this is incomplete. It needs to work for the default output format and for CSV output, not just for the table view.

@sferik sferik reopened this Nov 11, 2012
@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 5d98563 on timvdalen:master into * on sferik:master*.

sferik added a commit that referenced this pull request Oct 29, 2014
Make list output flush so that can be piped.
@sferik sferik merged commit 65e7e02 into sferik:master Oct 29, 2014
@sferik sferik mentioned this pull request Oct 29, 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.

4 participants