moxley / atom-ruby-test

Run Ruby tests, Rspec examples, and Cucumber features from Atom
MIT License
43 stars 36 forks source link

Use BufferedProcess wrapper #19

Closed dsandstrom closed 10 years ago

dsandstrom commented 10 years ago

For #13 : I got something working. I used the atom module BufferedProcess [1] that I saw being used by the run-command package [2]. It is a wrapper for child process with stdout/stderr variables.

You can take over or I will work on it more later today/tommorrow.

Ctrl-c is just a temp keymap right? Ctrl-shift-c would be better.

Also, it appears we can substitute a different shell if we are not using bash.

[1] https://github.com/atom/atom/blob/a24d1d1af7a601302b80161ec452680c29ec0ad3/src/buffered-process.coffee [2] https://github.com/kylewlacy/run-command

moxley commented 10 years ago

Thanks @dsandstrom. The problem with BufferedProcess was that it waited until the tests were done before displaying anything. See https://github.com/moxley/atom-ruby-test/commit/c4fa9dd4eac6bd94dc31bd42874e88a9cbae3f5c for the fix for that. Unless you've figured out a way to stream the output and display it as it is produced, BufferedProcess this is going to introduce a regression.

dsandstrom commented 10 years ago

BufferedProcess waits until it has a line before writing to stdout. Maybe we can make a custom one that outputs characters.

dsandstrom commented 10 years ago

It will probably have to be buffered. So it allows us a way to kill it.

dsandstrom commented 10 years ago

I got it working, but by going through that exercise, I figured out how to make yours work. I'll submit a new pr

dsandstrom commented 10 years ago

Neither ways kill the process on my computer, but it does allow me to start a new test.

dsandstrom commented 10 years ago

From Node docs

Note that while the function is called kill, the signal delivered to the child process may not actually kill it. kill really just sends a signal to a process.

You set up a process.on 'exit' to watch for the signal. But, it doesn't actually kill the ruby process.

moxley commented 10 years ago

Works good! Can you remove the console.log statements and commented out code? I'm going to start a discussion in #13 about choosing the best key binding for the cancel function.

dsandstrom commented 10 years ago

You have two options. This option includes the modified BufferedProcess. The other option does not. The other option is probably the way to go because it is cleaner. I found no real benefit in using Atom's BufferedProcess.

moxley commented 10 years ago

Replaced with #20