Topic on User talk:Erutuon/Flow

Hi, I see there is a unit test failing after your change -- line #54 has nil as the first param. Do you want to fix it?

Fixed! It looks like I didn't check the testcases after saving, so didn't notice that params.n was nil in my code.

Thanks for fixing it! Could you also add a unit test to verify your change? Something that would break the current implementation, but would pass the sandbox?

I don't know if there is any case where my changes would fix anything. It would be where msg in formatMessage has n parameters where n >= 2, and params[i] (where i < n - 1) is nil and params[j] (where j > i and j <= n) is not nil. But I have no idea if such a case exists, how to find one, or, if it might exist in the future, how to simulate it in the testcases.

@Erutuon I must be misunderstanding then the purpose of your change. If it doesn't change functionality, doesn't fix a bug, or if it is not a refactoring to simplify/clean up the code - why make the change? :) If it does introduce new functionality or fix a bug, there should be a unit test that demonstrate it.

@Yurik It was a theoretical improvement to avoid the pitfalls of Lua's odd array semantics. Unfortunately I'm not familiar enough with the module to know if it is actually useful. So it might as well be reverted unless someone who understands my explanation and the module can tell me whether it is useful.