Bug 548344 - [Run Configurations] Add inline table editing support for Java environment variables tab
Summary: [Run Configurations] Add inline table editing support for Java environment va...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 4.13   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.13 M1   Edit
Assignee: Andrew Obuchowicz CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy, usability
: 483084 (view as bug list)
Depends on: 548557
Blocks: 549366
  Show dependency tree
 
Reported: 2019-06-17 10:28 EDT by Andrew Obuchowicz CLA
Modified: 2019-12-04 17:29 EST (History)
10 users (show)

See Also:


Attachments
State of table after using Inline Editing (30.55 KB, image/png)
2019-06-24 23:46 EDT, Sarika Sinha CLA
no flags Details
Table gets empty after editing inline sometime (13.63 KB, image/png)
2019-06-25 04:51 EDT, Sarika Sinha CLA
no flags Details
Gif tried in new workspace (692.82 KB, image/gif)
2019-06-26 05:40 EDT, Sarika Sinha CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Obuchowicz CLA 2019-06-17 10:28:55 EDT
Currently working on a patch for this.
Comment 1 Eclipse Genie CLA 2019-06-17 16:42:14 EDT
New Gerrit change created: https://git.eclipse.org/r/144298
Comment 3 Mickael Istria CLA 2019-06-21 10:55:36 EDT
Thank you very much Andrew!
It's worth mentioning that by this change, you introduced a very interesting a reusable TextGetSetEditingSupport class that can be useful in many many (yes, 2 times many) circumstances. At some point, we'll probably be able to move it to JFace directly when the number of use-cases for this class grows.
Comment 4 Lars Vogel CLA 2019-06-21 11:03:42 EDT
Very cool, thanks Andrew. Please add to N&N.

Of topic:
As you are currently looking into inline editing, please consider implementing a inline rename functionality for the project and/or package explorer. This is a highly requested feature in the user case I met If you consider that, it should be similar to the refactor class name in JDT, first inline with an option to open the dialog.
Comment 5 Mickael Istria CLA 2019-06-21 11:05:41 EDT
(In reply to Lars Vogel from comment #4)
> As you are currently looking into inline editing, please consider
> implementing a inline rename functionality for the project and/or package
> explorer. This is a highly requested feature in the user case I met If you
> consider that, it should be similar to the refactor class name in JDT, first
> inline with an option to open the dialog.

I think it's not possible to have inline editing of trees on all OS (while it's possible for tables), but maybe I'm wrong.
Eric might be able to confirm/infirm this.
Comment 6 Andrew Obuchowicz CLA 2019-06-21 11:21:59 EDT
(In reply to Mickael Istria from comment #3)
> Thank you very much Andrew!
> It's worth mentioning that by this change, you introduced a very interesting
> a reusable TextGetSetEditingSupport class that can be useful in many many
> (yes, 2 times many) circumstances. At some point, we'll probably be able to
> move it to JFace directly when the number of use-cases for this class grows.

My pleasure Mickael, I couldn't have done it without your great suggestion :) The final implementation, using TextGetSetEditingSupport is a LOT nicer than my original implementation.

I'm excited about the idea of eventually moving it to JFace directly, keep me updated and let me know if there's anything you need me to do with regards to that.
Comment 7 Andrew Obuchowicz CLA 2019-06-21 11:39:03 EDT
(In reply to Lars Vogel from comment #4)
> Very cool, thanks Andrew. Please add to N&N.
> 
> Of topic:
> As you are currently looking into inline editing, please consider
> implementing a inline rename functionality for the project and/or package
> explorer. This is a highly requested feature in the user case I met If you
> consider that, it should be similar to the refactor class name in JDT, first
> inline with an option to open the dialog.

My pleasure Lars - I will add it to N&N shortly (probably before I go home for the weekend).

As for inline renaming in the project and/or package explorer, I definitely would like to give it a try as this is something I have thought about as well (and I believe it would be a very welcome quality-of-life improvement).

However, I will have to confirm with Eric about whether this would be possible on all OS'.
Comment 8 Sarika Sinha CLA 2019-06-23 23:30:25 EDT
This resulted in a Build Warning fixed by Bug 548557.
Comment 9 Sarika Sinha CLA 2019-06-24 23:43:07 EDT
Got this error while editing the Environment table -

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4713)
	at org.eclipse.swt.SWT.error(SWT.java:4628)
	at org.eclipse.swt.SWT.error(SWT.java:4599)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:451)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:350)
	at org.eclipse.swt.widgets.Widget.addDisposeListener(Widget.java:220)
	at org.eclipse.jface.viewers.ColumnViewerEditor.activateCellEditor(ColumnViewerEditor.java:260)
	at org.eclipse.jface.viewers.ColumnViewerEditor.handleEditorActivationEvent(ColumnViewerEditor.java:425)
	at org.eclipse.jface.viewers.ColumnViewer.triggerEditorActivationEvent(ColumnViewer.java:680)
	at org.eclipse.jface.viewers.ColumnViewerEditor.processTraverseEvent(ColumnViewerEditor.java:541)
	at org.eclipse.jface.viewers.ColumnViewerEditor.lambda$1(ColumnViewerEditor.java:245)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:269)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4141)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1056)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1080)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1065)
	at org.eclipse.swt.widgets.Control.traverse(Control.java:4268)
	at org.eclipse.swt.widgets.Control.translateTraversal(Control.java:4250)
	at org.eclipse.swt.widgets.Display.translateTraversal(Display.java:4697)
	at org.eclipse.swt.widgets.Display.filterMessage(Display.java:1216)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3549)
	at org.eclipse.jface.window.Window.runEventLoop(Window.java:823)
	at org.eclipse.jface.window.Window.open(Window.java:799)
	at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationsDialog.open(LaunchConfigurationsDialog.java:1241)
	at org.eclipse.debug.ui.DebugUITools.lambda$1(DebugUITools.java:631)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:72)
	at org.eclipse.debug.ui.DebugUITools.openLaunchConfigurationDialogOnGroup(DebugUITools.java:637)
	at org.eclipse.debug.ui.DebugUITools.openLaunchConfigurationDialogOnGroup(DebugUITools.java:575)
	at org.eclipse.debug.ui.actions.OpenLaunchDialogAction.run(OpenLaunchDialogAction.java:85)
	at org.eclipse.jface.action.Action.runWithEvent(Action.java:474)
	at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:580)
	at org.eclipse.jface.action.ActionContributionItem.lambda$4(ActionContributionItem.java:412)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4141)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1056)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3954)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3553)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1159)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1048)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:634)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:558)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:155)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:137)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:107)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:400)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:660)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:597)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1468)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1441)

