Bug 9590 - CLI commands no longer needing a question mark (?) for a query
: CLI commands no longer needing a question mark (?) for a query
Status: RESOLVED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: CLI
: 7.2.1
: PC Windows XP
: -- major (vote)
: ---
Assigned To: Unassigned bug - please assign me!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-26 13:23 UTC by Barry Gordon
Modified: 2008-10-03 14:35 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Barry Gordon 2008-09-26 13:23:14 UTC
I just started testing my Pronto Program that uses CLI after receiving many copmplaints that it stopped working during the 7.2 nightly build cycle.  I went right to the current beta of 7.2.1 and found the following

"can randomplay ?" does not return 1 or 0 as it used to, just what was sent; (i.e. "can randomplay ?") but
"can randomplay"  properly returns the status (e.g "can randomplay 1")

"albums n m tags:j ?" where n and m are proper integers (in my case lets say 100 and 9 returns what was sent "albums 100 9 tags:j ?"; but 
"albums 100 9 tags:j" returns what is expected.

Now my question is - Bug and if so when fixed? 

At the current time "player count ?" works but "player count" does not so removing it blindly is not an option

On the other hand "players 0 5" works but "players 0 5 ?" does not

"info total genres ?" works, but "info total genres" does not

My issue is that I need to know what to change in my program, that is where to remove a ? and where to retain it. Consistency would be greatly appreciated 

IMHO the ? should always indicate the query operation and the fact that the ? may no longer be needed on a command (the case for can randomplay or the albums query for cover info)) should still allow it to be used.  This maintains good backward compatibility.

If this is a desired change then where can I find the documentation about where the ? must be removed and where it must be retained?

Please advise (Quickly if possible!!)
Comment 1 Barry Gordon 2008-09-26 14:09:40 UTC
The only bug seems to be with can as the documentation clearly states that a ? is permitted when making a query, in fact I believe it states that the ? is required on the query.

In the case of things like albums, artists, etc. a question mark at the end was ignored.  That is no longer the case which I do not believe I have an issue with.

I feel it would be nice if on any command that was a query type that a question mark just before the final terminator was permitted and just ignored.  I will go through and clean up where I have superfluous question marks although it would hve been nice to be told that they would now cause an error. 
Comment 2 Barry Gordon 2008-09-27 10:55:46 UTC
Revise this so that it is just about can randomplay.  In 7.2 it works properly returning state for both cases "can randomplay" and "can randomplay ?".

In 7.2.1 it works properly for "can randomplay" returning state, but merely ecjos what is sent for "can randomplay ?".

The CLI documentation indicates that the ? is required on the query.

 
Comment 3 Adrian Smith 2008-09-28 01:48:56 UTC
Barry - the parser in 7.2.1 for cli has been updated to improve speed and more rigorously enforces the syntax previously defined.  The previous syntax only required ? for some queries - it however allowed a trailing ? on some when it was not required.

All of the syntax is described in the cli documentation web page, but the more formal definition which I suggest you look at is the addDispatch definitions in Slim/Control/Requests.pm as this defines the entries used by the parser.

Let me think about adding a fallback to the old more lax way of doing it, but I don't want to impact the speed improvement we have gained.
Comment 4 Barry Gordon 2008-09-28 10:14:07 UTC
Don't bother with the fallback if it means a sacrifice in speed.  I already changed my code.  The 7.2 version for the can command allowed for either case, with the ? or no ?.  The 7.2.1 case only allows for no ?.  I have adjusted my code so it works in both cases, and fixed all other cases where I had a ? that was not needed.

Suggest you close this bug, but do modify the documentation (CLI web manual) for the can command to remove reference to the trailing ? as it no longer works. 
Comment 5 Chris Owens 2008-09-29 09:44:04 UTC
triode if you don't have time to update the docs let us know and we will try to make alternate arrangements.
Comment 6 Adrian Smith 2008-10-03 14:07:48 UTC
Barry - having looked at the documentation and code for the cli can command, I've changed its implementation to match the documentation.  It now needs the ? on the end of the command.  I hope this doesn't cause you too much hassle, but I thought it was best to try to keep to the old definition of how it worked (which was my target with the rest of the code changes).

Updated in change 23416.
Comment 7 Barry Gordon 2008-10-03 14:31:07 UTC
Could you explicitly state which ccommands?  I assume at least the can command as that was the one that started it all.  What about a status request?  I do not believe the documentation required a "?" but having it was handled by ignoring it.  I really would not like to do it by trial and error.  
Comment 8 Barry Gordon 2008-10-03 14:32:31 UTC
I read your comment too fast.  It looks like now only the 'can' command is affected and it now matches the CLI doc.  is that correct?
Comment 9 Adrian Smith 2008-10-03 14:35:45 UTC
Yes - only changed the can command (which was implemented as a special case and I didn't realise this before)

All other commands are unchanged by this and should match the documentation.  Please let me know if you find a case which does not.