[O] bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template ‘g’: Match data clobbered by buffer modification hooks)

2016-07-26 Thread npostavs
Eli Zaretskii  writes:

>> From: Stefan Monnier 
>> Cc: rpl...@gmail.com,  23...@debbugs.gnu.org,  alex.ben...@linaro.org,  
>> jwieg...@gmail.com,  nljlistb...@gmail.com
>> Date: Tue, 19 Jul 2016 00:48:19 -0400
>> 
>> > The more general problem is when there's at least one more
>> > sub-expression, whose start and/or end are after the new EOB.
>> > Those sub-expression's data will be completely bogus after the
>> > adjustment,
>> 
>> If they were after the EOB, they were already bogus to start with.
>
> I think we are mis-communicating.  I mean the following scenario:
>
> Before call to replace_range in replace-match:
>
>|---|---|--||
>s1 e1   s2e2   EOB
>
> (s1, e1, etc. are the start and end of the corresponding
> sub-expressions.)
>
> After the call to replace_range in replace-match:
>
>|-|---|--||
>s1   e1   s2e2   EOB
>
> IOW, the 1st sub-expression got replaced with a much shorter text,
> which made EOB be smaller than the original beginning and end of the
> 2nd sub-expression.  There's nothing bogus with this, is there?  The
> user will expect to get match-data adjusted as shown in the second
> diagram, and that's what she will really get -- unless there are
> buffer-modification hooks that use save-match-data.  In the latter
> case, what the user will get instead is this:
>
>|-|---|--||
>s1   EOB
> e1
>   s2
>   e2
>
> and that is even before the adjustment code kicks in and makes
> "adjustments" with an incorrect adjustment value, which is computed as
>
>   newpoint = search_regs.start[sub] + SCHARS (newtext);
>   [...]
>   ptrdiff_t change = newpoint - search_regs.end[sub];
>
> and so will use the new EOB as search_regs.end[sub], instead of the
> correct original value of e1 from the first diagram above.
>
> IOW, the call to save-match-data in a buffer-modification hook
> _disrupts_ the normal operation of replace-match in this case, by
> indirectly sabotaging the adjustment of match data after the
> replacement.

Is it not possible to adjust the match data *before* calling buffer
modification hooks?  Seems to me the root of the problem is that buffer
modification hooks get to see this invalid intermediate state where the
match data is out of sync with the buffer.





[O] bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template ‘g’: Match data clobbered by buffer modification hooks)

2016-07-26 Thread npostavs
Stefan Monnier  writes:

>> Maybe there's a misunderstanding.  What Noam suggested was just to
>> move the code which adjusts search_regs.start[i] and .end[i] to before
>> the call to replace_range.
>
> Oh, right, that's also an option.  It might suffer from another problem,
> which is that the match-data will be broken while the
> before-change-functions are run, so if there's a save-match-data there
> we're back to square one.

Solution: adjust in between the before and after change functions.
Patch below.  I think there shouldn't be performance problems, although
it does add yet another flag to replace_range (by the way, I noticed
that the MARKERS flags is never 0, so it's redundant).  I tested with
the attached bug-23917-match-data-buffer-modhook.el.

>From a8098080dff5f83f7cbcbec2bc263f9db3b45ad9 Mon Sep 17 00:00:00 2001
From: Noam Postavsky 
Date: Wed, 20 Jul 2016 20:15:14 -0400
Subject: [PATCH v1] Adjust match data before calling after-change-funs

* src/insdel.c (replace_range): Add new parameter ADJUST_MATCH_DATA, if
true.  Update all callers except Freplace_match to pass 0 for the new
parameter.
* src/search.c (update_search_regs): New function, extracted from
Freplace_match.
(Freplace_match): Remove match data adjustment code, pass 1 for
ADJUST_MATCH_DATA to replace_range instead.
---
 src/cmds.c|  2 +-
 src/editfns.c |  6 +++---
 src/insdel.c  | 10 --
 src/lisp.h|  4 +++-
 src/search.c  | 50 +-
 5 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/src/cmds.c b/src/cmds.c
index 1e44ddd..4003d8b 100644
--- a/src/cmds.c
+++ b/src/cmds.c
@@ -447,7 +447,7 @@ internal_self_insert (int c, EMACS_INT n)
 	  string = concat2 (string, tem);
 	}
 
