#63 Indent Warning and Progress Bar
If a user selects a significant amount of text and hits tab, they are presented with a Yes/No confirmation dialog warning them that the indent operation may take a long time, defaulting to No. This should be changed to a progress dialog with a cancel button or removed completely, IMHO.
The current behavior is difficult to predict. It is unclear to me what heuristic is used to determine how long an indent operation will take and how much time is considered "long". This heuristic may or may not be accurate or reasonable depending on the complexity of the text and the speed of the user's machine. In my experience, this message is sometimes presented even when the indent operation takes less than 10 seconds to complete. For comparison, it can take a similar amount of time to open a file and much more time to launch DrJava in the first place. With the threshold set this low, it could be very annoying for users with fast machines.
The current behavior is also modal and therefore wasteful. During the time it takes to display the dialog, read it, and respond, no useful computation is being performed. This problem is exacerbated by the default button selection of No, which requires the user to use the mouse to force DrJava to perform the requested action. (Using tab to select a different button does not work reliably.) This interruption time is often a significant fraction of the time it takes to perform the actual computation.
Incidentally, if the user selects Yes, no busy cursor is presented to indicate that the system is performing an action and will be unresponsive.
Using a progress bar will not solve the threshold issue, but it will make it less costly to the user when it is activated. The progress bar gives all of the benefit of a confirmation dialog, namely the ability to cancel a long-running operation, plus it provides the necessary information to make that decision intelligently. More importantly, it does not waste the user's time in the most common case that they want DrJava to perform as requested.
Logged In: YES
user_id=429731
This is actually unnecessary effort, since the dialog box is
entirely a temporary solution to the performance problem. We
don't plan on keeping it long term, so improving the performance
itself is higher priority than changing the notification.
(I recently determined what's causing it to be so slow, and
I'm going to address it over the summer.)
Incidentally, as the JUnit group is discovering, progress bars
are currently very hard to implement in DrJava, since the model
and view are running in the same thread, so the bar doesn't
update itself until all computation is finished. This might
require a transition to separate threads for model and view,
but that in itself is a non-trivial change (to make sure
everything is thread-safe).
Logged In: YES
user_id=429731
I agree that this would be an improvement-- we just have to be
careful about how we interrupt the worker thread. Would we undo
the work we've done so far, or leave it in an intermediate state?
I guess we would have a new thread start doing the work, while
an hourglass is shown over the single DefinitionsPane being
indented (other DefinitionsPanes would be editable). If we chose
to undo the changes on cancel (which is perhaps cleaner, though
less productive), we would just have to close the current
compound undo and trigger it, since a block indent is treated as a
compound undo.
To interrupt/cancel, perhaps the indenting thread would check a
field value after doing each line to see if it should proceed. This
would also be a good time for it to report on its progress to the
progress bar. (Well, maybe every few lines, anyway.)
Logged In: YES
user_id=431096
I have implemented almost precisely your suggestion, though I
didn't read it until afterward. Great minds, eh? I used a swing
ProgressMonitor, which has support for all of the things you
mention. Unfortunately, my copy of DrJava is now horribly
broken. When I start the indent, the entire frame becomes
unresponsive, and not even normal repaints get through. This
happens whether the indent is occurring in a SwingWorker or
not. In fact, it also happens in the latest beta. I assume that
this has been around for a while, and we never noticed.
Needless to say, the ProgressMonitor is never able to display
it's dialog.
Worse yet, it is now very easy for me to produce exceptions
with undo/redo and indenting large blocks of text. It is
possible that my additions to CompoundUndoManager are
violating somebody's invariants -- there's a lot of inexplicable,
uncommented code in there. For example, we keep a Vector
of CompoundEdits, but we explicitly use only the zero element.
I've gotta be missing something here. I could use some help
figuring this out. For now I'm going to work on something else.
...
Logged In: YES
user_id=429731
Yikes... Neal should be able to give you a hand with
CompoundUndoManager, since I think he's worked with it before.
We should try to document any invariants it's expecting so this
doesn't happen again.
You mention that the frame is unresponsive in the latest beta--
that's because we do the indent in the Swing thread itself, and we
always have. Yes, this is awful and needs to be fixed, but it at
least prevents people from modifying the document while the
indent is running (for the time being). Clearly, we should just
make the DefPane uneditable and do the work in another thread.
Actually, the new thread might be causing problems with the
ReducedModel-- remember that it isn't thread safe, so all accesses
must go through DefDoc. I wouldn't expect this, though, as long
as no other thread is modifying the document (or, more
specifically, the current location) during the indent.
Maybe an incremental approach would be better-- try to get the
new thread and hourglass working before addressing the progress
bar/cancel/undo stuff. I can pair with you later on if you'd like.
Logged In: YES
user_id=1075744
Originator: NO
Reclassified as feature request.
We were trying to use asynchronous tasks for indenting and "Find All", to at least make them abortable.