Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 548356

Summary: [console] Fix input handling in IOConsole
Product: [Eclipse Project] Platform Reporter: Paul Pazderski <paul-eclipse>
Component: DebugAssignee: Paul Pazderski <paul-eclipse>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: fabiofz, loskutov, register.eclipse, sarika.sinha
Version: 4.11   
Target Milestone: 4.13 M1   
Hardware: All   
OS: All   
See Also: https://git.eclipse.org/r/144295
https://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=1962d9672d5af1a8c4d749a819be3d9784bf9ee0
https://git.eclipse.org/r/145282
https://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=999fda5f46e5566560ede1638b730d561f650ab2
Whiteboard:
Bug Depends on:    
Bug Blocks: 250306, 525966, 549366, 552030    

Description Paul Pazderski CLA 2019-06-17 15:45:37 EDT 
Probably the worst for most users is that typing in an input partition which is not the last partition (e.g. input followed by output) results in chaotic unpredictable behavior and something the content send to the process is not what is visible in console. A special variant of this problem is bug 525966 which is fixed along with this bug.

Also the more rare bug 250306 will be fixed with this.


Problematic parts of IOConsolePartitioner:

1. lastPartition. This value get out of sync way to often. From javadoc it is "The last partition appended to the document" which is equal to partitions.get(partitions.size() - 1) and after all the partitioning code shouldn't depend on the information what was added last.
A bug produced from lastPartition: in a console type "abc", let the program produce any output, clear the console, type "123" and press enter. Expected result is "123" but actual it is "abc123".
Also an out of sync lastPartition is the precondition for some of the other problems.

2. IOPartition manages a separate buffer of user input. It is an unnecessary erroneous duplication to store the content of the partition again in the partition itself. This was already a problem in bug 453407. But it is still a problem if you for example: type "abc", then the program produce output, type "123" and then type '+' inside the first "abc"-part. If you press enter the content send to the process is not what is shown in the console. Also the console coloring gets wrong. Another variant is instead of adding a character in the first part, remove one. The result is also wrong in any way.
And while bug 453407 covered SIOOBE for the insert() part it can still be triggered for delete() by starting a character composition inside an output partition.

3. Implementation of getPartition and isReadOnly. Both can magically produce new partitions and getPartition sometimes even returns a partition which don't contain the requested offset.
Comment 1 Eclipse Genie CLA 2019-06-17 15:46:44 EDT 
New Gerrit change created: https://git.eclipse.org/r/144295
Comment 3 Sarika Sinha CLA 2019-07-02 01:19:19 EDT 
@Paul, Please fix the jdt debug test fialures due to this fix.
https://download.eclipse.org/eclipse/downloads/drops4/I20190701-1805/testResults.php
Comment 4 Till Brychcy CLA 2019-07-02 02:41:06 EDT 
I have the pydev source code open in my workspace and noted that it uses
org.eclipse.ui.internal.console.IOConsolePartition.getString(), which was removed by this patch.

See org.python.pydev.debug.model.AbstractDebugTarget.addConsoleInputListener() in
https://github.com/fabioz/Pydev/blob/8fa736b95f25544da4f679c2e0f2bd5f659c7df1/plugins/org.python.pydev.debug/src/org/python/pydev/debug/model/AbstractDebugTarget.java#L932
Comment 5 Paul Pazderski CLA 2019-07-02 03:31:54 EDT 
Thanks for the hints. Will look into both.
Comment 6 Eclipse Genie CLA 2019-07-02 06:16:21 EDT 
New Gerrit change created: https://git.eclipse.org/r/145282
Comment 7 Paul Pazderski CLA 2019-07-02 06:16:53 EDT 
(In reply to Sarika Sinha from comment #3)
> @Paul, Please fix the jdt debug test fialures due to this fix.

An annoying mistake. I had a working version and in an attempt to simplify my code I broke this particular scenario.
Comment 8 Fabio Zadrozny CLA 2019-07-02 07:18:41 EDT 
@Paul Pazderski just to give some more info, as @Till Brychcy commented, PyDev relies on being able to get the text contents whenever the user types in some input (it uses the console during debugging so that the user can interact with the debugger -- any input entered/pasted in the console is evaluated by the debugger -- unless the backend is actually expecting some user input, in which case it's sent to stdin), so, what would be needed in the PyDev side is a way to get the last line entered by the user.
Comment 9 Paul Pazderski CLA 2019-07-02 08:41:39 EDT 
I see the problem. The good news first there should be an easy workaround to keep PyDev running. Apart from the cases where it run out of sync the partition's buffer contained the same content as the document part it partitioned. So this [1] change in PyDev should obtain the existing behavior.

I also tried to find a better solution for your problem but it seems in fact almost impossible to access the user input entered in console. I found a theoretical solution only relying on API methods. [2] Unfortunately the JavaDoc is (for me) not clear if it should work and with current ProcessConsole it will not work.

[1] https://github.com/fabioz/Pydev/pull/245
[2] https://github.com/PPazderski/Pydev/commit/b1fb8107625aaa2db1b06c7655f5b90a1e5f4b64
Comment 11 Paul Pazderski CLA 2019-07-03 05:13:51 EDT 
Test failure fixed and a workaround for PyDev merged. Closing again.
Comment 12 Sarika Sinha CLA 2019-07-10 03:31:28 EDT 
@Paul, 
Can you verify the changes in the I build?
Comment 13 Paul Pazderski CLA 2019-07-10 11:30:43 EDT 
Verified with I20190709-1800