-  replace_range (PT, PT + chars_to_delete, string, 1, 1, 1);
+  replace_range (PT, PT + chars_to_delete, string, 1, 1, 1, 0);
   Fforward_char (make_number (n));
 }
   else if (n > 1)
diff --git a/src/editfns.c b/src/editfns.c
index 412745d..32c8bec 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -3192,7 +3192,7 @@ DEFUN ("subst-char-in-region", Fsubst_char_in_region,
 	  /* replace_range is less efficient, because it moves the gap,
 		 but it handles combining correctly.  */
 	  replace_range (pos, pos + 1, string,
-			 0, 0, 1);
+			 0, 0, 1, 0);
 	  pos_byte_next = CHAR_TO_BYTE (pos);
 	  if (pos_byte_next > pos_byte)
 		/* Before combining happened.  We should not increment
@@ -3405,7 +3405,7 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal,
 		  /* This is less efficient, because it moves the gap,
 		 but it should handle multibyte characters correctly.  */
 		  string = make_multibyte_string ((char *) str, 1, str_len);
-		  replace_range (pos, pos + 1, string, 1, 0, 1);
+		  replace_range (pos, pos + 1, string, 1, 0, 1, 0);
 		  len = str_len;
 		}
 	  else
@@ -3446,7 +3446,7 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal,
 		{
 		  string = Fmake_string (make_number (1), val);
 		}
-	  replace_range (pos, pos + len, string, 1, 0, 1);
+	  replace_range (pos, pos + len, string, 1, 0, 1, 0);
 	  pos_byte += SBYTES (string);
 	  pos += SCHARS (string);
 	  cnt += SCHARS (string);
