darcs patch: fix bug leading to early exit in XPrompt.

I think perhaps this bug only manifests itself when working with tabbed.
David
Mon Aug 27 14:58:58 EDT 2007 David Roundy

On Mon, Aug 27, 2007 at 03:01:12PM -0400, David Roundy wrote:
I think perhaps this bug only manifests itself when working with tabbed.
I started yesterday to chase that bug (I didn't notice before): the problem shows up only when tabs are visible and the completions list hides them. When the completion list get destroyed the generated Expose events will be incorrectly handled by XPrompt, that will exit the event loop and silently terminate any further action. (I'm explaining just to let others reproduce it). Yesterday I realized that this was going to be quite hard to find, so it's a big relief that you've already solved it! Thank you. Andrea

On Tue, Aug 28, 2007 at 08:31:01AM +0200, Andrea Rossato wrote:
On Mon, Aug 27, 2007 at 03:01:12PM -0400, David Roundy wrote:
I think perhaps this bug only manifests itself when working with tabbed.
I started yesterday to chase that bug (I didn't notice before): the problem shows up only when tabs are visible and the completions list hides them. When the completion list get destroyed the generated Expose events will be incorrectly handled by XPrompt, that will exit the event loop and silently terminate any further action. (I'm explaining just to let others reproduce it).
Ah, that explains why it only showed up on my laptop (which has a small screen, but is configured to use a moderately large font... plus through Combo, I usually have tabs visible near the bottom of the screen).
Yesterday I realized that this was going to be quite hard to find, so it's a big relief that you've already solved it! Thank you.
You're welcome! Basically, I just used the printf approach, combined with code review, in which I just searched for cases where eventLoop might have exited early. There's got to be a better idiom for writing an event loop. (so that exiting the loop is done explicitly, rather than implicitly... which would make it harder to exit the loop accidentally.) -- David Roundy http://www.darcs.net

On Tue, Aug 28, 2007 at 10:08:58AM -0400, David Roundy wrote:
You're welcome! Basically, I just used the printf approach, combined with code review, in which I just searched for cases where eventLoop might have exited early. There's got to be a better idiom for writing an event loop. (so that exiting the loop is done explicitly, rather than implicitly... which would make it harder to exit the loop accidentally.)
yes, indeed. Since I suspect you already have an idea on how the idiom should look like, may I ask you: - what would you suggest? - ;-) Andrea

On Tue, Aug 28, 2007 at 07:51:48PM +0200, Andrea Rossato wrote:
On Tue, Aug 28, 2007 at 10:08:58AM -0400, David Roundy wrote:
You're welcome! Basically, I just used the printf approach, combined with code review, in which I just searched for cases where eventLoop might have exited early. There's got to be a better idiom for writing an event loop. (so that exiting the loop is done explicitly, rather than implicitly... which would make it harder to exit the loop accidentally.)
yes, indeed. Since I suspect you already have an idea on how the idiom should look like, may I ask you: - what would you suggest? - ;-)
I'm not sure. My only idea would be to use exceptions to exit the loop. Then you could have something like (I'll write my example in IO, but you can extrapolate) eventLoop :: (EventType -> IO ()) -> IO () eventLoop job = forever (getNextEvent >>= job) `catchJust` breakException (\_ -> cleanup) forever a = a >> forever a exitLoop :: IO () where breakException should be a function that probably uses dynException to filter out only exceptions thrown by exitLoop. Another option would be to avoid exceptions and instead make the job return explicitly whether or not to continue eventLoop :: (EventType -> IO ContinueOrNot) -> IO () eventLoop job = do done <- getNextEvent >>= job case done of ExitNow -> cleanup KeepGoing -> eventLoop job data ContinueOrNot = ExitNow | KeepGoing This makes the code a bit cleaner, and also has the advantage of working in monads that don't support exceptions. It means that every branch of the job has to explicitly decide whether or not to keep going, but at least the compiler will enforce that you need to have a clear decision on each branch. But I'm not sure either of these is optimal. Exceptions are ugly. You could also (given you're in a state monad) throw a "stop now" flag into that state, which is perhaps better than either of these. Then you'd only need to specify exitLoop (or whatever you choose to call it)... -- David Roundy http://www.darcs.net

On Tue, Aug 28, 2007 at 02:14:15PM -0400, David Roundy wrote:
Another option would be to avoid exceptions and instead make the job return explicitly whether or not to continue
eventLoop :: (EventType -> IO ContinueOrNot) -> IO () [...] data ContinueOrNot = ExitNow | KeepGoing
[...]
But I'm not sure either of these is optimal. Exceptions are ugly. You could also (given you're in a state monad) throw a "stop now" flag into that state, which is perhaps better than either of these. Then you'd only need to specify exitLoop (or whatever you choose to call it)...
I think exceptions are not the right solution. Instead I like the other two and I'll study them. Thanks Andrea

On Monday 27 August 2007 14:01:12 David Roundy wrote:
I think perhaps this bug only manifests itself when working with tabbed.
David
Mon Aug 27 14:58:58 EDT 2007 David Roundy
* fix bug leading to early exit in XPrompt.
Applied, thanks.
participants (3)
-
Andrea Rossato
-
David Roundy
-
Spencer Janssen