Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 327842 - Key bindings broken in editor when showing status
Summary: Key bindings broken in editor when showing status
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.6.1   Edit
Hardware: PC Linux-GTK
: P2 major (vote)
Target Milestone: 3.7 M4   Edit
Assignee: Markus Keller CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 322312 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-10-14 20:18 EDT by Jeremy CLA
Modified: 2011-02-24 09:23 EST (History)
5 users (show)

See Also:


Attachments
Picture of editor with F5 hint (4.37 KB, image/png)
2010-10-26 13:04 EDT, Dani Megert CLA
no flags Details
Fix for the focus problem (4.85 KB, patch)
2010-10-28 14:07 EDT, Markus Keller CLA
no flags Details | Diff
Improved fix for focus problem (6.45 KB, patch)
2010-11-01 10:39 EDT, Markus Keller CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy CLA 2010-10-14 20:18:19 EDT
Build Identifier: 20100617-1415

First off, I know there is a feature called Refresh automatically under the General/Workspace preferences.  That's not what I'm talking about; that feature periodically refreshes your entire workspace.

What I would like is much simpler: frequently when I checkout a different branch in git, or otherwise modify files in some way outside of Eclipse, Eclipse considers the files to be out of sync.  This is true whether I have them open or not.  When I go to an out of sync file, I not only have to hit F5, I first have to click away from the main editor (as F5 won't work if the editor has the focus and it's on an out of sync file).  This isn't that terrible, but it involves going to the mouse when I normally wouldn't have to, and because I change my git checkouts a lot it happens frequently.

What would be really nice instead is if there was an option that, when I select an "out of sync" resource, makes Eclipse automatically refresh that file, rather than requiring me to click away and hit F5.

Describing this, it sounds so simple I'm baffled why it's not the default Eclipse behavior ...

Reproducible: Always

