[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Chicken-users] [Chicken-hackers] [PATCH] Fix substring not checking
Re: [Chicken-users] [Chicken-hackers] [PATCH] Fix substring not checking its number of arguments
Sun, 10 Nov 2013 17:57:22 +0100
On Sun, Nov 10, 2013 at 05:47:33PM +0100, Michele La Monaca wrote:
> Hi all,
> substring is picky towards string indexes, not so about the number of its
> arguments. Es:
> (substring "abc" 1 2 (sleep 10))
> This patch tries to remedy the situation. While at that I also removed tabs
> and trailing whitespaces.
Thanks for attempting to fix the situation. However, it seems rather
pointless to me to fix only this procedure. There are numerous other
procedures which have the same problem. If this is to be fixed, I think
all procedures should be fixed (and that's a shitload).
Besides, why such a huge patch with explicit argument checkers? It
makes more sense to just use #!optional, which will automatically cause
a check to be inserted. It's simpler, shorter and less error-prone.
Finally, you before you try to "fix" things like leading tabs, please
check the rest of the codebase first. "fixing" this causes a massive
inconsistency with the rest of the codebase: tabs are used everywhere.
I also think tabs are evil, but I don't want to start a tabs vs spaces
flamewar on these lists. That's just how our current code style is, and
you should try to alter that in only one place. Trailing spaces should
really be removed, but there are a few left in the code, and it's good to
get rid of them as we find them.