Improve workspace lock error dialog.
Write workspace lock info like user, host, java process id, display properties onto .lock file if the lock was successful. Read the current lock data in case of lock was unsuccessful and shown it in error dialog. If the .lock file has no info then nothing is shown. For older eclipse versions.
see https://github.com/eclipse-platform/eclipse.platform.ui/issues/2343
@iloveeclipse FYI
Can you pls. provide a screenshot showing the new dialog?
By the way if on the lock dialog is worked, it would be great to have a cancel button!
Currently one can only Retry or Choose where on the second one can then finally cancel the startup of Eclipse. Instead I have most of the time not the case that another user locked the workspace but I have just an instance already running. In that case a early Cancel would be great.
Can you pls. provide a screenshot showing the new dialog?
Dialog looks like this with this fix.
Test Results
1 818 files + 3 1 818 suites +3 1h 33m 2s :stopwatch: + 8m 40s 7 709 tests + 45 7 481 :white_check_mark: +155 228 :zzz: ± 0 0 :x: - 1 24 288 runs +128 23 541 :white_check_mark: +441 747 :zzz: +22 0 :x: - 1
Results for commit 03c222e5. ± Comparison against base commit 1eead54a.
:recycle: This comment has been updated with latest results.
Can you pls. provide a screenshot showing the new dialog?
Dialog looks like this with this fix.
Maybe User, Host, etc. should be written in with an uppercase first letter (because they are some kind of label)
And these display strings must be localizeable.
Fixed all the review comments. Committed as separate commit to help the review. Later I will squash them as single commit.
Unfortunately it looks like writing to the locked file unlocks it again!
I'm able to start multiple Eclipse instances on same workspace now :-(
I have changed the approach now. First we will check if the workspace is locked, If not then write the data who is going to lock it.(Also backup .lock file data) Now initiate actual lock, If we are successful all good Otherwise write back what was there before onto .lock file.
Also please rebase on latest master before next push
Also please rebase on latest master before next push
I went through comments. I will refactor the code with .loc_info file approach. which will not have any risks. If file exists we show owner info otherwise old error dialog.
By the way if on the lock dialog is worked, it would be great to have a cancel button!
Currently one can only
RetryorChoosewhere on the second one can then finally cancel the startup of Eclipse. Instead I have most of the time not the case that another user locked the workspace but I have just an instance already running. In that case a early Cancel would be great.
I will do this change as separate PR. As we are only improving error message here. Other suggestions are fixed.
All the review suggestions are fixed.
All the review suggestions are fixed.
How does the UI look now?
All the review suggestions are fixed.
How does the UI look now?
Now it looks like this.
Now it looks like this.
The labels are still not aligned, it should be something like
User: abcd
Host: 234.567.7
Display: :1
Display: :1
Especially this looks strange because of the two ":". I knot that's they displays are called. Maybe we should put all the values into single or double quotes. So that is would look like
User: "abcd"
Host: "234.567.7"
Display: ":1"
Especially this looks strange because of the two ":". I knot that's they displays are called.
Display has format <hostname>:<sequence number>.<screen number> so the format can potentially be longer.
Especially this looks strange because of the two ":". I knot that's they displays are called.
Display has format
<hostname>:<sequence number>.<screen number>so the format can potentially be longer.
Sure. But this does not contradict my proposal...
Especially this looks strange because of the two ":". I knot that's they displays are called.
Display has format
<hostname>:<sequence number>.<screen number>so the format can potentially be longer.
I am using System.getenv("DISPLAY"); to read display number. I use 2 monitors it always returns value as shown above. like :1 for vnc display 1.
Display: :1
Especially this looks strange because of the two ":". I knot that's they displays are called. Maybe we should put all the values into single or double quotes. So that is would look like
User: "abcd" Host: "234.567.7" Display: ":1"
I have introduced tab spacing and single quotes for the lock owner details. Now the dialog looks like this.
I have introduced tab spacing and single quotes for the lock owner details. Now the dialog looks like this.
That looks nice now. Thanks
@BeckerWdf do you think the quotes are required with the new layout? They look a bit misplaced from my point of view.
@BeckerWdf do you think the quotes are required with the new layout? They look a bit misplaced from my point of view.
Yes, unless one expects there might be trailing or leading whitespace that needs to be called out because that relevant, then the quotes seem like merely noise. And I believe in this case there is no interesting whitespace.
Yes. In the new layout we can get rid of them from my point of view.
Fixed review comments. Now dialog looks like this.
This change seems to have introduced a regression documented in #2423
