Bugzilla – Full Text Bug Listing
| Summary: | [IDE] Allow to apply file attribute changes recursively | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | carolm <cb_maz> | ||||||||
| Component: | IDE | Assignee: | Oleg Besedin <ob1.eclipse> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | daniel_megert, jwsnyder, ob1.eclipse, prakash, pwebster, sptaszkiewicz, Szymon.Brandys | ||||||||
| Version: | 3.6.1 | ||||||||||
| Target Milestone: | 3.8 M2 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
carolm
Eclipse is consistent with the Windows command prompt behaviour here. It looks like the Windows Explorer UI that is offering to apply the read only attribute recursively. Even when I do that in Windows, if I create more files in the directory afterwards they are *not* read only. The issue is not what happens with the read status of new members added after the read only option is set , but to existing members when the folder option is set. To re-state: Under RDz, there is a folder with several members and neither the folder nor any members are read only. If the folder is set to read only, none of the existing members are set to read only. This is not like Windows which gives 2 alternate options: o apply changes to this folder only o apply changes to this folder, subfolders and files. You are correct that in both the RDz and Windows cases, any members added to a read only folder start out as not read only. (In reply to comment #1) > Eclipse is consistent with the Windows command prompt behaviour here. It looks > like the Windows Explorer UI that is offering to apply the read only attribute > recursively. Even when I do that in Windows, if I create more files in the > directory afterwards they are *not* read only. Shouldn't we be consistent with Windows Explorer UI as well? We could add similar dialog in ResourceInfoPage. John, what do you think? I would leave the core.resources part consistent with Windows command prompt and address setting the read-only flag recursively on the UI level. Moving back to IDE. Changing the summary to reflect the problem. The UI relies on resource to perform the actual change; I don't see a facility in there to do recursive change. We use ResourceAttributes#setReadOnly() for this particular change. The UI class in question is ResourceInfoPage. AFAIK, the recursive part could be done on the UI level by invoking ResourceAttributes#setReadOnly() for each member of the folder. I think there is no need to add any additional functionality in Resources. Moving back to IDE. (In reply to comment #8) > AFAIK, the recursive part could be done on the UI level by invoking > ResourceAttributes#setReadOnly() for each member of the folder. I think there is > no need to add any additional functionality in Resources. More detailed you would need to use IResource#accept and the visitor would set the flag recursively. (In reply to comment #8) > AFAIK, the recursive part could be done on the UI level by invoking > ResourceAttributes#setReadOnly() for each member of the folder. I think there > is no need to add any additional functionality in Resources. How about links? While there is no question that it could be done, UI does not seem like the right place to deal with OS-specific issues which we will get here. (In reply to comment #12) > How about links? While there is no question that it could be done, UI does not > seem like the right place to deal with OS-specific issues which we will get > here. Recursive apply will be a new feature so we need to define behaviour for links. I checked how regular attribute change works for links and it changes the attributes of the resource that the link points to. I think, initially, we can treat links as regular resources. Created attachment 198953 [details]
Initial patch
Here is a patch with initial approach. I think there is no additional OS-specific issue other than already present there. Processing link change had to be moved below processing attribute change so that when the user cancels the new dialog, link is not updated yet.
(In reply to comment #14) > I think there is no additional > OS-specific issue other than already present there. I was thinking more in terms of having a link / shortcut in the sub-directory that point to a parent directory. Also need to think what happens if the running process don't have the right to change some of the files / or if user presses on "Cancel" on this potentially long-running operation. Ideally we'd want to rollback a partial change or, at least, pre-verify that change seems to be possible before proceeding. (In reply to comment #15) > I was thinking more in terms of having a link / shortcut in the sub-directory > that point to a parent directory. That's true. I think we could just collect paths that were already visited and apply changes to the unvisited ones. > Also need to think what happens if the running process don't have the right to > change some of the files / or if user presses on "Cancel" on this potentially > long-running operation. Ideally we'd want to rollback a partial change or, at > least, pre-verify that change seems to be possible before proceeding. At the moment we do not verify the rights so I am not sure we need to add this here. "Cancel" during the long-running operation sound good. AFAICS, Windows UI does not allow to rollback partial change. Besides, it could be confusing for the user - doing the rollback requires roughly the same amount of time as the main operation. If the user canceled the operation at 80% because got impatient, then he would need to wait the same amount of time to roll back the change. Personally, when I cancel an operation I want it to stop immediately instead of waiting again the same amount of time. Created attachment 199747 [details]
Patch v2
Improved patch. Long running operation is run as a new job.
Oleg, what do you think about Patch v2? Any update on this fix? (In reply to comment #18) > Oleg, what do you think about Patch v2? It looks better, but there are some issues: 1) The patch works on Windows, but does not work on Mac. (I haven't tried Linux.) Unlike Windows, on Mac we have Owner/Group/Other permissions; the "executableBox" is null and "permissionBoxes" is not null. Having (executableBox == null) cancels new processing. 2) The handling of "visited" - right away I haven't being able to trick my computer into creating a link causing endless cycle, so this is purely a code review comment, not something that I was able to test (those OSes are getting smart!): - "visited.add(uri);" is only done when "childAttrs != null". Would it make sense to add resource to the visited list regardless of that condition? - IResourceProxyVisitor#visit() always returns "true". Should it return "false" for resources that have been already visited? (This is in #scheduleResourceAttributesChangeJob() and #getTotalWork().) 3) Message "Do you want to apply this change also to subfolders and files?" -> how about: "Do you want to apply this change to subfolders and files?". (In reply to comment #20) > 1) The patch works on Windows, but does not work on Mac. (I haven't tried > Linux.) > > Unlike Windows, on Mac we have Owner/Group/Other permissions; the > "executableBox" is null and "permissionBoxes" is not null. > > Having (executableBox == null) cancels new processing. Do you mean we should allow for recursive permission changes as well? > 2) The handling of "visited" - right away I haven't being able to trick my > computer into creating a link causing endless cycle, so this is purely a code > review comment, not something that I was able to test (those OSes are getting > smart!): > - "visited.add(uri);" is only done when "childAttrs != null". Would it make > sense to add resource to the visited list regardless of that condition? Agreed, I will change that. > - IResourceProxyVisitor#visit() always returns "true". Should it return "false" > for resources that have been already visited? (This is in > #scheduleResourceAttributesChangeJob() and #getTotalWork().) Agreed, I will change that. > 3) Message "Do you want to apply this change also to subfolders and files?" -> > how about: "Do you want to apply this change to subfolders and files?". Ok. (In reply to comment #21) > (In reply to comment #20) > > 1) The patch works on Windows, but does not work on Mac. (I haven't tried > > Linux.) ... > Do you mean we should allow for recursive permission changes as well? Well, on Mac the patch has no effect, and I suspect the same for Linux. So it is either Windows-only enhancement, or it needs to be changed to also work with 3x3 permission. (In reply to comment #22) > Well, on Mac the patch has no effect, and I suspect the same for Linux. So it > is either Windows-only enhancement, or it needs to be changed to also work with > 3x3 permission. This is true, the original request is only about Windows specific attributes like read-only and archive. I think I did too much by adding "executable" since I don't see the executableBox in Windows. However, we could add also support for 3x3 permissions for completeness if it is also a valid enhancement. Created attachment 201807 [details]
Patch v3
Added support for all attributes and permissions. Tested on Windows and Linux. Unfortunately, I don't have any Mac in here. Oleg, could you check if it works also on Mac?
(In reply to comment #25) > Created attachment 201807 [details] > Patch v3 > > Added support for all attributes and permissions. Tested on Windows and Linux. > Unfortunately, I don't have any Mac in here. Oleg, could you check if it works > also on Mac? Tested on Mac and Windows, works fine. Patch released into 3.8 and 4.2. Thank you! (In reply to comment #26) > Patch released into 3.8 and 4.2. Oleg, did you release the patch to git repo? (In reply to comment #27) > Oleg, did you release the patch to git repo? Yes: 3.8: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R3_development&id=5594fb406ce14e7af0ff0948c540578b635398da 4.2: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_development&id=7e4fb75a060e47c7f39a8dfdf0afc002b9fc0835 Verified in I20110913-0200. |