Skip to content

Output system commands and options to be executed when in debug mode #95

Merged
merged 3 commits into from Aug 5, 2011

4 participants

@uk-ar
uk-ar commented Jun 25, 2011

When errors occured, I want to know wheter Guardfile has problem or not.
(ex. guard/guard-test#9)
So it is usefult to know the system command exactly which executed by guard.
This patch output the command and it's options
To use this

guard --dry_run

It's work for me in RMI 1.9.2p180.
How about this?

@thibaudgg
Guard member

Sounds useful, thanks! Please can you add one spec, and one line in the README/CHANGELOG.

@netzpirat

This is a nice idea, but it assumes that every guard uses the Kernel module system method. So this is a bit misleading, because you could also execute commands with %x or ` and write content through the File class. I'm fine with this, but at least it needs a clear statement in the README that points out the issue with --dry-run.

@uk-ar
uk-ar commented Jun 28, 2011

I added some specs, and some lines in README/CHANGELOG.
And added support for Kernel.#` and %x literal.
But I don't know the way to write content through the File class.
Can you show me an example or reference?

@netzpirat

In Ruby file IO is usually done through the File class:

File.open(filename, 'w') { |f| f.write(content) }

But the open method yields an IO instance. So you'll fine with the methods write and write_nonblock on the IO class.

@uk-ar
uk-ar commented Jun 29, 2011

Thank you for your explanation. But I'm sorry, I cannot understand the relation between external command execution and File/IO class.So please can you add a note about the issue in README.

@netzpirat

From my understanding a dry run should not write anything. Not all Guards uses system calls to execute an external command, some Guards uses normal Ruby libraries like guard-compass that uses Compass, which writes through standard Ruby IO to a file, hence the hint to make any IO write operations non functional for the dry run.

I mean you already figured out how to redefine a method and wrote a nice spec for it, so you'll just have to do the same for IO's write methods. Then you have a 99% dry run possibility.

One last thing I noticed: You should have no space before the closing parenthesis, so instead of (:define_method, :system ) please write (:define_method, :system).

@netzpirat

In addition you also should include FileUtils::DryRun when in dry run mode:

include FileUtils::DryRun

Then you're at 99.5% :-) Sadly I haven't found something similar for File or IO.

@uk-ar
uk-ar commented Jul 1, 2011

OK, You are right. I can understand what you say. I thought a dry run only print a command and it's options at first. But now, I think a dry run should not change files too. I will try to support write and write_nonblock in IO and to use FileUtils::DryRun.

@uk-ar
uk-ar commented Jul 3, 2011

Hi, I tried to use FileUtils::DryRun. But, I realize it is difficult for me. Here is a problem.

  • Once include FileUtils::DryRun in global, cannot exclude the FileUtils::DryRun module. So I cannnot execute specs at one time.

Do you have any ideas?

@netzpirat

Oops. Didn't think about this side effect. I think you can leave it, because you anyway don't get a 100% dry run for sure. Just add some information what dry run does to the readme.

I'm leaving today for a three week holiday, so @yannlugrin, @rymai or @thibaudgg, can you please take care of this? Thanks a lot!

@thibaudgg
Guard member

@netzpirat Ok, we'll take care of it. Thanks, enjoy your holidays!

@rymai
Guard member
rymai commented Aug 4, 2011

Hum hum, we did_'nt_ take care of this sorry... ^^

@netzpirat, are you still interested (and available) for taking care of this?

@netzpirat

I just merged this branch and spent some time playing with this feature. To be honest, I have no good feeling with it. Catching IO writes has large-scale changes in the whole Ruby interpreter and makes all the specs brittle. I know this was something I suggested, but I think I went to far with this demand and I was misguided by the term dry-run. After reading the description of this pull request again, I came to the conclusion that dry-run is probably not the right term for what the original need for this pull request was: Seeing what system commands have been executed.

So if there are no objections, I suggest to drop the dry-run option completely (since we anyway cannot guarantee this for 100%) and just output a hint like Execute command: command args* before any system call is executed when we are in debug mode.

Is this ok for you?

@rymai
Guard member
rymai commented Aug 5, 2011

I TOTALLY agree, and I must confess I was already thinking of it... Let's use UI.debug more! :)

@thibaudgg
Guard member

Sounds goods, thanks!

@netzpirat netzpirat merged commit 8d02eec into guard:master Aug 5, 2011
@uk-ar
uk-ar commented Aug 9, 2011

I'm sorry for late reply. Thank you for your merging my pull request! This is the function that I imagined first time. So I should describe this feature as debug or verbose option instead of dry-run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.