First off, we are talking about a method here that is already 350
lines
long. This surely is wrong and needs to be changed. The patch only
slightly shortens the method, but makes a few things clearer, which
surly is a good thing.
Christopher also took create care to release the captured mouse on all
possible paths. This is very important, as otherwise the application
could stop responding. But now consider that somebody else works on
this
method in the future. The person might easily not know about that
necessity and accidentally break stuff. Even in this patch the mouse
sometimes gets release after a tracking in a submenu. I don't have a
clue how this is supposed to work. We always have to release the mouse
before handling over control to a different window. This of course is
easy to fix for now. But who will remember this for future changes?
As much as I like the idea of this patch, I think it is too fragile
code
to be added. We should think hard on how to restructure the code
here to
make it less fragile and then we might be able to safely add a mouse
capture to it.