Steps to Reproduce:
1. open a file (or don't)
2. modify that file using something other than Eclipse (another editor, version control system, whatever)
3. select that file (if it's open; open it if you skipped #1)
4. what *should* happen is you see your file; what happens instead is you get an annoying message about being out of sync and you have to click away then hit F5 to fix it
Comment 1 Jeremy CLA 2010-10-14 20:28:38 EDT
Thanks for cleaning up my bug for me, but you got the product wrong; my issue has nothing to do with egit (or even with git really, that just happens to be the program I modify files with the most outside of Eclipse).
Comment 2 Remy Suen CLA 2010-10-14 20:46:59 EDT
What view are you using? This sounds awfully like bug 225889.
Comment 3 Paul Webster CLA 2010-10-14 21:40:51 EDT
(In reply to comment #1)
> Thanks for cleaning up my bug for me, but you got the product wrong; my issue
> has nothing to do with egit (or even with git really, that just happens to be
> the program I modify files with the most outside of Eclipse).

The Platform UI is for menu items, creating parts, etc.  That's not this.

Is the problem that you would like a preference on the editors where instead of asking you, it would refresh automatically?  I can send it to Platform/Text for text editors.  If the system won't inform listeners unless the active part changes, that would be Platform/Resources.

PW
Comment 4 Jeremy CLA 2010-10-15 14:32:53 EDT
Paul: sorry, I wasn't sure which product to use (I just knew it wasn't eGit); Platform/Text sounds good to me.  And yes, you have it correct: I "would like a preference on the editors where instead of asking you, it would refresh automatically?".

Remy: I looked at that bug, but I think this is a different issue.  That bug is about adding a refresh button vs. what I'm asking for is no button, just either a new preference or a default behavior to do a refresh automatically when ever you see "Resource is out of sync with the file system:"
Comment 5 Jeremy CLA 2010-10-15 20:21:35 EDT
Just realized my earlier description was misleading: you can't just "click away" *anywhere* to make the refresh work, you do have to click on the Package Explorer, and either on to the file you're trying to refresh or elsewhere in that pane that isn't on a specific file.  The point is, you have to do the refresh in the package explorer, because the refresh in the editor doesn't work (even when you hit F5) ... and really it should have happened automatically in the first place (IMHO).
Comment 6 Dani Megert CLA 2010-10-16 04:38:56 EDT
> I not only have to hit F5,
>I first have to click away from the main editor (as F5 won't work if the editor
>has the focus and it's on an out of sync file).
This works fine for me in the Text and Java editor. Which editor are you using?
Comment 7 Dani Megert CLA 2010-10-20 02:28:04 EDT
Ping.
Comment 8 Dani Megert CLA 2010-10-23 03:24:13 EDT
.
Comment 9 Jeremy CLA 2010-10-25 12:23:51 EDT
Odd, Bugzilla was emailing me comments before, and it emailed me when this bug's status changed, but I didn't get emails on your last comments (so sorry for not responding).

This is happening to me in the Java editor, the JSP (WTP? WPT? WST?  I can never keep the acronym straight since it doesn't have "JSP" in it ...) editor, or pretty much any editor I use (I'm pretty sure I've seen it happen with Javascript and Ruby files as well).

I just switched to "Helios Service Release 1" the other day and I'm still seeing the issue.  To be fair, I guess it *might* be something specific to Git (I can't easily reproduce it without using Git, but I know I had this problem before we started using Git, as we only started a few months ago).  

When I retraced my original instructions I didn't see the issue this time though, so here are some similar but slightly different instructions that definitely reproduce the issue under this most recent (released) Eclipse version:

1. Open some file (with modifications), then close it (this step might not be necessary, but I did it when testing so ...)
2. Git stash (which should stash that file since it has modifications)
3. Git stash pop
4. Try to open the file again; you get a "Resource out of sync with ..."
Comment 10 Dani Megert CLA 2010-10-26 02:37:07 EDT
Sorry but it works for me using Eclipse SDK and the text or Java editor. If it's not working in another editor or in combination with Git, then please report the problem there or provide steps based on
http://download.eclipse.org/eclipse/downloads/drops/R-3.6.1-201009090800/index.php or newer.
Comment 11 Dani Megert CLA 2010-10-26 02:37:55 EDT
Also note that JSP (WTP? WPT? WST? ) are different editors i.e. not directly related to the Java editor.
Comment 12 Jeremy CLA 2010-10-26 12:18:05 EDT
>>Also note that JSP (WTP? WPT? WST? ) are different editors i.e. not directly
>>related to the Java editor.

So, here's the thing: I never said this was a Java editor problem.  In fact, I believe I said the exact opposite, that it's a general Eclipse problem that seems to occur in every editor I have.  So, I'm really not sure where you got the idea this was related to the Java editor, but ... it's not.

As for reproducing the issue with the build you linked, that seems kind of ridiculous to me.  I downloaded the latest version of Eclipse off the website like a week ago.  Are you really telling me that the Eclipse team doesn't fix bugs in the current release version on your site?  That for any user to submit a bug they have to download the development edition of Eclipse?  I've never seen such a requirement in open source or commercial software in my entire life.
Comment 13 Paul Webster CLA 2010-10-26 12:28:08 EDT
(In reply to comment #12)
> As for reproducing the issue with the build you linked, that seems kind of
> ridiculous to me.  I downloaded the latest version of Eclipse off the website
> like a week ago.

Just FYI: That's not a development version, but the latest current release, 3.6.1.  It would have been at the top of http://download.eclipse.org/eclipse/downloads/  ... technically this is "Helios Service Release 1".

Dani is asking you to try that build as it is a clean, Eclipse SDK install.  It will help narrow down if this is an eclipse problem (or something else).  The implication was you need to open a bug against EGit if it is a git problem (i.e. you can't repo it in Eclipse SDK).

PW
Comment 14 Paul Webster CLA 2010-10-26 12:29:43 EDT
We'll just move this to EGit for comments, then.

PW
Comment 15 Jeremy CLA 2010-10-26 12:52:23 EDT
>>Just FYI: That's not a development version, but the latest current release,
>>3.6.1. 
>>  ... technically this is "Helios Service Release 1".
Ah, thank you for the clarification; that makes MUCH more sense.

>>Dani is asking you to try that build as it is a clean, Eclipse SDK install.  It
>>will help narrow down if this is an eclipse problem (or something else). 
Ok, that makes perfect sense ... but I did that already.  I downloaded a fresh copy of the Javascript version of Eclipse, installed the Ruby and Java editors (and nothing else), and I still have the problem.

>> We'll just move this to EGit for comments, then.
Noooooooo!  This has NOTHING to do with EGit (I don't even use EGit).  It has something to do with normal command-line git, but even that is just how I reproduce the issue; I don't think git has some magic that let's it mess with the Linux filesystem in a way no other program can ;-)
Comment 16 Dani Megert CLA 2010-10-26 13:04:20 EDT
> So, here's the thing: I never said this was a Java editor problem.  In fact, I
> believe I said the exact opposite, that it's a general Eclipse problem that
> seems to occur in every editor I have.  So, I'm really not sure where you got
> the idea this was related to the Java editor, but ... it's not.
Right, and what I try to tell you is that for the editors that I know (text, Java) it does work i.e. if you change a file outside Eclipse it
- asks you to refresh if the editor was already open and gets focus
- if you open such a file you get a hint in the editor (see attached picture) 
  and F5 works like a charm
Comment 17 Dani Megert CLA 2010-10-26 13:04:59 EDT
Created attachment 181748 [details]
Picture of editor with F5 hint
Comment 18 Jeremy CLA 2010-10-26 20:36:58 EDT
>>Right, and what I try to tell you is that for the editors that I know (text,
>>Java) it does work

Ok, so clearly there is some discrepancy between our testing environments.  I'm on Linux, and I'm using Git to make the "changes" to the file.  Those are the only two things I can think of that might be different and relevant between us.  Are you in Linux, and are you using "git stash" to "modify" the file?  If not, can anyone else on the team reproduce those conditions (git stashing on a Linux environment)?  That'd be ideal.

If you don't have anyone on the team who uses Git, I can try to find out what exactly git stash is doing (I assume it's just updating the modified date on the file, but maybe there's more?).  And if you don't have anyone on the team with Linux ... what's wrong with you people? ;-)
Comment 19 Jeremy CLA 2010-10-26 20:37:53 EDT
P.S. That last bit was a joke, just in case the smiley didn't make it clear (although I will be honestly surprised if none of the Eclipse devs uses Linux ...)
Comment 20 Remy Suen CLA 2010-10-27 12:59:54 EDT
Why don't we just take Git out of the equation here?

1. Start Eclipse on a new workspace.
2. Make a project.
3. Create a new file. Edit it. Save and close the editor.
4. Now open the same file in the command line with Vim/Emacs/whatever, make some changes, add newline characters or whatever, save and close the editor.
5. Try reopening the file in Eclipse now, you do not see an editor show up like the one Dani has in attachment 181748 [details]?
Comment 21 Jeremy CLA 2010-10-27 18:35:06 EDT
>> Why don't we just take Git out of the equation here?

I tried that before, and for whatever reason had difficulty reproducing the issue.  But, I just tried it now, and it "worked", in the sense that I got a screen like the one in attachment 181748 [details].  But my problem was never that Eclipse failed to notice that the file changed; my problem is that when Eclipse notices it, I get that annoying white screen telling me to hit F5, but when I actually hit F5 it doesn't do any good.  I wind up having to refresh in the Package Explorer, as that's the only way to refresh the file.

And really, showing the white page makes no sense in the first place.  I mean, gee, do I really want to stare at a white screen, or do I want to see my file?  I think if I was trying to open that file, I probably want to see it, so why is Eclipse even showing me an (evidently broken) white screen in the first place?  It does no one any good.

So I've really got two issues I guess:

1) (Bug) Hitting F5 doesn't work for me (and this is with a fresh download with only the official JS, Java, WTP, and Ruby plug-ins ... and the issue happens with any of the editors from any of those plug-ins).  But I could care less about getting this bug fixed if my second issue gets addressed instead ...

2) (Feature Request) Don't even show the pointless white screen in the first place (or at least give an option not to)!  No one cares if there file got "changed" by Git since they last opened it.  If they do care, there are certainly better ways to inform them of it (but I understand y'all don't have infinite time to create new informing mechanisms, so I'll I'm asking for is to just have an option for people like me to simply have Eclipse refresh the file rather than wasting my time telling me something I don't care about).
Comment 22 Remy Suen CLA 2010-10-27 18:58:41 EDT
(In reply to comment #21)
> And really, showing the white page makes no sense in the first place.  I mean,
> gee, do I really want to stare at a white screen, or do I want to see my file? 
> I think if I was trying to open that file, I probably want to see it, so why is
> Eclipse even showing me an (evidently broken) white screen in the first place?

Please see bug 248068 and bug 264196. There are probably other bugs on this topic.

> 1) (Bug) Hitting F5 doesn't work for me (and this is with a fresh download with
> only the official JS, Java, WTP, and Ruby plug-ins ... and the issue happens
> with any of the editors from any of those plug-ins).

F5 works for both myself and Dani. I suppose it is possible we have a bug on Linux here.

Which download did you get, by the way? Do you still have the file? What is its name? I didn't think any of the downloads included a Ruby plug-in.

There might also be a keybinding conflict, if you remap the 'Refresh' command to another keybinding, does it work?
Comment 23 Jeremy CLA 2010-10-27 19:33:11 EDT
Thanks for the response.  Bug #264196 is requesting a refresh button; that's lipstick on a pig in my ever so humble opinion ;-) (Adding another button to a page that shouldn't exist doesn't make the fact that that page exists any better).  Bug #248068 is much closer to what I'm asking for; the key difference I think is that the existing auto-refresh option really has nothing to do with what I'm asking for; that option simply makes Eclipse (really annoyingly) refresh your files constantly in the background.  The option I'm requesting would be entirely different; it wouldn't do any background refreshing, it would simply trigger the equivalent of an F5 keypress whenever the user opens an out of date file.

So *maybe* this can be merged in to 248068, but because that issue has to do with the existing auto-refresh option and I'm requesting a totally distinct and separate option, I *think* my issue is separate.  Y'all are the experts though, so if this bug should be merged in to that one I'll defer to your judgment.  (Of course, as I mentioned I've kind of got both a bug and a feature request in this issue, and 248068 only would address the feature request part, so I guess we'd just want to take the feature request part out of this bug and leave it to only be the bug part?  Although if 248068 is going to do what I hope, I couldn't give a fig about the bug part of this.)

On the download version, I downloaded the latest Javascript version off the main site, then downloaded Ruby/Java/WST (or whatever the acronym for the JSP one is) afterwards.  All of these are standard Eclipse plug-ins, not third party ones, so I doubt they're the cause, but I can certainly download a different version and not install any plug-ins at all to do some testing.  Which version would you like me to download?

Also, I'll be happy to test the keybinding remapping testing as well, but I don't really have time for any of that tonight.  Tomorrow (or *maybe* later this week if I get really busy) I'll be sure to download a fresh copy of whatever flavor of Eclipse is best, and I'll try re-binding F5 on it as well.
Comment 24 Remy Suen CLA 2010-10-27 21:01:18 EDT
There is also bug 228924, which was closed as WONTFIX.
Comment 25 Dani Megert CLA 2010-10-28 02:13:42 EDT
I agree that there are two problems: 2) is bug 248068, so lets focus on 1) in this bug here.

