Community
Participate
Working Groups
Created attachment 189189 [details] fix 1 Text search pops up a modal error dialog for out-of-sync resources. Changes in bug 303517 will mean the workspace responds more proactively to such resources. This attached patch lets Text Search handle out-of-sync resources gracefully. (Note other changes such as: deleted on filesystem, gender change is still reported as before.) Existing Text Search tests pass.
> Changes in bug 303517 will mean the workspace responds more proactively to > such resources. Good, this means we will get that dialog less frequently. > This attached patch lets Text Search handle out-of-sync resources gracefully. Sorry, but that's not what we want. *** This bug has been marked as a duplicate of bug 268797 ***
(In reply to comment #1) > > Changes in bug 303517 will mean the workspace responds more proactively to > > such resources. > Good, this means we will get that dialog less frequently. Text search which iterates over all resources will string bring this up. > > > This attached patch lets Text Search handle out-of-sync resources gracefully. > Sorry, but that's not what we want. Why not? This patch makes a real qualitative improvement to user experience for the vast majority of cases where files are modified externally. This isn't a duplicate of bug 268797 which seems to call for refresh during search, which isn't what this is about (i.e. newly created files / folders won't be picked up). I think this patch on its own makes a big difference to the complaints we get from users.
As said: out-of-sync files are reported. If your other fixes reduces that scenario then that's fine. As outlined in bug 268797, we could use your fix but in combination with checking the refresh preference. Please don't reopen again. *** This bug has been marked as a duplicate of bug 268797 ***
OK. I'll reopen. This is a common problem we face in the CDT space where users often have tools that run outside of Eclipse and update files causing them to be out of sync. Could you explain why you don't consider this usability issue important. Thanks. Doug.
(In reply to comment #4) > OK. I'll reopen. This is a common problem we face in the CDT space where users > often have tools that run outside of Eclipse and update files causing them to > be out of sync. > > Could you explain why you don't consider this usability issue important. I did not say that. What I said is that Eclipse does not fore refresh out of the box, telling the user when something changed outside so that he is aware of it. Users that don't like that can enable General > Workspace: [ ] Refresh automatically The problem in search is that it does not honor that preference. This is covered by bug Bug 268797. *** This bug has been marked as a duplicate of bug 268797 ***
Dani I think the fix proposed here is different to the refresh solution proposed in bug 268797. This fix simply allows text search to read out-of-sync files. An actual refresh would be more expensive. Other non-sync related errors will still be reported. AFAICS this change doesn't affect the quality of the text search, and the user gets what they want! > Users that don't like that can enable > General > Workspace: [ ] Refresh automatically > > The problem in search is that it does not honor that preference. This is > covered by bug Bug 268797. Which enables a refresh provider. On any non-Windows platform this just polls, is expensive, and isn't timely. From my POV this isn't bug 268797. I don't want Search to perform the refresh. I want search to reflect the contents of the files being searched. Dani can you leave this bug open until there's some consensus?
(In reply to comment #6) > Dani I think the fix proposed here is different to the refresh solution > proposed in bug 268797. This fix simply allows text search to read out-of-sync > files. An actual refresh would be more expensive. > > Other non-sync related errors will still be reported. AFAICS this change > doesn't affect the quality of the text search, and the user gets what they > want! > > > Users that don't like that can enable > > General > Workspace: [ ] Refresh automatically > > > > The problem in search is that it does not honor that preference. This is > > covered by bug Bug 268797. > > Which enables a refresh provider. On any non-Windows platform this just polls, > is expensive, and isn't timely. Yes, I got that now. Your patch in core resources does a fallback when out-of-sync his hit. This is a new mechanism which wasn't present in Eclipse and which should not replace the well known use case where the user gets informed about out-of-sync for any operation he does. I'm in favor of your new option but against replacing the existing out of the box behavior. In that sense this bug here is a duplicate of bug 268797 but with the addition that it should also auto-refresh when your new option is enabled. Can we agree on that?
(In reply to comment #7) > In that sense this bug here is a duplicate of bug 268797 but with the addition > that it should also auto-refresh when your new option is enabled. Can we agree > on that? The problem is that doesn't match what I've proposed in fix1. If you look at the patch (which is trivial), it simply replaces #getContents() with #getContents(true). There's no refresh or anything else happening in search. Refresh might discovery additional resources, which this implementation doesn't address. If it's decided that the platform needs another preferences, then this could very well be #getContents(REFRESH_ENABLED ? true : false). But this is still distinct from having the search perform a refresh.
> The problem is that doesn't match what I've proposed in fix1. If you look at > the patch (which is trivial), Of course I did look at the patch. What you don't seem to understand is that it is the policy of the SDK (and the message we send out to others) that out of the box every out-of-sync is reported and not overridden by forcing the refresh, but that's what your patch does. Please also see my longer comment in 303517.
> I'm in favor of your new option but against replacing the existing out of the > box behavior. Actually, I have to revert that, see my explanation in bug 303517.
(In reply to comment #9) > > The problem is that doesn't match what I've proposed in fix1. If you look at > > the patch (which is trivial), > Of course I did look at the patch. What you don't seem to understand is that it > is the policy of the SDK (and the message we send out to others) that out of > the box every out-of-sync is reported and not overridden by forcing the > refresh, but that's what your patch does. > Please also see my longer comment in 303517. But the patch doesn't do any refresh at all. It merely grabs the contents of the file, without touhing the resource tree. Prior to the patch, it would see that the file is out of sync and give up. Users expect that a search is going to read the contents of the file, so I am really at a loss to see why this would be a problem. I cannot imagine a user ever NOT wanting Eclipse to just read the file like they expect it to.
> But the patch doesn't do any refresh at all. It merely grabs the contents of > the file, without touhing the resource tree. Prior to the patch, it would see > that the file is out of sync and give up. Which means it ignores the out-of-sync exception that's normally used everywhere in the SDK. > Users expect that a search is going to read the contents of the file, so I am > really at a loss to see why this would be a problem. I cannot imagine a user > ever NOT wanting Eclipse to just read the file like they expect it to. OK, assume this: 1. you open file X in the editor - you see "foo" in there 2. you search for "bar" and search reports you file X without any message BAD!
(In reply to comment #12) > OK, assume this: > 1. you open file X in the editor - you see "foo" in there > 2. you search for "bar" and search reports you file X without any message > BAD! But you would get notified as far as I see it. Here are the scenarios I see, if I have any of these wrong please correct me. File X is secretly out of sync. Scenario 1: File X is dirty in the editor. Search looks through the working copy rather than the contents on disk. No issue, you get what you expect... the search matches what you see in the editor. Scenario 2: File X is open in the editor, but isn't dirty. When you click on the match for bar in file X, it tries to open that location in the editor. On focus change to the editor, you are notified that the editor is out of sync, and prompted as to what to do. I don't see any problem with this. Scenario 3: File X isn't open in the editor. When you click on the match, the editor notifies you that it's out of sync, and prompts you as to what to do. Again, I don't see a problem here.
(In reply to comment #13) > (In reply to comment #12) > > OK, assume this: > > 1. you open file X in the editor - you see "foo" in there > > 2. you search for "bar" and search reports you file X without any message > > BAD! > > But you would get notified as far as I see it. How?
(In reply to comment #14) > > But you would get notified as far as I see it. > How? Exactly as Chris has outlined in comment 13, no?
(In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #12) > > > OK, assume this: > > > 1. you open file X in the editor - you see "foo" in there > > > 2. you search for "bar" and search reports you file X without any message > > > BAD! > > > > But you would get notified as far as I see it. > How? Seriously? That whole comment was my explanation of how the user is being notified of the out of sync whenever they attempt to navigate to the match.
(In reply to comment #16) > (In reply to comment #14) > > (In reply to comment #13) > > > (In reply to comment #12) > > > > OK, assume this: > > > > 1. you open file X in the editor - you see "foo" in there > > > > 2. you search for "bar" and search reports you file X without any message > > > > BAD! > > > > > > But you would get notified as far as I see it. > > How? > > Seriously? That whole comment was my explanation of how the user is being > notified of the out of sync whenever they attempt to navigate to the match. Yes, seriously. I gave the scenario in comment 12 so don't throw in others. The match will be reported in the Search view and what you see in the Search view will be inconsistent to what you see in the editor. So: how is the editor updated in that scenario if no user action follows (and of course were talking about HEAD and not assume that the patch from the other bug has been released)?
(In reply to comment #17) > Yes, seriously. I gave the scenario in comment 12 so don't throw in others. The > match will be reported in the Search view and what you see in the Search view > will be inconsistent to what you see in the editor. This doesn't seem true. I just tried this patch without any other changes, and text search seems to search the editor buffers in preference to the IFile. ergo the Search View remains in sync with the active editor.
(In reply to comment #18) > (In reply to comment #17) > > Yes, seriously. I gave the scenario in comment 12 so don't throw in others. The > > match will be reported in the Search view and what you see in the Search view > > will be inconsistent to what you see in the editor. > > This doesn't seem true. > > I just tried this patch without any other changes, and text search seems to > search the editor buffers in preference to the IFile. ergo the Search View > remains in sync with the active editor. Yes, that's indeed correct but isn't it then against your goal to get the latest state of the file? Also, if the editor would not be open, then it would report a match but opening the editor would give you out of sync. Anyway - the important thing is to watch the outcome of the other bug. Releasing this patch without an additional check is simply a no go as this is against how the SDK behaves when seeing an out-of-sync file.
Bug 303517 has reached consensus and will be in 3.7. There is no need to special case Text search. *** This bug has been marked as a duplicate of bug 303517 ***
