tartley / rerun2

Rerun a given command (eg tests) on filesystem events
MIT License
98 stars 21 forks source link

Proposal: avoid using eval and use double-quotes #1

Closed denilsonsa closed 9 years ago

denilsonsa commented 9 years ago

I wrote this pull request directly into the browser, using GitHub web interface. I have not tested the actual changes.

I'm sending this as a proposal, as an alternative implementation. Feel free to modify (or reject) as desired.


In bash, "$@" will preserve the arguments the way they were passed.

Save this sample script as printargs.sh:

#!/bin/bash

printargs() {
    while [ -n "$1" ] ; do
        echo "$1"
        shift
    done
}

printargs $@
echo '***'
printargs "$@"

Then try:

$ printargs.sh foo bar
$ printargs.sh "foo bar"

That being said, I understand that sometimes using bash aliases in rerun might be useful, then I come up with the idea of --eval. I could have used a global variable, but I wanted to avoid polluting the environment if rerun was being sourced into the current shell.

I just threw in the ideas, have fun! :)

tartley commented 9 years ago

Brilliant. Let me play with it. Thanks.

tartley commented 9 years ago

Thanks heaps for all the input. I've been playing with this a bunch, and slept on it.

Firstly: The use of quotes around all uses of "$@" is a clear win, it fixes things like rerun 'ls "some file". I'll add this separately from this PR, just to try and clearly separate the issues.

Although of course without using the single quotes around the whole command, rerun ls "some file", this still fails. I think this latter problem is an example of a wider problem, that without the single quotes, commands are almost never run as they would be if typed at a bash prompt. I think the most useful solution to this is to specify that rerun ought to usually be run with a command given in single quotes. If a user wishes to not do this, and is willing and able to escape the command themselves so it does what they want, then fair enough, but for most uses, I just want to say rerun 'X' and have X run on every iteration as though I'd typed it in each time.

This falls down a little if COMMAND itself already contains single quotes - these will then have to be escaped. But this seems simple to do, and remember to do, compared to the alternative.

I'm happy to hear more if this seems misguided.

denilsonsa commented 9 years ago

Indeed, "$@" is 99.99% of the time what should be written, instead of $@.

Regarding the wider problem… One idea is to try to have the same behavior/semantics of other tools. Some examples:

Personally, I dislike things being (re)evaluated implicitly by commands. Partly because it is more fragile (and Python subprocess module explains it), and partly because: if something is not passed through shell, I can always explicitly call sh -c to invoke the shell; but the other way around is impossible. Maybe also because xargs by default uses whitespace, which breaks often.

In addition, re-evaluation is a mess for escaping characters. What if one of the files (expanded from * or from tab-completion) contains & or even |? Running eval or sh -c implicitly turns out to be a disaster.

So, in my head, the less re-evaluation of the command, the better.

That's why, for convenience, I suggested an option --eval to enable it when needed (similar to Python's shell=True).

Still, that's my opinion. :)

denilsonsa commented 9 years ago

Also, remember to update the code in your StackOverflow answer: https://superuser.com/a/970780/ (regarding the use of quotes)

tartley commented 9 years ago

You da best. Lots for me to think about there.

tartley commented 9 years ago

Thanks heaps for all the advice. I've decided to try not eval-ing by default, as you advise, to try it out, see if I learn something while using it. Maybe getting my head around bash quoting in order to use it that way will actually be good for me. Closing this PR, going to do without the --eval for the moment - will see if I feel I need it after a little while using rerun without it.

FWIW: I got 'treat a flurry of events as one' working, see master