I got hold on a Linux box this morning and could reproduce the problem: when the editor opens with the F5 hint, all key bindings  (e.g. Ctrl+H to open the Search dialog) are not working, not just F5. I'll have to investigate whether this is an SWT or Text issue.

> There is also bug 228924, which was closed as WONTFIX.
This is basically covered by bug 248068.
Comment 26 Deepak Azad CLA 2010-10-28 05:49:08 EDT
This is broken in 3.6, works till 3.5. 

Problematic code: org.eclipse.ui.texteditor.StatusTextEditor.setFocus()
removing this method, fixes the problem.
Comment 27 Remy Suen CLA 2010-10-28 05:49:35 EDT
(In reply to comment #25)
> > There is also bug 228924, which was closed as WONTFIX.
> This is basically covered by bug 248068.

My point was that either they should both be closed as WONTFIX or the WONTFIX'd
one should be marked as a duplicate of bug 248068 if it has now been deemed
that the request is reasonable.
Comment 28 Jeremy CLA 2010-10-28 12:32:34 EDT
Thank you all for helping diagnose this!  I assume I don't need to download a fresh Eclipse anymore now that someone else has verified the issue?

As for the other related bugs (228924, 248068), the main problem I see is the existing auto-refresh option.  It *seems* to me that the logical thing for that option to do is what is being requested in those other two bugs.  However, what it actually does (refresh stuff periodically in the background) ... while I personally find it incredibly annoying and a waste of an option ... someone else out there might actually want that option for whatever reason.

If there is a desire to keep that existing option working as is (again, I don't see the point, but y'all would have a much better idea), then I really think there needs to be a new separate option OR the default behavior should simply change.  What I don't think should happen is for the two functions (simulated F5 click AND periodic background refreshing) to both be triggered by the existing auto-refresh option, as those two functions really don't have much to do with each other.
Comment 29 Markus Keller CLA 2010-10-28 14:07:36 EDT
Created attachment 181974 [details]
Fix for the focus problem

The focus problem on GTK and Cocoa occurred due to a few SWT problems and bugs in StatusTextEditor:

- StatusTextEditor#updatePartControl(IEditorInput) often disposes and re-creates the status control, but it doesn't take care of the focus. 

- The InfoForm doesn't contain any control that can take keyboard focus (this is an accessibility issue that is also fixed by this patch)

- The InfoForm contained an empty Composite, which screws setFocus() calls (bug 170631, workaround was to make it NO_FOCUS)

- When the focus was in the status control and we re-create the status control, then SWT behaves differently depending on platform:
  - on Windows, it forces the focus into the parent composite, that's why F5 and Ctrl+PageDown, etc. worked
  - on GTK, the focus goes to nowhere (doesn't even come back on Ctrl+Tab)
  - on Cocoa, the focus also goes to nowhere, but at least it comes back on Tab
Comment 30 Dani Megert CLA 2010-10-29 02:12:55 EDT
(In reply to comment #28)
> I assume I don't need to download a
> fresh Eclipse anymore now that someone else has verified the issue?
Right.

> it actually does (refresh stuff periodically in the background) ... 
The way the refresh happens depends on the OS.

> as those two functions really don't have much to
> do with each other.
They do. It's basically whether you want your Eclipse model tell you that something changed outside or whether you always want it up-to-date.
Comment 31 Jeremy CLA 2010-10-29 12:03:58 EDT
> > as those two functions really don't have much to
> > do with each other.
> They do. It's basically whether you want your Eclipse model tell you that
> something changed outside or whether you always want it up-to-date.

Sorry, what I meant wasn't whether their intent was related, I just meant what they actually do is unrelated.  In both cases (evidently) the intent is to keep my stuff always up to date, but:

Option A) Means my computer will randomly slow down frequently as it refreshes things

Option B) Means I won't have to hit a keystroke a few times a day (a keystroke that likes to stop working from time to time)

Me (or any other user) wanting to have B) is in no way related to me/anyone else wanting A); in fact, I very much don't want A) (on Linux at least, it's very obnoxious).  So that's what I meant by them being un-related: if I have to deal with constant background updates, checking the option won't be worthwhile to me, even if it does also add an auto-F5 functionality.