Attaching the screenshot of the state of the table, table can not be reused at all after this action.
Comment 10 Sarika Sinha CLA 2019-06-24 23:46:49 EDT
Created attachment 279058 [details]
State of table after using Inline Editing
Comment 11 Sarika Sinha CLA 2019-06-24 23:48:17 EDT
@Mickael,
Please make sure this is resolved in the next build or revert it for time being and then release it after the issue is fixed.
Comment 12 Mickael Istria CLA 2019-06-25 01:43:04 EDT
@Sarika: can you tell a bit more about the steps to reproduce?
Comment 13 Sarika Sinha CLA 2019-06-25 01:49:07 EDT
I just did New Variable and then Inline edited the variable name. I tried double clicking on variable value to see if that can be edited but by then whole table was unusable.

Before that I tried right click features added to this tab via Bug 548520.
Comment 14 Andrey Loskutov CLA 2019-06-25 02:41:52 EDT
(In reply to Eclipse Genie from comment #2)
> Gerrit change https://git.eclipse.org/r/144298 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/
> ?id=4eff8d4b2f99b831a37349bf393acac911e6bea5

This causes javadoc error in releng tests, see for example https://download.eclipse.org/eclipse/downloads/drops4/I20190624-1800/testResults.php

https://download.eclipse.org/eclipse/downloads/drops4/I20190624-1800/compilelogs/platform.doc.isv.javadoc.txt

../../../eclipse.platform.debug/org.eclipse.debug.ui/ui/org/eclipse/debug/ui/EnvironmentTab.java:577: error: @param name not found
	 * @param forceOverwrite whether the user should be given a confirmation prompt
	          ^
1 error
Comment 15 Eclipse Genie CLA 2019-06-25 02:48:35 EDT
New Gerrit change created: https://git.eclipse.org/r/144791
Comment 17 Andrey Loskutov CLA 2019-06-25 03:11:03 EDT
(In reply to Andrey Loskutov from comment #14)
> (In reply to Eclipse Genie from comment #2)
> > Gerrit change https://git.eclipse.org/r/144298 was merged to [master].
> > Commit:
> > http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/
> > ?id=4eff8d4b2f99b831a37349bf393acac911e6bea5
> 
> This causes javadoc error in releng tests

Fixed via

(In reply to Eclipse Genie from comment #16)
> Gerrit change https://git.eclipse.org/r/144791 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/
> ?id=aec1476ad5841a388ad9df1df307f7469f183ca8
Comment 18 Mickael Istria CLA 2019-06-25 04:33:16 EDT
(In reply to Sarika Sinha from comment #9)
> Got this error while editing the Environment table -

Testing for ~15 minutes with random actions, I managed to see this stack once; but cannot reproduce it intentionally.
Comment 19 Sarika Sinha CLA 2019-06-25 04:50:42 EDT
(In reply to Mickael Istria from comment #18)
> (In reply to Sarika Sinha from comment #9)
> > Got this error while editing the Environment table -
> 
> Testing for ~15 minutes with random actions, I managed to see this stack
> once; but cannot reproduce it intentionally.

Yes the stack is not consistent but the unusable state of table is totally reproducible:
1. Start with an empty variable list.
2. Add a new Variable.
3. Change the variable name by inline. Table becomes unusable.
4. Add more variables, can't see the new variables in the table.
5. close and reopen the configuration dialog, we can see new variables added.
6. Tried again adding new variable and editing inline , now whole table is empty as attached in the new screenshot.
Comment 20 Sarika Sinha CLA 2019-06-25 04:51:28 EDT
Created attachment 279065 [details]
Table gets empty after editing inline sometime
Comment 21 Mickael Istria CLA 2019-06-25 05:06:07 EDT
(In reply to Sarika Sinha from comment #19)
> Yes the stack is not consistent but the unusable state of table is totally
> reproducible:
> 1. Start with an empty variable list.
> 2. Add a new Variable.
> 3. Change the variable name by inline. Table becomes unusable.
> 4. Add more variables, can't see the new variables in the table.
> 5. close and reopen the configuration dialog, we can see new variables added.
> 6. Tried again adding new variable and editing inline , now whole table is
> empty as attached in the new screenshot.

On my Linux laptop, I couldn't reproduce it with the given steps.
As I could see the stack earlier, I wouldn't say it Windows-only. However, it seems like the reproducibility depend on the environment (maybe even performances...).
Comment 22 Sarika Sinha CLA 2019-06-26 05:40:48 EDT
Created attachment 279090 [details]
Gif tried in new workspace

Attaching a gif tried in new workspace with 25th Build.
Comment 23 Mickael Istria CLA 2019-06-26 08:28:07 EDT
(In reply to Sarika Sinha from comment #22)
> Attaching a gif tried in new workspace with 25th Build.

I don't see such error on my machine.
@Andrew: any idea?
Comment 24 Andrew Obuchowicz CLA 2019-06-26 10:06:08 EDT
(In reply to Mickael Istria from comment #23)
> (In reply to Sarika Sinha from comment #22)
> > Attaching a gif tried in new workspace with 25th Build.
> 
> I don't see such error on my machine.
> @Andrew: any idea?

I haven't been able to reproduce the mentioned bugs on my Linux system (tried under X11 and Wayland).
It could potentially be a SWT bug on Windows.

Paul, could you help me debug this issue? (as you are a Windows user)

I'm going to try investigating further in the meantime.
Comment 25 Paul Pazderski CLA 2019-06-26 12:08:46 EDT
I might found the pivotal step to reproduce the error.

1. Add variable.
2. Change variable name and end edit *by pressing tab*

This give me always the widget dispose exception and the other table problems Sarika described.
Comment 26 Andrew Obuchowicz CLA 2019-06-26 12:16:32 EDT
(In reply to Paul Pazderski from comment #25)
> I might found the pivotal step to reproduce the error.
> 
> 1. Add variable.
> 2. Change variable name and end edit *by pressing tab*
> 
> This give me always the widget dispose exception and the other table
> problems Sarika described.

Thank you Paul, this is really helpful.
I'm going to look into why ending the edit by pressing tab causes this issue.
Comment 27 Eclipse Genie CLA 2019-06-26 14:45:16 EDT
New Gerrit change created: https://git.eclipse.org/r/144953
Comment 28 Andrew Obuchowicz CLA 2019-06-26 15:25:33 EDT
(In reply to Eclipse Genie from comment #27)
> New Gerrit change created: https://git.eclipse.org/r/144953

I removed tab support from the table (for now), hopefully this will help.

If anyone continues to experience issues, please report them here.
Comment 29 Sarika Sinha CLA 2019-06-27 00:26:13 EDT
(In reply to Eclipse Genie from comment #27)
> New Gerrit change created: https://git.eclipse.org/r/144953

This does solve the issue. May be create another bug in Debug or SWT to investigate the tab issue.
Comment 31 Mickael Istria CLA 2019-06-27 02:49:15 EDT
Thanks Sarika for the tests and Andrew for the patch
@Andrew: can you please add a note about it in https://www.eclipse.org/eclipse/news/4.13/platform.php ? That happens by submitting patches to https://git.eclipse.org/c/www.eclipse.org/eclipse/news.git
Comment 32 Andrew Obuchowicz CLA 2019-06-27 09:59:55 EDT
(In reply to Mickael Istria from comment #31)
> Thanks Sarika for the tests and Andrew for the patch
> @Andrew: can you please add a note about it in
> https://www.eclipse.org/eclipse/news/4.13/platform.php ? That happens by
> submitting patches to
> https://git.eclipse.org/c/www.eclipse.org/eclipse/news.git

For sure, will work on that now.
Comment 33 Eclipse Genie CLA 2019-06-27 15:36:44 EDT
New Gerrit change created: https://git.eclipse.org/r/145064
Comment 34 Sarika Sinha CLA 2019-06-30 23:49:43 EDT
Please create the bug to investigate the tab issue.
Comment 35 Andrew Obuchowicz CLA 2019-07-01 17:58:22 EDT
(In reply to Sarika Sinha from comment #34)
> Please create the bug to investigate the tab issue.

Will do
Comment 36 Andrew Obuchowicz CLA 2019-07-02 10:24:21 EDT
(In reply to Sarika Sinha from comment #34)
> Please create the bug to investigate the tab issue.

See Bug 548869
Comment 38 Sarika Sinha CLA 2019-07-04 10:05:42 EDT
(In reply to Eclipse Genie from comment #37)
> Gerrit change https://git.eclipse.org/r/145040 was merged to [master].
> Commit:
> http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/
> ?id=7b911e9d8dc1f6fc7733cee59b92fd5ee6a7a8ae

Why is the N&N entry under Preferences ? It should be placed under Debug.
Comment 39 Andrew Obuchowicz CLA 2019-07-04 10:37:51 EDT
(In reply to Sarika Sinha from comment #38)
> (In reply to Eclipse Genie from comment #37)
> > Gerrit change https://git.eclipse.org/r/145040 was merged to [master].
> > Commit:
> > http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/
> > ?id=7b911e9d8dc1f6fc7733cee59b92fd5ee6a7a8ae
> 
> Why is the N&N entry under Preferences ? It should be placed under Debug.

I will move it to Debug when I have a moment, thank you for pointing this out.
Comment 40 Andrew Obuchowicz CLA 2019-07-04 11:32:30 EDT
Please refer to bug 548869 to look into the issues regarding the table becoming unusable after exiting an inline edit with tab.
Comment 41 Sarika Sinha CLA 2019-07-07 23:15:43 EDT
(In reply to Andrew Obuchowicz from comment #39)
> > Why is the N&N entry under Preferences ? It should be placed under Debug.
> 
> I will move it to Debug when I have a moment, thank you for pointing this
> out.

Just a reminder, Wednesday 10th is the last day to do it !
Comment 42 Andrew Obuchowicz CLA 2019-07-07 23:25:18 EDT
(In reply to Sarika Sinha from comment #41)
> (In reply to Andrew Obuchowicz from comment #39)
> > > Why is the N&N entry under Preferences ? It should be placed under Debug.
> > 
> > I will move it to Debug when I have a moment, thank you for pointing this
> > out.
> 
> Just a reminder, Wednesday 10th is the last day to do it !

Thanks, I'll get it done tomorrow (monday) !
Comment 43 Eclipse Genie CLA 2019-07-08 10:13:47 EDT
New Gerrit change created: https://git.eclipse.org/r/145620
Comment 45 Andrew Obuchowicz CLA 2019-07-08 15:22:33 EDT
I was able to patch the bug causing issues with tab key in the environment variables table. 

Commit: https://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=07064e46dde8282d6bc872a73f1936ed5643af51
Comment 46 Sarika Sinha CLA 2019-07-09 04:20:19 EDT
(In reply to Andrew Obuchowicz from comment #45)
> I was able to patch the bug causing issues with tab key in the environment
> variables table. 
> 
> Commit:
> https://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/
> ?id=07064e46dde8282d6bc872a73f1936ed5643af51

The tab from value in the last row does not work. If there is a row after that, it moves to the next row Variable's name which is correct. If it's the last row, the tab from "Value" should just select the whole row or move to the bttons but current behaviour looks odd.


For News entry,
There is no link to the Debug Index in the starting and it should be below •Views, Dialogs and Toolbar
Check out news/4.8/platform.html
Comment 47 Eclipse Genie CLA 2019-07-09 09:52:51 EDT
New Gerrit change created: https://git.eclipse.org/r/145692
Comment 49 Sarika Sinha CLA 2019-07-10 04:21:56 EDT
Eclipse SDK

Version: 2019-09 (4.13)
Build id: I20190709-1800
Comment 50 Lakshmi P Shanmugam CLA 2019-07-10 06:14:07 EDT
(In reply to Eclipse Genie from comment #48)
> Gerrit change https://git.eclipse.org/r/145692 was merged to [master].
> Commit:
> http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=5443229977e0fddcfbb45e22d5cd923a645c0edd
> 

Both the images in the N&N entry have a lot of empty space at the bottom, can you please trim them.
Comment 51 Eclipse Genie CLA 2019-07-10 09:25:51 EDT
New Gerrit change created: https://git.eclipse.org/r/145860
Comment 53 Paul Pazderski CLA 2019-12-04 17:29:24 EST
*** Bug 483084 has been marked as a duplicate of this bug. ***