Community
Participate
Working Groups
Currently, when deleting a task repoistory, the user is just presented with a dialog telling them that they have queries and tasks that are still associated and need to be deleted before the repository can be deleted. Maybe we should consider having the delete task repository action support deleting all of the queries and the tasks for a repository so that it is easier for a user to delete a task repository.
In terms of UI, I think we need the delete dialog for a repository to state the following. ------------------------ Delete the "Mumble Frotz" task repository? This will delete [2000] tasks and [20] queries from your Task List. ------------------------ Other: * Label needs to be changed to "Delete Repository..." * Don't support deleting multiple repositories since that complicates the UI, instead disable for more than 1 selection
Created attachment 124222 [details] Here's a patch to implement the proposed solution.
Created attachment 124223 [details] mylyn/context/zip
Created attachment 124225 [details] updated
Created attachment 124226 [details] updated patch
Created attachment 124227 [details] updated
Created attachment 124230 [details] fixed problem with repository deletions when no tasks or queries
Created attachment 124231 [details] mylyn/context/zip
Thanks for the patch. The last attachment seems to have only one file, can you attach it again? A few notes: Please simplify the expression in updateSelection(). The nested loops in run() seem a bit complicated. Try building a set of repositories first, then iterate over all queries and tasks once and add the elements that need to be deleted to a list. Executing the deletion in the progress monitor will not work well unless you call Display.readAndDispatch() to update the UI. IProgressService.busyCursorWhile() could work better in this case, even though it doesn't provide cancellation support. Generally monitor.beginTask() should always be followed by monitor.done(): try { monitor.beginTask(); ... } finally { monitor.done(); } You should never call PlatformUI.getWorkbench().getActiveWorkbenchWindow() without checking if the returned window is null. A progress dialog, error dialog or other shells might get displayed between the event that invoked your action and execution of your action. In this case you could have used TasksUiInternal.getShell() which does the checking.
(In reply to comment #9) > A few notes: Please simplify the expression in updateSelection(). Done (although the original expression was way more fun) > The nested > loops in run() seem a bit complicated. Try building a set of repositories first, > then iterate over all queries and tasks once and add the elements that need to > be deleted to a list. Done.* Note: This is how the function was originally written. Generally, should I refactor code when submitting patches? I had shied away from this, as I didn't want to make any major changes to existing code, which could obviously cause problems. What's the policy you follow in this type of situation? > Executing the deletion in the progress monitor will not > work well unless you call Display.readAndDispatch() to update the UI. > IProgressService.busyCursorWhile() could work better in this case, even though > it doesn't provide cancellation support. Generally monitor.beginTask() should > always be followed by monitor.done(): Done (used progressservice, do I need to also call Display.readAndDispatch in this case?). > You should never call PlatformUI.getWorkbench().getActiveWorkbenchWindow()... Done.
Created attachment 124300 [details] fixed according to comments
Created attachment 124301 [details] mylyn/context/zip
Created attachment 124305 [details] fixed a updateSelection problem
Created attachment 124306 [details] mylyn/context/zip
Created attachment 124367 [details] updated patch
I have wrapped the all modifications in an ITaskListRunnable to avoid concurrent changes of the task list while deletion is in progress. Please look into these two cases before I apply the patch: * If there a no queries or tasks the message should be different, i.e. "Are you sure you want to delete the repository?" * Unsubmitted tasks for the repository need to be deleted as well.
Created attachment 124499 [details] updated as per your comments
Created attachment 124500 [details] mylyn/context/zip
> * If there a no queries or tasks the message should be different, i.e. "Are you > sure you want to delete the repository?" Done. > * Unsubmitted tasks for the repository need to be deleted as well. Done.
Patch applied. I have refactored the implementation that executed the deletion of task list elements to also close editors, delete task data and contexts.
(This is an old comment, which I neglected to submit a while back. Ignore it if it's obsolete.) In terms of UI, I think we need the delete dialog for a repository to state the following. ------------------------ Delete the "Mumble Frotz" task repository? This will delete [2000] tasks and [20] queries from your Task List. ------------------------ Other: * Label needs to be changed to "Delete Repository..." * Don't support deleting multiple repositories since that complicates the UI, instead disable for more than 1 selection