(But if their intent is the same, then B) seems like a MUCH better way to achieve that intent, and there really seems to be zero need for A).)
Comment 32 Markus Keller CLA 2010-10-29 12:45:13 EDT
Released the fix for the focus problem (broken keybindings) to HEAD.

(In reply to comment #31)
I see a dependency between the two options, but it's only one-way:
If workspace auto-refresh (option A) is enabled, then it makes sense to also enable editor auto-refresh (option B) for free (bug 248068).

I agree there should be a way to get rid of the out-of-sync dialog without having to enable the workspace auto-refresh option (i.e. B without A).

E.g. "Warn when activating editor on out-of-sync file" on the "Text Editors" preference page. When bug 248068 is fixed, a tooltip on the option could tell that we never warn when "General > Workspace > Refresh Automatically" is enabled. Note that we still show a dialog on Save when the editor was dirty at the time we detected the out-of-syncness.

Jeremy, could you please open a new enhancement request for Platform/Text for this separate editor-auto-refresh option? This bug is already too long and has been used to solve a different part of the original problem description.
Comment 33 Jeremy CLA 2010-10-29 16:12:38 EDT
>>Jeremy, could you please open a new enhancement request for Platform/Text for
>>this separate editor-auto-refresh option?

Absolutely ... well except it wouldn't let me do Platform/Text, so I did Platform/IDE instead (sorry).  The new bug is #329100.

Thanks!
Comment 34 Markus Keller CLA 2010-11-01 10:39:51 EDT
Created attachment 182142 [details]
Improved fix for focus problem

Blinking caret in message field was not nice -> setCaret(null).

Original fix always sets the focus to the status control. If a subclass 
creates its own status control and sets its focus manually, then the original fix would override that. New fix only sets the focus again if necessary.

AbstractDecoratedTextEditor now sets initial focus to its "Set Encoding..." button.
Comment 35 James Blackburn CLA 2011-02-24 09:23:03 EST
*** Bug 322312 has been marked as a duplicate of this bug. ***