From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Jared Finder via "Emacs development discussions." Newsgroups: gmane.emacs.devel Subject: Re: mouse-face and help echo support for xterm mouse Date: Wed, 04 Nov 2020 09:54:11 -0800 Message-ID: <5928a03c79bcf0afee270de1df4d9fa5@finder.org> References: <31ff05295c806c4596c54fdcc8994a5f@finder.org> <3e7e0f6d1e272d03913e97254b2eabff@finder.org> <83imalazw7.fsf@gnu.org> Reply-To: Jared Finder Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="8844"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Roundcube Webmail/1.3.15 Cc: Eli Zaretskii , emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Nov 04 18:55:15 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kaN06-0002AN-Gr for ged-emacs-devel@m.gmane-mx.org; Wed, 04 Nov 2020 18:55:14 +0100 Original-Received: from localhost ([::1]:56476 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kaN05-00047X-J5 for ged-emacs-devel@m.gmane-mx.org; Wed, 04 Nov 2020 12:55:13 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:39388) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kaMzB-0003Wp-8M for emacs-devel@gnu.org; Wed, 04 Nov 2020 12:54:17 -0500 Original-Received: from greenhill.hpalace.com ([2600:3c01::f03c:91ff:fe73:2daa]:56466) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kaMz9-0003II-AQ; Wed, 04 Nov 2020 12:54:16 -0500 Original-Received: from mail.finder.org (greenhill.hpalace.com [IPv6:2600:3c01::f03c:91ff:fe73:2daa]) by greenhill.hpalace.com (Postfix) with ESMTPSA id 6EEF78E4; Wed, 4 Nov 2020 17:54:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=finder.org; s=2018; t=1604512451; bh=b5wXO/KPukHde/n5tyA/chv/Jt1xpgPmUg7fWthyeLQ=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=XE0PZztk7SMHmwmiGYdHbJmb6MQ5toPtGU2/FJGsvifyGVcucgQNjBRi4biqOcfz9 dr69qsxFtItV60oR/HMUe2aRhhY51gkDpe3wvZnzv0hq5RzjuwcggssQTsIRZyPVse 7QcDZtkZeDU89AqWXNDncJNffWpAZoR2jcZcCP8nWw2FIEC4uOxCetoCAx+2JgG1TJ lwy/SGPKLEsh4own1bZE6eRUbRMG+t5ehGAhMWTtOmUI9xBL9gearBrbHsDWMjcAOl KKSs6vRm1fTkyNfuYGrvavx3RIep0ZPIWhqa9vjH8irNsRGr4x+HuWqV0SHIVohfyZ xDU9aY4sfOIxQ== In-Reply-To: X-Sender: jared@finder.org Received-SPF: pass client-ip=2600:3c01::f03c:91ff:fe73:2daa; envelope-from=jared@finder.org; helo=greenhill.hpalace.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: 12 X-Spam_score: 1.2 X-Spam_bar: + X-Spam_report: (1.2 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_SBL_CSS=3.335, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:258689 Archived-At: On 2020-11-04 7:56 am, Stefan Monnier wrote: > > diff --git a/src/term.c b/src/term.c > index ff1aabfed2..a03a246415 100644 > --- a/src/term.c > +++ b/src/term.c > @@ -2562,7 +2562,8 @@ > ie.kind = NO_EVENT; > ie.arg = Qnil; > > - if (event->type & (GPM_MOVE | GPM_DRAG)) { > + if (event->type & (GPM_MOVE | GPM_DRAG)) > + { Old code style is OLD. Apparently this formatting is from when the function was first added in 2007. :) > @@ -2577,29 +2578,45 @@ > || !NILP (previous_help_echo_string)) > do_help = 1; > > - goto done; > + eassert (ie.kind == NO_EVENT); This is the assert for the mouse movement path. Please remember this value. > } > - else { > + else > + { > f->mouse_moved = 0; > + eassert (ie.kind == NO_EVENT); > term_mouse_click (&ie, event, f); > + eassert (ie.kind == GPM_CLICK_EVENT); > if (tty_handle_tab_bar_click (f, event->x, event->y, > (ie.modifiers & down_modifier) != 0, > &ie)) > { > + eassert (ie.kind == GPM_CLICK_EVENT > + || ie.kind == TAB_BAR_EVENT); > /* tty_handle_tab_bar_click stores 2 events in the event > queue, so we are done here. */ > + /* FIXME: Actually, `tty_handle_tab_bar_click` returns true > + without storing any events, when > + (ie.modifiers & down_modifier) != 0 */ > count += 2; > return count; > } > + eassert (ie.kind == GPM_CLICK_EVENT); This assert is for the mouse click path, please remember this value. > } > > - done: > - if (ie.kind != NO_EVENT) > + if (ie.kind != NO_EVENT) /* FIXME: We retest the previous if's > condition! */ > { > + /* FIXME: `hold_quit` could already hold a previous event, > + so it risks crushing that previous event. > + What am I missing? */ > kbd_buffer_store_event_hold (&ie, hold_quit); This is the meat of the refactor. Given the current code hold_quit will never hold a previous event. tty_read_avail_input initializes hold_quit right before calling handle_one_term_event and and sets kind to NO_EVENT. This is the first and only time in this function hold_quit is passed to any other function. Because it is the first time passed, we can safely deduce that hold_quit->kind will still be NO_EVENT *unless* a previous loop iteration changed it. Because it is the only time passed, we can safely deduce that hold_quit->kind can only changed in a previous loop iteration by this function storing a quit event. However, this can not happen. hold_quit would get set by kbd_buffer_store_buffered_event which kbd_buffer_store_event_hold calls. To get set, the following must all be true: 1. An event of kind ASCII_KEYSTROKE_EVENT must be processed. 2. And that event's character code must be quit_char. 3. And that event must be on the current frame's KBOARD (not quite sure what this means). However, 1 will never be true as the only events that ever reach this point are of kind GPM_CLICK_EVENT, as deduced from the above asserts. > count++; > } > > if (do_help > + /* FIXME: `hold_quit` is never NULL?! */ Agreed. > + /* FIXME: Why do we test `hold_quit->kind != NO_EVENT` here? > + This either comes from `ie` (via kbd_buffer_store_event_hold > above) > + or from an earlier call to us (tty_read_avail_input calls us > in a > + loop with the same `hold_quit` struct). */ And from above reasoning, we know that hold_quit is always non-NULL and hold_quit->kind is always NO_EVENT. -- MJF