zRange and zRevRange has inconsistent parameters

[RobbieLePommie]

Expected behaviour

zRange and zRevRange should accept an array of options ['WITHSCORES' => true] as the fourth parameter, and not the boolean.

Reasons:

• is consistent with REDIS itself
• is consistent with zRangeByScore
• is consistent with the readme where there is an Freudian slip in the documentation (section zRemRangeByRank ) where it refers to $redis->zRange('key', 0, -1, ['withscores' => TRUE]); /* ['three' => 3] */

Actual behaviour

It accepts a boolean (true/false), not array.

I'm seeing this behaviour on

Master and Developer branch. PHP 7.1 - PHP 7.3

I've checked

• There is no similar issue from other users
• Issue isn't fixed in develop branch

Other Notes

As a breaking change, I would suggest accepting both as a deprecation path. Currently prevents easy adoption of the library.

[michael-grunder]

Hi @RobbieLePommie,

I tend to agree that the inconsistency is weird, but it's just a historical anachronism at this point that predates even my involvement in the project.

There are a variety of things like this that I'd like to remove such as SortAsc, SortDesc, and a bunch of aliases (del -> delete, eval -> evaluate, etc).

Allowing users to pass either the boolean or ['withscores' => true] seems reasonable to me. @yatsukhnenko Any thoughts?

[yatsukhnenko]

@michael-grunder IMO we need to remove all anachronism from code and make API similar for all methods but from the other side if we will handle array and boolean users will swearn less 😄

[RobbieLePommie]

To add to the list:

PING returned +PONG if no parameters sent; it should return PONG. REF: https://redis.io/commands/ping

Reason for suggesting fix: IT caused automated tests to fail while adapting to the library compared to other systems.

[michael-grunder]

PING returned +PONG if no parameters sent; it should return PONG. REF: redis.io/commands/ping

I've wanted to change this one for years, and we can since we're allowed to break things for 5.0. 😄

[michael-grunder]

It's still a work in progress, but the initial commit for these changes is above in f6e92fc.

Right now I'm returning true if PING is called without arguments since phpredis has always represented simple string replies as true and I'm guessing users have come to rely on that.

@yatsukhnenko What do you think about the reply type?

[yatsukhnenko]

@michael-grunder don't you want to move changes related to PING to separated branch?

[michael-grunder]

I can, sure.

Edit: The PING flame war can be found -> #1563 😂
Edit: Branch for ZRANGE changes

[RobbieLePommie]

@michael-grunder - Thanks for addressing - it will help with migration and compatibility moving forward.