diff --git a/src/insdel.c b/src/insdel.c
index 4ad1074..fc3f19f 100644
--- a/src/insdel.c
+++ b/src/insdel.c
@@ -1268,7 +1268,9 @@ adjust_after_insert (ptrdiff_t from, ptrdiff_t from_byte,
 /* Replace the text from character positions FROM to TO with NEW,
If PREPARE, call prepare_to_modify_buffer.
If INHERIT, the newly inserted text should inherit text properties
-   from the surrounding non-deleted text.  */
+   from the surrounding non-deleted text.
+   If ADJUST_MATCH_DATA, then adjust the match data before calling
+   signal_after_change.  */
 
 /* Note that this does not yet handle markers quite right.
Also it needs to record a single undo-entry that does a replacement
@@ -1279,7 +1281,8 @@ adjust_after_insert (ptrdiff_t from, ptrdiff_t from_byte,
 
 void
 replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new,
-	   bool prepare, bool inherit, bool markers)
+   bool prepare, bool inherit, bool markers,
+   bool adjust_match_data)
 {
   ptrdiff_t inschars = SCHARS (new);
   ptrdiff_t insbytes = SBYTES (new);
@@ -1426,6 +1429,9 @@ replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new,
   MODIFF++;
   CHARS_MODIFF = MODIFF;
 
+  if (adjust_match_data)
+update_search_regs (from, to, from + SCHARS (new));
+
   signal_after_change (from, nchars_del, GPT - from);
   update_compositions (from, GPT, CHECK_BORDER);
 }
diff --git a/src/lisp.h b/src/lisp.h
index 6a98adb..25f811e 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3516,7 +3516,7 @@ extern void adjust_after_insert (ptrdiff_t, ptrdiff_t, ptrdiff_t,
  ptrdiff_t, ptrdiff_t);
 extern void adjust_markers_for_delete (ptrdiff_t, ptrdiff_t,
    

[O] bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template ‘g’: Match data clobbered by buffer modification hooks)

2016-07-26 Thread npostavs
Eli Zaretskii  writes:

>> From: npost...@users.sourceforge.net
>> Cc: 23...@debbugs.gnu.org,  nljlistb...@gmail.com,  jwieg...@gmail.com,  
>> rpl...@gmail.com,  monn...@iro.umontreal.ca,  alex.ben...@linaro.org
>> Date: Wed, 20 Jul 2016 23:00:59 -0400
>> 
>> > Please also make sure bug#23869 is still fixed after this.
>> 
>> Following the recipe in
>> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23869#11 gives me 'Lisp
>> error: (error "Match data clobbered by buffer modification hooks")',
>> that indicates it's still fixed, right?
>
> Yes, but I thought we want to remove the error-out code.  Since we now
> protect ourselves from clobbered data, we don't need that extra
> protection, and I think leaving it in place will cause false positives
> (as a few people already reported).  That's because the adjustment of
> the search registers in the new function you introduce will itself
> trigger the error message, won't it?

I made the same adjustments to the saved sub_start and sub_end
variables, but I had a mistake in that adjustment which caused the false
positives.  Fixed in the attached v2 patch.  We could just drop the
check, though I've already found it useful to catch bugs
(https://github.com/joaotavora/yasnippet/issues/720).

If I drop the checks (see attached v3 patch), then after following the
bug#23869 recipe, I get:

## -*- Octave -*-
-module(bug).
-export([identity/1, is_even/1, size/1, reverse/1]).

>From 3e8f9b9f89108bcebf04e06e41a5ce2c719c6ad2 Mon Sep 17 00:00:00 2001
From: Noam Postavsky 
Date: Wed, 20 Jul 2016 20:15:14 -0400
Subject: [PATCH v2] Adjust match data before calling after-change-funs

* src/insdel.c (replace_range): Add new parameter ADJUST_MATCH_DATA, if
true call update_search_regs.  Update all callers (except
Freplace_match) to pass 0 for the new parameter.
* src/search.c (update_search_regs): New function, extracted from
Freplace_match.
(Freplace_match): Remove match data adjustment code, pass 1 for
ADJUST_MATCH_DATA to replace_range instead.
---
 src/cmds.c|  2 +-
 src/editfns.c |  6 +++---
 src/insdel.c  | 10 --
 src/lisp.h|  4 +++-
 src/search.c  | 52 ++--
 5 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/src/cmds.c b/src/cmds.c
index 1e44ddd..4003d8b 100644
--- a/src/cmds.c
+++ b/src/cmds.c
@@ -447,7 +447,7 @@ internal_self_insert (int c, EMACS_INT n)
 	  string = concat2 (string, tem);
 	}
 
-  replace_range (PT, PT + chars_to_delete, string, 1, 1, 1);
+  replace_range (PT, PT + chars_to_delete, string, 1, 1, 1, 0);
   Fforward_char (make_number (n));
 }
   else if (n > 1)
diff --git a/src/editfns.c b/src/editfns.c
index 412745d..32c8bec 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -3192,7 +3192,7 @@ DEFUN ("subst-char-in-region", Fsubst_char_in_region,
 	  /* replace_range is less efficient, because it moves the gap,
 		 but it handles combining correctly.  */
 	  replace_range (pos, pos + 1, string,
-			 0, 0, 1);
+			 0, 0, 1, 0);
 	  pos_byte_next = CHAR_TO_BYTE (pos);
 	  if (pos_byte_next > pos_byte)
 		/* Before combining happened.  We should not increment
@@ -3405,7 +3405,7 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal,
 		  /* This is less efficient, because it moves the gap,
 		 but it should handle multibyte characters correctly.  */
 		  string = make_multibyte_string ((char *) str, 1, str_len);
-		  replace_range (pos, pos + 1, string, 1, 0, 1);
+		  replace_range (pos, pos + 1, string, 1, 0, 1, 0);
 		  len = str_len;
 		}
 	  else
