RFR 8143006: JShell tool: EditPad doesn't process each line as same as inputs for jshell

[ShinyaYoshida]

Thank you for your review.

Yes, interwining input processing is not appeared by /edit.
But, it is already appeared by /open on jshell.
I think it will not be a problem because JShellTool#run and the callee are
already well-considered to implement /open.

In my patch, the command is disallowed from only /e.
It will be possible fix to disallow the command from /open.

In the shouldBeProcessed(), I check that the input which will be processed
is changed.
If it is not changed, then it will not be processed:

In shouldBeProcessed
+                        @Override
+                        public boolean shouldBeProcessed(CompletionInfo
ci) {
+                            return !failed &&
!currSrcs.contains(trimNewlines(ci.source));
                         }

In processSource
+            if (input.shouldBeProcessed(an)) {
+                boolean failed = processCompleteSource(an.source);
+                if (failed) {

That's very good! Enjoy and Have a good vacation!

Regards,
shinyafox(Shinya Yoshida)

2015-11-16 17:03 GMT+09:00 Robert Field <robert.field at oracle.com>:

> I like that this has shifted the usage pattern to a more consistent
> mechanism.  I'm concerned that this intertwines input processing with
> IOContext in a way that it did not before, this breaking encapsulation.
>
> It is good that this patch disallows command entry from Edit Pad /
> external editors.
>
> One of the characteristics of editor result processing is that only
> snippets which have been changed are reprocessed, I don't see that in this
> code, but that may be the little screens in using.
>
> I'm taking a little for day vacation before returning to the U.S. I'll be
> able to look at this in more depth on my return.
>
> -Robert
>
> On November 16, 2015 3:32:35 AM ShinyaYoshida <bitterfoxc at gmail.com>
> wrote:
>
>> Hi,
>> I have another fix, please review this.
>> I think this is better than before.
>>
>> http://cr.openjdk.java.net/~shinyafox/kulla/8143006/webrev.01/
>>
>> In this patch, SaveHandler calls JShellTool#run.
>> So it commits itself to deal with the code like /open or normal jshell
>> input.
>>
>> Regards,
>> shinyafox(Shinya Yoshida)
>>
>> 2015-11-15 2:43 GMT+09:00 Robert Field <robert.field at oracle.com>:
>>
>>> Thanks for catching this.
>>>
>>> You make a good point that it is not processed like other input.  Can we
>>> factor out and share code so that it is processed with the same code?
>>>
>>> List adds missing semicolons, can we share mechanism there.
>>>
>>> The pre-existing approach where cmdEdit builds up the snippet list and
>>> the src, then the further processing is in SaveHandler seems worrisome —
>>> not to mention it forces you to interrupt the pretty stream sequencing you
>>> built.
>>>
>>> Thanks,
>>> Robert
>>>
>>> > On Nov 14, 2015, at 9:05 AM, ShinyaYoshida <bitterfoxc at gmail.com>
>>> wrote:
>>> >
>>> > Hi,
>>> > I'd like to change the behavior of SaveHandler.
>>> >
>>> > When I put following into EditPad, it makes an error:
>>> >
>>> > System.out.println("Hello")
>>> > System.out.println("World")
>>> > --
>>> > |  Error:
>>> > |  ';'がありません
>>> > |  System.out.println("hello")
>>> > |                             ^
>>> >
>>> > It is very confusing and useless.
>>> > Because when I open EditPad after running
>>> "System.out.println("Hello")" on
>>> > jshell, the text in EditPad contains "System.out.println("Hello")" and
>>> it
>>> > could be cause of error after several editing.
>>> > (it is only happen when the invoked method returns void because
>>> > JShell#cmdEdit don't put ";" to the end of statements.)
>>> >
>>> > And, above all, the user will be surprised at the input for EditPad
>>> because
>>> > it is not processed like the input for jshell.
>>> >
>>> > Could you review this?
>>> > http://cr.openjdk.java.net/~shinyafox/kulla/8143006/webrev.00/
>>> >
>>> > Bugs is here:
>>> > https://bugs.openjdk.java.net/browse/JDK-8143006
>>> >
>>> > Regards,
>>> > shinyafox(Shinya Yoshida)
>>>
>>>
>>