UI review: Key.id() was: jshell commands

[Robert Field]

I have created —
JDK-8130452 <https://bugs.openjdk.java.net/browse/JDK-8130452>
and
JDK-8130453 <https://bugs.openjdk.java.net/browse/JDK-8130453>

for tool-ids and temp-var naming API 

-Robert

> On Jul 2, 2015, at 12:47 AM, Robert Field <robert.field at oracle.com> wrote:
> 
>> 
>> On Jul 1, 2015, at 3:31 PM, Brian Goetz <brian.goetz at oracle.com <mailto:brian.goetz at oracle.com>> wrote:
>> 
>>> All the above are API level.  The API doesn’t know anything about start-up files or list filtering. Or commands for that matter.
>> 
>> And, the mapping from "id" as seen by the command-line shell in commands like /drop need not match the IDs in the API.  This discussion is about "what should the user see.”
> 
> Absolutely, we can map Key or id to anything we want, but we would need to do so keeping the semantics I outlined in mind.
> 
>> 
>> The user sees that every command entered has a (currently numeric) "handle", and some commands will take such handles as input.  /list and /list all shows known snippets along with their handles; /list all shows more snippets than /list.
>> 
>> Still, from a user-model perspective, I think it's a bug that
>> 
>> repl> 1 + 1
>> repl> 2 + 2
>> repl> /list
>> 
>> yields
>> 
>> 9: 1+1
>> 10: 2+2
>> 
>> It's a total WTF for the user.
> 
> Yes it is.  And there are two ways about that.  Best, of course, would be to eliminate the problem.  Next best is to explain it by some means (like your elided note)
> 
>> 
>> There are a few reasons why something doesn't show up in /list:
>> - It was part of the startup
>> - It was a bogus snippet
>> - It replaces an existing key
>> 
>> Is that complete?
> 
> Or it was dropped.
> 
>> 
>>> (5) /list, by default, shows interactively entered active (not failed, dropped, or withdrawn) snippets in id() order.
>>> (6) That means, even if there are no start-up entries (-nostartup), /list won’t necessarily start with “1” since your first entries may have failed.
>>> (7) The last entered snippet might not be the last listed in /list if it has the same Key of a previously entered snippet.
>>> (8) /list all — shows snippets from the start-up, from /open, it shows failed, dropped, and withdrawn snippets.
>> 
>> None of these seem like good arguments against the proposed approach. If the user enters well-behaved snippets, snippets will start at (1), and temporaries will line up with "line numbers".  The /list header text can be amended to:
>> 
>> repl> /list
>> 
>>   [ some entries suppressed, enter /list all to see ]
>>   1: 1+1
>>   3: $1 + 1
>> 
>> repl> /list all
>> 
>>   startup.1: import java.util.*
>>   1: 1+1
>>   2: blargle(*$#$^N%#$)(
>>   3: $1 + 1
>> 
>> possibly adorning "2" with something indicating its status.
> 
> But now let’s apply your suggestion to the example/challenge I proposed --
> 
> Start-up:
> 
> 	int x = 5;
> 
> Interactive:
> 
> 	snurble snurble;
> 	int y = 3;
> 	int x = y * y;
> 	/list
> 
> With a straight key mapping and two tool-id namespaces, /list would show:
> 
>  2:  int y = 3;
> 
> Because /list all would be 
> 
>  startup.1: int x = y * y;
>  1: snurble snurble;
>  2:  int y = 3;
> 
> If we create a new tool-id for each snippet (thus having many tool-ids to one Key/id()) we could get:
> 
>  2: int y = 3;
>  3: int x = y * y;
> 
> but we still aren’t starting at 1 because of the bogus input.  If bogus input also has its own namespace it would be what we “want”, where /list all would be:
> 
>  bogus.1:snurble snurble
>  1: int y = 3;
>  2: int x = y * y;
> 
> (since the user should be able to use the start-up or bogus tool-ids, and since they should be differentiated from program identifiers, these should probably be suffixes and shorter)
> 
> But now we have a new problem, if the API uses id() to generate temp variables they won’t line up at all.
> 
> Since we can’t reasonably hide/transform the generated name, the only approach I could see for aligning temp var names with tool-ids is to add a callback to the API to generate the temp var name.  Maybe:
> 
>   void setTempVariableNameGenerator(BiFunction<Integer, Integer, String> generator);
> 
> Kinda ugly, but it should do the job.
> 
> -Robert