On 26/05/10 08:22, Andres P wrote:
On Tue, May 25, 2010 at 5:16 PM, Xavier Chantry
<[email protected]> wrote:
On Tue, May 25, 2010 at 10:06 PM, Andres P<[email protected]> wrote:
I was about to do that, but first I think I should talk about another
change.
`type -p foo` has a return val, so consider this:
$ time for i in {1..1000}; do [[ $(type -p sh) ]]; done
real 0m1.564s
user 0m0.160s
sys 0m0.337s
$ time for i in {1..1000}; do type -p sh&>/dev/null; done
real 0m0.166s
user 0m0.060s
sys 0m0.103s
Even though there is a negligible perfomance difference in makepkg, this
is the correct way to check for $?.
I don't understand this sentence.
negligible = 10x ?
And if your "correct" way is 10x faster, why do you say "even though" ?
This makes it sound like we need to accept a compromise (like worse
perf) for a correct behavior.
But from my understanding of your mail, you are saying that using type
-p directly is both more correct and faster.
I'm saying negligible *in* makepkg. I'm making it very clear that the
justification for a change is not performance. Obviously, the same scale
on that timediff could be applied to makepkg, but even then, the gains
from implementing my suggestion are unperceivable.
If the redir to /dev/null becomes to much of a burden to type, it could
become a function. Not to mention that the whole series of checking
for vcs in PATH, which is where most of `type p` gets used, could be
changed in to a more platable chunk.
Would there be interest in making this change?
I really think that throwing away the return val and making a string
check is poor practice.
So the only downside of using type -p sh&>/dev/null directly without
[/[[ and a subshell is that we need to type&>/dev/null ?
It does not seem like a big deal to me.
And yeah if it's used a lot, like more than 5 times, just make a function.
Just to be absolutely clear, I prefer&>/dev/null over the current way
of handling this.
This is why I proposed this. ;)
I agree with the change. Feel free to change all the "[[ $(type -p
foo) ]]" to "type -p foo > /dev/null".
In fact this seems to be best in two separate patches for me.
1) [ $(type -p foo) ] -> type -p foo > /dev/null
2) [ "$(type -t foo)" = "bar" ] -> [[ $(type -t foo) == "bar" ]]
Allan
