Make LSC command does not accept WITHMATCHLEN or MINMATCHLEN options when used without IDX, as it does not make sense.

[Shivshankar-Reddy]

ref: https://github.com/redis/redis/pull/12387

WITHMATCHLEN and MINMATCHLEN options only makes sense when used with IDX, But still we are allowing it without IDX. Without IDX its doesn't matter and creates the confusion

Current Behaviour

127.0.0.1:6379> mget k1 k2
1) "mynameisrandomhello"
2) "mygateisnamehellorandom"

127.0.0.1:6379> lcs k1 k2 MINMATCHLEN 3 idx
1) "matches"
2) 1) 1) 1) (integer) 14
         2) (integer) 17
      2) 1) (integer) 12
         2) (integer) 15
   2) 1) 1) (integer) 5
         2) (integer) 7
      2) 1) (integer) 5
         2) (integer) 7
3) "len"
4) (integer) 13
127.0.0.1:6379> lcs k1 k2
"myaeisnmhello"
127.0.0.1:6379> lcs k1 k2 MINMATCHLEN 4
"myaeisnmhello"
127.0.0.1:6379> lcs k1 k2 MINMATCHLEN 2
"myaeisnmhello"
127.0.0.1:6379> lcs k1 k2 LEN MINMATCHLEN 4
(integer) 13

127.0.0.1:6379> lcs k1 k2 IDX WITHMATCHLEN MINMATCHLEN 3
1) "matches"
2) 1) 1) 1) (integer) 14
         2) (integer) 17
      2) 1) (integer) 12
         2) (integer) 15
      3) (integer) 4
   2) 1) 1) (integer) 5
         2) (integer) 7
      2) 1) (integer) 5
         2) (integer) 7
      3) (integer) 3
3) "len"
4) (integer) 13
127.0.0.1:6379> lcs k1 k2 WITHMATCHLEN
"myaeisnmhello"
127.0.0.1:6379> lcs k1 k2 WITHMATCHLEN LEN
(integer) 13
127.0.0.1:6379> lcs k1 k2 WITHMATCHLEN MINMATCHLEN 3
"myaeisnmhello"

What I was expecting some error mentioning that it can only used with IDX ,

This PR Covers

Makes WITHMATCHLEN and MINMATCHLEN mutually inclusive with IDX Covers 1 missing testcase when len and idx is given together.

127.0.0.1:6379> lcs k1 k2 withmatchlen minmatchlen 1
(error) ERR WITHMATCHLEN and MINMATCHLEN can only be used with IDX.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 68.48%. Comparing base (443d80f) to head (2509c38). Report is 3 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #420      +/-   ##
============================================
+ Coverage     68.44%   68.48%   +0.03%     
============================================
  Files           109      109              
  Lines         61671    61674       +3     
============================================
+ Hits          42212    42236      +24     
+ Misses        19459    19438      -21     

Files	Coverage Δ
src/t_string.c	96.95% <100.00%> (+0.42%)	:arrow_up:

... and 12 files with indirect coverage changes

[madolson]

Does anybody use the LCS? I would rather not introduce a breaking change since I'm not convinced people use this command.

[Shivshankar-Reddy]

I am not sure about popularity of the command, However, this command seems to be useful in DNA and RNA analysis as per ref: https://redis.io/blog/diving-into-redis-6/#:~:text=Meet%20the%20longest,isn%E2%80%99t%20supported%20everywhere., https://twitter.com/antirez/status/1245377580880060422 , If you think this is not important at this point I am good with any decision. Thanks!

[madolson]

I am not sure about popularity of the command, However, this command seems to be useful in DNA and RNA analysis as per ref: https://redis.io/blog/diving-into-redis-6/#:~:text=Meet%20the%20longest,isn%E2%80%99t%20supported%20everywhere., https://twitter.com/antirez/status/1245377580880060422 , If you think this is not important at this point I am good with any decision. Thanks!

I think it's possible someone might use the algorithm, I just don't believe it would realistically be used in Valkey. I'm going to mark this as to-be-closed, so someone can jump in and argue to improve it if they want.