@@ -3446,7 +3446,7 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal,
 		{
 		  string = Fmake_string (make_number (1), val);
 		}
-	  replace_range (pos, pos + len, string, 1, 0, 1);
+	  replace_range (pos, pos + len, string, 1, 0, 1, 0);
 	  pos_byte += SBYTES (string);
 	  pos += SCHARS (string);
 	  cnt += SCHARS (string);
diff --git a/src/insdel.c b/src/insdel.c
index 4ad1074..fc3f19f 100644
--- a/src/insdel.c
+++ b/src/insdel.c
@@ -1268,7 +1268,9 @@ adjust_after_insert (ptrdiff_t from, ptrdiff_t from_byte,
 /* Replace the text from character positions FROM to TO with NEW,
If PREPARE, call prepare_to_modify_buffer.
If INHERIT, the newly inserted text should inherit text properties
-   from the surrounding non-deleted text.  */
+   from the surrounding non-deleted text.
+   If ADJUST_MATCH_DATA, then adjust the match data before calling
+   signal_after_change.  */
 
 /* Note that this does not yet handle markers quite right.
Also it needs to record a single undo-entry that does a replacement
@@ -1279,7 +1281,8 @@ adjust_after_insert (ptrdiff_t from, ptrdiff_t from_byte,
 
 void
 replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new,
-	   bool prepare, bool inherit, bool markers)
+   bool prepare, bool inherit, bool markers,
+   bool 

[O] bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template ‘g’: Match data clobbered by buffer modification hooks)

2016-07-26 Thread npostavs
tags 23917 fixed
close 23917 25.1
quit

Eli Zaretskii  writes:

>> From: npost...@users.sourceforge.net
>> Cc: 23...@debbugs.gnu.org,  nljlistb...@gmail.com,  jwieg...@gmail.com,  
>> rpl...@gmail.com,  monn...@iro.umontreal.ca,  alex.ben...@linaro.org
>> Date: Thu, 21 Jul 2016 21:08:43 -0400
>> 
>> I made the same adjustments to the saved sub_start and sub_end
>> variables, but I had a mistake in that adjustment which caused the false
>> positives.  Fixed in the attached v2 patch.  We could just drop the
>> check, though I've already found it useful to catch bugs
>> (https://github.com/joaotavora/yasnippet/issues/720).
>> 
>> If I drop the checks (see attached v3 patch), then after following the
>> bug#23869 recipe, I get:
>> 
>> ## -*- Octave -*-
>> -module(bug).
>> -export([identity/1, is_even/1, size/1, reverse/1]).
>
> OK, let's wait for a few days to give time to the people who were
> affected by the issue to test the patch, and if no new issues come up,
> please push the version with the error code to emacs-25.

Since they've now reported that the new patch is working well, I've
pushed it as e1def617.







[O] bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template ‘g’: Match data clobbered by buffer modification hooks)

2016-07-26 Thread npostavs
Eli Zaretskii  writes:

>> From: npost...@users.sourceforge.net
>> Cc: Eli Zaretskii ,  23...@debbugs.gnu.org,  
>> nljlistb...@gmail.com,  jwieg...@gmail.com,  rpl...@gmail.com,  
>> alex.ben...@linaro.org
>> Date: Wed, 20 Jul 2016 20:56:28 -0400
>> 
>> >> Maybe there's a misunderstanding.  What Noam suggested was just to
>> >> move the code which adjusts search_regs.start[i] and .end[i] to before
>> >> the call to replace_range.
>> >
>> > Oh, right, that's also an option.  It might suffer from another problem,
>> > which is that the match-data will be broken while the
>> > before-change-functions are run, so if there's a save-match-data there
>> > we're back to square one.
>> 
>> Solution: adjust in between the before and after change functions.
>> Patch below.  I think there shouldn't be performance problems, although
>> it does add yet another flag to replace_range (by the way, I noticed
>> that the MARKERS flags is never 0, so it's redundant).  I tested with
>> the attached bug-23917-match-data-buffer-modhook.el.
>
> Thanks.
>
> Please also make sure bug#23869 is still fixed after this.

Following the recipe in
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23869#11 gives me 'Lisp
error: (error "Match data clobbered by buffer modification hooks")',
that indicates it's still fixed, right?





[O] bug#25132: 26.0.50; emacs hangs when loading org file with python source blocks

2017-01-23 Thread npostavs
Dmitry Gutov  writes:

> On 20.01.2017 03:52, npost...@users.sourceforge.net wrote:
>
>> My feeling is that inhibit-modification-hooks should usually be buffer
>> local anyway.
>
> Maybe you're right.
>
> inhibit-read-only, bound nearby, seems to be in the same situation.
>
>>> If we are not, why not make inhibit-modification-hooks always
>>> buffer-local instead?
>>
>> It would have to be in addition to, because even after doing
>> (make-variable-buffer-local 'var), (let ((var 'foo))...) still makes a
>> global binding.  `make-variable-buffer-local' only has effect for
>> `setq', which I think will hardly ever happen for
>> `inhibit-modification-hooks'.
>
> You're right, and that sounds a little too complicated for my taste.
>
> So, personally, I'd try to fix the particular instance
> first. Switching buffers inside with-silent-modifications is not a
> very common usage, I think.
>
> Maybe org-src should itself let-bind the aforementioned variable(s)
> where it visits other buffers.

Yeah, that works, and is my proposal for emacs-25, but I'm still leaning
towards solving this more broadly in with-silent-modifications, probably
also add a mention about this to the inhibit-modification-hooks
docstring.

I think doing the same to inhibit-read-only isn't worth the trouble
because if it happens to be let-bound to t in a buffer where it wasn't
"supposed" to be, the worst that happens is that an error *isn't*
thrown.





[O] bug#25132: 26.0.50; emacs hangs when loading org file with python source blocks

2017-01-29 Thread npostavs
tags 25132 fixed
close 25132 25.2
quit

npost...@users.sourceforge.net writes:

> Dmitry Gutov  writes:
>
>> So, personally, I'd try to fix the particular instance
>> first. Switching buffers inside with-silent-modifications is not a
>> very common usage, I think.
>>
>> Maybe org-src should itself let-bind the aforementioned variable(s)
>> where it visits other buffers.
>
> Yeah, that works, and is my proposal for emacs-25, but I'm still leaning
> towards solving this more broadly in with-silent-modifications, probably
> also add a mention about this to the inhibit-modification-hooks
> docstring.

I changed my mind.  Bug#25561 reminded me about the "Making  local
to  while let-bound!" message.  My change to
`with-silent-modifications' would trigger that on any nested invocations
of `with-silent-modifications' which seems more likely to happen than
switching buffers.

I've pushed the simpler let-bind in org-src solution [1: ae8264c] to
emacs-25.

1: 2017-01-29 11:01:32 -0500 ae8264c5cccf19d5b25a340a605bf2f07de1577e
  Call modification hooks in org-src fontify buffers





[O] bug#25132: 26.0.50; emacs hangs when loading org file with python source blocks

2017-01-19 Thread npostavs
Dmitry Gutov  writes:

> On 08.01.2017 00:20, npost...@users.sourceforge.net wrote:
>> -(inhibit-modification-hooks t))
>> +(inhibit-modification-hooks
>> + (progn (make-local-variable 'inhibit-modification-hooks) t)))
>
> Are we not worried that inhibit-modificaiton-hooks will become
> buffer-local even after control flow leaves this let*?

My feeling is that inhibit-modification-hooks should usually be buffer
local anyway.

> If we are not, why not make inhibit-modification-hooks always
> buffer-local instead?

It would have to be in addition to, because even after doing
(make-variable-buffer-local 'var), (let ((var 'foo))...) still makes a
global binding.  `make-variable-buffer-local' only has effect for
`setq', which I think will hardly ever happen for
`inhibit-modification-hooks'.

Actually, I just grepped for inhibit-modification-hooks and the only
non-let I found is this:

(defun read-passwd (prompt  confirm default)
   ...
  (minibuffer-with-setup-hook
  (lambda ()
 ...
 (setq-local inhibit-modification-hooks nil) ;bug#15501.
 ...))
   ...)





[O] bug#25132: 26.0.50; emacs hangs when loading org file with python source blocks

2017-01-19 Thread npostavs
Clément Pit--Claudel  writes:

> On 2017-01-19 19:52, npost...@users.sourceforge.net wrote:
>> because even after doing (make-variable-buffer-local 'var), (let
>> ((var 'foo))...) still makes a global binding.
>> `make-variable-buffer-local' only has effect for `setq', which I
>> think will hardly ever happen for `inhibit-modification-hooks'.
>
> On 2017-01-19 19:52, npost...@users.sourceforge.net wrote:
>> because even after doing (make-variable-buffer-local 'var), (let
>> ((var 'foo))...) still makes a global binding.
>> `make-variable-buffer-local' only has effect for `setq', which I
>> think will hardly ever happen for `inhibit-modification-hooks'.
>
> Hi Noam,
>
> Can you explain a bit more? I'm not sure what you meant.
>
> I tried the following to illustrate your point:
>
> (defvar aa 0)
>
> (with-temp-buffer
>   (setq-local aa 1)
>   (let ((b1 (current-buffer)))
> (with-temp-buffer
>   (let ((aa 2))
> (message "In b2: %S" aa)
> (with-current-buffer b1
>   (message "In b1: %S" aa))

My point was that the setq-local (or make-local-variable) is required
and that defvar-local (or make-variable-buffer-local) is not enough.

Compare:

(defvar-local bb 0)

(with-temp-buffer
  (let ((b1 (current-buffer)))
(with-temp-buffer
  (let ((bb 2))
(message "In b2: %S" bb)
(with-current-buffer b1
  (message "In b1: %S" bb))





[O] bug#25132: 26.0.50; emacs hangs when loading org file with python source blocks

2017-01-07 Thread npostavs
tags 25132 patch
quit

npost...@users.sourceforge.net writes:
> The problem is that org updates its temporary fontification buffer from
> its fontify rules which are called by jit-lock-function, which means
> that inhibit-modification-hooks is bound to t.  Therefore, when
> org-src-font-lock-fontify-block calls delete-region to remove leftover text 
> from
> the previous source block fontification, the `before-change-functions'
> are not run.  In this case `syntax-ppss-flush-cache' is the important
> function that doesn't get run, so `syntax-propertize--done' is still set
> from before and messes up python.el's fontification routines.

I think with-silent-modifications should let-bind
inhibit-modification-hooks buffer locally:

>From da4f1c0338b2b98f97a553568c4b80872484ee97 Mon Sep 17 00:00:00 2001
From: Noam Postavsky 
Date: Sat, 7 Jan 2017 15:47:37 -0500
Subject: [PATCH v1] Inhibit modification hooks buffer locally

`with-silent-modifications' let-binds `inhibit-modification-hooks' to t
globally.  So modifications to other buffers don't trigger modication
hooks.  This causes unexpected results when functions called from
`jit-lock-function' use temporary buffers and modifies them (Bug#25132).

* lisp/subr.el (with-silent-modifications): Bind
inhibit-modification-hooks buffer locally.
---
 lisp/subr.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 5377416..fe20d68 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3298,7 +3298,8 @@ with-silent-modifications
 `(let* ((,modified (buffer-modified-p))
 (buffer-undo-list t)
 (inhibit-read-only t)
-(inhibit-modification-hooks t))
+(inhibit-modification-hooks
+ (progn (make-local-variable 'inhibit-modification-hooks) t)))
(unwind-protect
(progn
  ,@body)
-- 
2.9.3


Perhaps the other variables it binds should be buffer local as well?

This bug is new in Emacs 25.1, but changing with-silent-modifications is
a bit risky.  Therefore, I propose the following for the emacs-25
branch:

>From 338aa0c37eba0401616e8e02f0143a57edffd486 Mon Sep 17 00:00:00 2001
From: Noam Postavsky 
Date: Sat, 7 Jan 2017 16:05:19 -0500
Subject: [PATCH v1] Call modification hooks in org-src fontify buffers

* lisp/org/org-src.el (org-src-font-lock-fontify-block): Let-bind
`inhibit-modification-hooks' to nil, since this function can be called
from jit-lock-function which binds that variable to t (Bug#25132).
---
 lisp/org/org-src.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/org/org-src.el b/lisp/org/org-src.el
index d01f108..9b66907 100644
--- a/lisp/org/org-src.el
+++ b/lisp/org/org-src.el
@@ -913,8 +913,9 @@ org-src-font-lock-fontify-block
 	  (with-current-buffer
 	  (get-buffer-create
 	   (concat " org-src-fontification:" (symbol-name lang-mode)))
-	(delete-region (point-min) (point-max))
-	(insert string " ") ;; so there's a final property change
+	(let ((inhibit-modification-hooks nil)) ; Avoid Bug#25132.
+  (delete-region (point-min) (point-max))
+  (insert string " ")) ;; so there's a final property change
 	(unless (eq major-mode lang-mode) (funcall lang-mode))
 (org-font-lock-ensure)
 	(setq pos (point-min))
-- 
2.9.3



[O] bug#25132: 26.0.50; emacs hangs when loading org file with python source blocks

2017-01-06 Thread npostavs
tags 25132 confirmed
quit

The problem is that org updates its temporary fontification buffer from
its fontify rules which are called by jit-lock-function, which means
that inhibit-modification-hooks is bound to t.  Therefore, when
org-src-font-lock-fontify-block calls delete-region to remove leftover text from
the previous source block fontification, the `before-change-functions'
are not run.  In this case `syntax-ppss-flush-cache' is the important
function that doesn't get run, so `syntax-propertize--done' is still set
from before and messes up python.el's fontification routines.

  org-src-font-lock-fontify-block(#("python" 0 6 (fontified t)) 19 65)
  org-fontify-meta-lines-and-blocks-1(172)
  org-fontify-meta-lines-and-blocks(172)
  font-lock-fontify-keywords-region(1 172 nil)
  font-lock-default-fontify-region(1 172 nil)
  font-lock-fontify-region(1 172)
  ...
  jit-lock--run-functions(1 172)
  jit-lock-fontify-now(1 501)
  jit-lock-function(1)
  redisplay_internal\ \(C\ function\)()
  redisplay()
  sit-for(2)
  execute-extended-command(nil "25132-test" "25")
  funcall-interactively(execute-extended-command nil "25132-test" "25")
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

(defun org-src-font-lock-fontify-block (lang start end)
  ...
  (with-current-buffer
  (get-buffer-create
   (concat " org-src-fontification:" (symbol-name lang-mode)))
(delete-region (point-min) (point-max)) ;<-- 
`syntax-propertize--done' not reset here!
(insert string " ") ;; so there's a final property change
(unless (eq major-mode lang-mode) (funcall lang-mode))
(org-font-lock-ensure)
...)
  ...)

(defun jit-lock-function (start)
  ...
  (jit-lock-fontify-now start (+ start jit-lock-chunk-size))
  ...)

(defun jit-lock-fontify-now ( start end)
  "Fontify current buffer from START to END.
Defaults to the whole buffer.  END can be out of bounds."
  (with-buffer-prepared-for-jit-lock
...))

  (defmacro with-buffer-prepared-for-jit-lock ( body)
"Execute BODY in current buffer, overriding several variables.
Preserves the `buffer-modified-p' state of the current buffer."
(declare (debug t))
`(let ((inhibit-point-motion-hooks t))
   (with-silent-modifications ; <-- binds inhibit-modification-hooks to 
t
 ,@body)))