From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.bugs Subject: bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned. Date: Sun, 22 Mar 2015 14:13:35 +0000 Message-ID: <20150322141335.GA3053@acm.fritz.box> References: <20150319230136.GC2753@acm.fritz.box> <20150320160744.GA3493@acm.fritz.box> <20150321000003.GF3493@acm.fritz.box> <20150321210325.GC3001@acm.fritz.box> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1427033667 24701 80.91.229.3 (22 Mar 2015 14:14:27 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 22 Mar 2015 14:14:27 +0000 (UTC) Cc: 20146@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Mar 22 15:14:15 2015 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1YZgdm-0004Db-Pe for geb-bug-gnu-emacs@m.gmane.org; Sun, 22 Mar 2015 15:14:11 +0100 Original-Received: from localhost ([::1]:51173 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YZgdm-0008HA-2B for geb-bug-gnu-emacs@m.gmane.org; Sun, 22 Mar 2015 10:14:10 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:36951) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YZgdh-0008Gr-EL for bug-gnu-emacs@gnu.org; Sun, 22 Mar 2015 10:14:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YZgde-0007ST-6j for bug-gnu-emacs@gnu.org; Sun, 22 Mar 2015 10:14:05 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:42724) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YZgde-0007SN-35 for bug-gnu-emacs@gnu.org; Sun, 22 Mar 2015 10:14:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1YZgdd-0000Ke-QR for bug-gnu-emacs@gnu.org; Sun, 22 Mar 2015 10:14:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 22 Mar 2015 14:14:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 20146 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 20146-submit@debbugs.gnu.org id=B20146.14270336391259 (code B ref 20146); Sun, 22 Mar 2015 14:14:01 +0000 Original-Received: (at 20146) by debbugs.gnu.org; 22 Mar 2015 14:13:59 +0000 Original-Received: from localhost ([127.0.0.1]:60733 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1YZgda-0000KE-3v for submit@debbugs.gnu.org; Sun, 22 Mar 2015 10:13:59 -0400 Original-Received: from colin.muc.de ([193.149.48.1]:28965 helo=mail.muc.de) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1YZgdX-0000K4-85 for 20146@debbugs.gnu.org; Sun, 22 Mar 2015 10:13:56 -0400 Original-Received: (qmail 10110 invoked by uid 3782); 22 Mar 2015 14:13:53 -0000 Original-Received: from acm.muc.de (pD951ADA2.dip0.t-ipconnect.de [217.81.173.162]) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 22 Mar 2015 15:13:52 +0100 Original-Received: (qmail 3953 invoked by uid 1000); 22 Mar 2015 14:13:35 -0000 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:100780 Archived-At: Hello, Stefan. On Sat, Mar 21, 2015 at 06:30:26PM -0400, Stefan Monnier wrote: > >> The major mode sets font-lock-extend-region-function and this functions' > >> result should be (and is) respected by the rest of font-lock. > > If only, but this is simply not the case at the moment. > > jit-lock-fontify-now has a hard-coded extension to whole lines, > > regardless of the contents of font-lock-extend-region-functions. > There's no relationship between the two. The bounds that > font-lock-fontify-region [gets] are not under the major mode's control. Yes they are. They can be set by font-lock-extend-after-change-region-function, as described in the elisp manual. > What is under the major mode's control is how those bounds are extended > by font-lock-extend-region-functions. But by that time, jit-lock has already played fast and loose with the bounds supplied by the major mode. It is too late by that time for anything in font-lock-fontify-region to repair the damage. > > This is surely a bug. > Nope. Yes. I think you've said, indeed quite often, that font-lock has the same behaviour regardless of whether jit-lock is enabled or not. Here, there is a marked difference of behaviour: different regions are provided to font-lock-fontify-region in the two cases. > > As a better idea, why not have jit-lock-fontify-now use > > f-l-extend-region-functions, and then pass the original `start' and > > `next' to f-l-fontify-region. > No, you have that backwards: it's f-l-fontify-region which runs > f-l-extend-region-functions, specifically so that you don't need to care > about what jit-lock (and other callers of f-l-fontify-region) might do > (you just need to make sure that f-l-fontify-region works correctly for > *any* bounds). I need to care a great deal about what jit-lock does, for the reasons in this thread. One way out of the current problem might involve testing font-lock-support-mode in the CC Mode code. This would not be ideal. Looking more critically at jit-lock-fontify-now, there is simply no need for it to expand the region to whole lines. The only things it ever does itself with (start next) are (i) Update jit-lock-context-unfontify-pos; (ii) Set fontified text properties; (iii) Call f-l-fontify-region (and other functions on jit-lock-functions). Not extending (start next) to whole lines (i) wouldn't make any difference, or if it would, it could be handled at the place where jit-lock-context-unfontify-pos is set; (ii) wouldn't make any difference at all; (iii) wouldn't make any difference to f-l-fontify-region, which itself does all the requisite region extending. The other functions (bug-reference-fontify, glasses-change, goto-address-fontify-region) may need a little adjustment, but probably not. As a bonus, not extending (start next) would completely prevent the condition that triggers (via a timer) jit-lock-force-redisplay, so we could remove that part of the code and j-l-f-redisplay itself. (It is true that buffer positions before start could have been refontified, but that is also true of the existing code). Likewise, there is then no longer any purpose in extending to whole lines in font-lock-extend-jit-lock-region-after-change, so we can remove that bit, too. All in all, a massive simplification, and it allows #19669 to be fixed easily, at least for Emacs 25.1. Abstractly seen, splitting the action of extension between two modules is poor design, leading to problems. Part of this extending MUST be done in f-l-fontify-region, so it makes sense to do it only there. > >> But callers of font-lock-fontify-region (such as > >> font-lock-after-change-function, or jit-lock) can choose *any* bounds > >> they feel like and font-lock-fontify-region should behave correctly. > > This seems to be true. When I (setq font-lock-support-mode nil), things > > seem to behave properly. It seems a poor workaround, though. > I don't see how what you say relates to what you quoted. Sorry, it was a bit opaque. I think I was saying that disabling jit-lock enables the major mode to chose the bounds given to f-l-fontify-region (via f-l-extend-after-change-r-f), and that f-l-f-r does indeed behave correctly w.r.t. whatever bounds it gets. But disabling jit-lock would be a poor workaround to the problem. So, as an opener, I propose the following improvement: diff --git a/lisp/font-lock.el b/lisp/font-lock.el index 1838a0f..c62945d 100644 --- a/lisp/font-lock.el +++ b/lisp/font-lock.el @@ -1316,20 +1316,6 @@ This function does 2 things: ;; the line was deleted. Or a \n was inserted in the middle ;; of a line. (1+ end)))) - ;; Finally, pre-enlarge the region to a whole number of lines, to try - ;; and anticipate what font-lock-default-fontify-region will do, so as to - ;; avoid double-redisplay. - ;; We could just run `font-lock-extend-region-functions', but since - ;; the only purpose is to avoid the double-redisplay, we prefer to - ;; do here only the part that is cheap and most likely to be useful. - (when (memq 'font-lock-extend-region-wholelines - font-lock-extend-region-functions) - (goto-char beg) - (setq beg (min jit-lock-start (line-beginning-position))) - (goto-char end) - (setq end - (max jit-lock-end - (if (bolp) (point) (line-beginning-position 2))))) (setq jit-lock-start beg jit-lock-end end)))) diff --git a/lisp/jit-lock.el b/lisp/jit-lock.el index 788646c..933bbe3 100644 --- a/lisp/jit-lock.el +++ b/lisp/jit-lock.el @@ -366,7 +366,7 @@ Defaults to the whole buffer. END can be out of bounds." ;; from the end of a buffer to its start, can do repeated ;; `parse-partial-sexp' starting from `point-min', which can ;; take a long time in a large buffer. - (let ((orig-start start) next) + (let (next) (save-match-data ;; Fontify chunks beginning at START. The end of a ;; chunk is either `end', or the start of a region @@ -376,15 +376,6 @@ Defaults to the whole buffer. END can be out of bounds." (setq next (or (text-property-any start end 'fontified t) end)) - ;; Decide which range of text should be fontified. - ;; The problem is that START and NEXT may be in the - ;; middle of something matched by a font-lock regexp. - ;; Until someone has a better idea, let's start - ;; at the start of the line containing START and - ;; stop at the start of the line following NEXT. - (goto-char next) (setq next (line-beginning-position 2)) - (goto-char start) (setq start (line-beginning-position)) - ;; Make sure the contextual refontification doesn't re-refontify ;; what's already been refontified. (when (and jit-lock-context-unfontify-pos @@ -409,35 +400,9 @@ Defaults to the whole buffer. END can be out of bounds." (quit (put-text-property start next 'fontified nil) (funcall 'signal (car err) (cdr err)))) - ;; The redisplay engine has already rendered the buffer up-to - ;; `orig-start' and won't notice if the above jit-lock-functions - ;; changed the appearance of any part of the buffer prior - ;; to that. So if `start' is before `orig-start', we need to - ;; cause a new redisplay cycle after this one so that any changes - ;; are properly reflected on screen. - ;; To make such repeated redisplay happen less often, we can - ;; eagerly extend the refontified region with - ;; jit-lock-after-change-extend-region-functions. - (when (< start orig-start) - (run-with-timer 0 nil #'jit-lock-force-redisplay - (copy-marker start) (copy-marker orig-start))) - ;; Find the start of the next chunk, if any. (setq start (text-property-any next end 'fontified nil)))))))) -(defun jit-lock-force-redisplay (start end) - "Force the display engine to re-render START's buffer from START to END. -This applies to the buffer associated with marker START." - (when (marker-buffer start) - (with-current-buffer (marker-buffer start) - (with-buffer-prepared-for-jit-lock - (when (> end (point-max)) - (setq end (point-max) start (min start end))) - (when (< start (point-min)) - (setq start (point-min) end (max start end))) - ;; Don't cause refontification (it's already been done), but just do - ;; some random buffer change, so as to force redisplay. - (put-text-property start end 'fontified t))))) ;;; Stealth fontification. > Stefan -- Alan Mackenzie (Nuremberg, Germany).