From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: Overalays and point-entered Date: Thu, 10 Dec 2009 03:32:59 -0500 Message-ID: References: <5e3a506e0909101709u2259d56h25f3ef1ec67326aa@mail.gmail.com> <5e3a506e0910230843r3fb837e7v9aa9cf5e57a7aed@mail.gmail.com> <5e3a506e0910270142y799d80dm7c4ebda24e31556@mail.gmail.com> <87aazcfiud.fsf@catnip.gol.com> <5e3a506e0910311003g1a16874em8e51baed60099a48@mail.gmail.com> <5e3a506e0911060654w6220a69j9e2af2b6988a4c5f@mail.gmail.com> <5e3a506e0912091541v71f7c4ebq6daf0bcac2ddccd2@mail.gmail.com> <5e3a506e0912091937v4a6ab81egee0529bb9603dfc8@mail.gmail.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1260434002 28066 80.91.229.12 (10 Dec 2009 08:33:22 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 10 Dec 2009 08:33:22 +0000 (UTC) Cc: emacs-devel@gnu.org, Miles Bader To: Nathaniel Flath Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Dec 10 09:33:14 2009 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1NIeSY-0004I8-JP for ged-emacs-devel@m.gmane.org; Thu, 10 Dec 2009 09:33:14 +0100 Original-Received: from localhost ([127.0.0.1]:46215 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NIeSY-00063g-7s for ged-emacs-devel@m.gmane.org; Thu, 10 Dec 2009 03:33:14 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NIeSS-00063b-TU for emacs-devel@gnu.org; Thu, 10 Dec 2009 03:33:08 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NIeSO-000631-0c for emacs-devel@gnu.org; Thu, 10 Dec 2009 03:33:08 -0500 Original-Received: from [199.232.76.173] (port=58366 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NIeSN-00062y-Rc for emacs-devel@gnu.org; Thu, 10 Dec 2009 03:33:03 -0500 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:43081 helo=ironport2-out.pppoe.ca) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NIeSL-0007y2-5V; Thu, 10 Dec 2009 03:33:01 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AvYEAIJBIEtMCpT8/2dsb2JhbACBS9N7hCwEihw X-IronPort-AV: E=Sophos;i="4.47,374,1257138000"; d="scan'208";a="51319471" Original-Received: from 76-10-148-252.dsl.teksavvy.com (HELO pastel.home) ([76.10.148.252]) by ironport2-out.pppoe.ca with ESMTP; 10 Dec 2009 03:32:59 -0500 Original-Received: by pastel.home (Postfix, from userid 20848) id C1B5C8065; Thu, 10 Dec 2009 03:32:59 -0500 (EST) In-Reply-To: <5e3a506e0912091937v4a6ab81egee0529bb9603dfc8@mail.gmail.com> (Nathaniel Flath's message of "Wed, 9 Dec 2009 22:37:35 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (gnu/linux) X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:118516 Archived-At: > The patch is attached - please let me know if you have any comments. I'm wondering whether it should be described as complete or complex. Especially for a feature which hsan't yet found a user. But it might indeed be the case that most/all of that info will be needed. Here are some comments on your patch: Index: buffer.c =================================================================== RCS file: /sources/emacs/emacs/src/buffer.c,v retrieving revision 1.591 diff -r1.591 buffer.c 399c399,401 < --- Please always use unified diff format (or context diff format). Index: buffer.h =================================================================== RCS file: /sources/emacs/emacs/src/buffer.h,v retrieving revision 1.130 diff -r1.130 buffer.h 560a561,567 > /* Vector of overlays that were on this window the last time > run_point_motion_hooks was run and this window was current */ > Lisp_Object* overlay_prev_vec; > > /* Number of overlays in overlay_prev_vec */ > unsigned int noverlays_prev; Only Lisp_Object fields are traced by the GC, so overlay_prev_vec as it stands will probably lead to core dumps occasionally. > /* Returns an array of overlays that are active at the indication position and buffer. > The lenght of the array will be stored in num_overlays. */ ^^ Please, state that the array is allocated with xmalloc. > if (!NILP (point_motion) && We like to put infix operators at the beginning rather than the end of lines. > run_point_motion_hooks (prev_c_buffer) This seems to run the hooks for all overlays at the starting position and all overlays at the ending position. If you add to that the fact that it runs them for windows and for the buffer, you get that under the usual circumstance of a command within the same buffer displayed in a single window, moving within the same overlay we run the hook 4 times? I think we need to be more careful and only run the hook when crossing the boundary, i.e. when moving out of or into an overlay with that property. But even if we decide to run it for all movements, we should be careful to run it a bit less liberally. It's probably OK to occasionally have a few spurious extra runs of the hook, but your current code needs to be a lot more careful. > check_run_point_motion_hook (overlay_cur_vec[i], > prev_buffer, > prev_window, > Qt, > 0, > 0); Please use NULL for pointers and keep 0 for integers. Especially in such contexts with numerous arguments, it helps figure out which is which. One more thing: I think the buffer and window object should only store the few overlays that do have a point-motion property. Maybe they could even limit themselves to storing "the" overlay with a point-motion property (if there are several, it should take the one with highest priority). Stefan