From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14564 invoked by alias); 7 Dec 2014 17:07:19 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 33905 Received: (qmail 28807 invoked from network); 7 Dec 2014 17:07:16 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-Originating-IP: [86.6.25.230] X-Spam: 0 X-Authority: v=2.1 cv=S8BXwecP c=1 sm=1 tr=0 a=c0CwWhpM9oUd/BnC3z6Gzg==:117 a=c0CwWhpM9oUd/BnC3z6Gzg==:17 a=NLZqzBF-AAAA:8 a=kj9zAlcOel0A:10 a=GhaCyahCPlcOX01uYYMA:9 a=CjuIK1q_8ugA:10 Date: Sun, 7 Dec 2014 17:07:13 +0000 From: Peter Stephenson To: "Zsh Hackers' List" Subject: Re: Interrupting globs (Re: Something rotten in tar completion) Message-ID: <20141207170713.1a71fe0d@pws-pc.ntlworld.com> In-Reply-To: <141206211828.ZM15934@torch.brasslantern.com> References: <20141202155452.647182b4@pwslap01u.europe.root.pri> <141202084858.ZM31517@torch.brasslantern.com> <20141202172654.30e7d380@pwslap01u.europe.root.pri> <141204085606.ZM9146@torch.brasslantern.com> <20141204171226.301e9d2c@pwslap01u.europe.root.pri> <141205002023.ZM19736@torch.brasslantern.com> <20141205145054.655a2f70@pwslap01u.europe.root.pri> <141205100632.ZM508@torch.brasslantern.com> <20141205181330.2b458b46@pwslap01u.europe.root.pri> <20141205203417.2bc66b7b@pws-pc.ntlworld.com> <20141205220717.2f86bdd2@pws-pc.ntlworld.com> <141206211828.ZM15934@torch.brasslantern.com> X-Mailer: Claws Mail 3.8.0 (GTK+ 2.24.7; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit I'm going to try and summarise things so far. Code: I've just pushed a branch "interrupt_abort" to the server which is top of tree with (so far) just my initial patch, which we know to be far from the end of the story. (It's *not* on the master branch everyone is currently using.) I'd suggest anyone with a weak stomach or limited grasp of what the issue actually is (I mean, even more limited than Bart, Mikael and me :-/) keep off this for now. I won't bother with a ChangeLog for this branch, just a git log; if we think something useful emerges, which isn't 100% obvious, I'll squash it and rebase it on to master when the time comes. (I'm now official head git for firmware at work so I've been learning heavily about this stuff.) Mikael and Bart and the usual suspects should certainly feel free to commit here if they want to play. Other issues: Mikael needs to ^C three times to get back to the command line --- apparently related to the code from Sven I took at its word. I'm confused here, too; I put back removing non-interrupt errors (I'll just refer to errors and interrupts from now on though I realise that's a bit glib) and it seems to be resetting interrupts somehow. For now the fix might be simply put back in the previous state with Sven's statement that it once did something, plus an additional comment that we're confused, and come back to this when we understand the general picture later. Where to reset interrupt state in the editor, in particular to enable aborts to menu completion: getbyte() or somewhere more specific? It's possible the getbyte() case suggested by Mikael is actually a reasonable place to do this. The argument would be that this is where the user next gets the chance to do some editing again. However, maybe that screws up some form of recursive reading of stuff? Certainly Bart's remark, that there are sub-cases we need to add, was also my first inclination and perhaps we ought to try that first as it's a bit more controlled. INT vs. QUIT --- no real feel for this, and I'm not sure how important it is in practice, but I think the change I made, where a TRAPQUIT that returns non-zero sets ERRFLAG_INT, should at least improve things if we're going to use TRAPQUIT at all, since that wouldn't have flagged an error before --- there was no special SIGQUIT handling, so telling the signal handler to behave as it normally would doesn't help; normally it wouldn't have handled SIGQUIT at all. (I suppose if we wanted to be really consistent we'd have exited the shell after handling a TRAPQUIT with a non-zero return value? That's obviously a separate issue.) More general points: The current code, both in completion functions an internally, don't give us much control over cleaning up on a keyboard interrupt (and didn't before --- Bart): yes, ick. I don't think shells do this very well generally. I agree we might want to look at some always blocks. BTW, I didn't actually think through the handling of "always" blocks for this patch, so it might want looking at. It's also just occurred to me I may have introduced some rare but entirely possible read-modify-write races because we set the ERRFLAG_INT bit in interrupts and set the other and clear both bits separately in the main shell. I guess it would be better to queue interrupts whenever we add or remove a single bit of errflag; that's probably not often enough to cause efficiency issues since should only be round significant chunks of shell code, or when an error has actually occurred. Opinions? > Are there any cases where errflag is unconditionally restored, or did you > change all such save/restore pairs? None of the cases of restoration unconditionally restore completely any more, they all keep the interrupt bit if it got set in the meanwhileX. I think we'd have to identify a place where it's sufficiently obvious to a user hitting ^C that they're going back to that point, i.e. it's not just an implementation detail in shell code as far as the person at the keyboard is concerned. I didn't spot one that looked plausible --- the cases where we want to strip the interrupt bit don't seem to be the same ones where we restore the previous error status. But I could easily have missed one. > Either the ternary is irrelevant here, or the "if (errflag)" is: Yes, the code inside the test should be "lastval = 1". I'll commit that. I'll post other things I think want to go on the interrupt_abort branch before I commit those. pws