[O] bug#23917: 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 Sebastian Wiesner
Please do not CC me on Emacs bug threads.  If there's an issue in Flycheck 
please take it to our issue tracker at 
https://github.com/flycheck/flycheck/issues where we can keep track of it.





[O] bug#23917: 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-19 Thread Eli Zaretskii
> From: Alex Bennée 
> Cc: monn...@iro.umontreal.ca, 23...@debbugs.gnu.org, rpl...@gmail.com, 
> jwieg...@gmail.com, nljlistb...@gmail.com, m...@lunaryorn.com
> Date: Tue, 19 Jul 2016 18:45:44 +0100
> 
>   ;; Save and restore the match data, as recommended in (elisp)Change Hooks
>   (save-match-data
> (when flycheck-mode
>   ;; The buffer was changed, thus clear the idle timer
>   (flycheck-clear-idle-change-timer)
>   (if (string-match-p (rx "\n") (buffer-substring beg end))
>   (flycheck-buffer-automatically 'new-line 'force-deferred)
> (setq flycheck-idle-change-timer
>   (run-at-time flycheck-idle-change-delay nil
>#'flycheck-handle-idle-change))
> 
> However it doesn't look as though it tweaks the buffer until idle timer
> has run. Weird

Tweaking the buffer is not what causes the problem.  It's the call to
save-match-data itself.  It doesn't matter at all what the code inside
save-match-data does.






[O] bug#23917: 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-19 Thread Alex Bennée

Eli Zaretskii  writes:

>> From: Alex Bennée 
>> Cc: Stefan Monnier , 23...@debbugs.gnu.org, 
>> rpl...@gmail.com, jwieg...@gmail.com, nljlistb...@gmail.com
>> Date: Tue, 19 Jul 2016 18:05:37 +0100
>>
>> > Do we care that using save-match-data in every call to replace-match
>> > might mean a performance hit?  If it will, then this will again punish
>> > most of the users for the benefit of those few who (1) have
>> > buffer-modification hooks, and (2) those hooks call save-match-data.
>>
>> I care unless there is an easy way to identify which buffer modification
>> hooks are responsible so I can take steps as a user to mitigate the
>> problems.
>
> Any hook in before-change-functions or after-change-functions that
> calls save-match-data.
>
> If we care about the performance hit, we need to come up with a
> different solution for this problem (or measure the performance hit
> and convince ourselves it is not a big deal).

Thanks for the hint. So in my case it was flycheck-handle-change which
was triggering the problem:

(defun flycheck-handle-change (beg end _len)
  "Handle a buffer change between BEG and END.

BEG and END mark the beginning and end of the change text.  _LEN
is ignored.

Start a syntax check if a new line has been inserted into the
buffer."
  ;; Save and restore the match data, as recommended in (elisp)Change Hooks
  (save-match-data
(when flycheck-mode
  ;; The buffer was changed, thus clear the idle timer
  (flycheck-clear-idle-change-timer)
  (if (string-match-p (rx "\n") (buffer-substring beg end))
  (flycheck-buffer-automatically 'new-line 'force-deferred)
(setq flycheck-idle-change-timer
  (run-at-time flycheck-idle-change-delay nil
   #'flycheck-handle-idle-change))

However it doesn't look as though it tweaks the buffer until idle timer
has run. Weird

--
Alex Bennée





[O] bug#23917: 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-19 Thread Eli Zaretskii
> From: Alex Bennée 
> Cc: Stefan Monnier , 23...@debbugs.gnu.org, 
> rpl...@gmail.com, jwieg...@gmail.com, nljlistb...@gmail.com
> Date: Tue, 19 Jul 2016 18:05:37 +0100
> 
> > Do we care that using save-match-data in every call to replace-match
> > might mean a performance hit?  If it will, then this will again punish
> > most of the users for the benefit of those few who (1) have
> > buffer-modification hooks, and (2) those hooks call save-match-data.
> 
> I care unless there is an easy way to identify which buffer modification
> hooks are responsible so I can take steps as a user to mitigate the
> problems.

Any hook in before-change-functions or after-change-functions that
calls save-match-data.

If we care about the performance hit, we need to come up with a
different solution for this problem (or measure the performance hit
and convince ourselves it is not a big deal).





[O] bug#23917: 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-19 Thread Alex Bennée

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 12:03:51 -0400
>>
>> I guess the next best thing is:
>> - copy search_regs.start and search_regs.end before calling replace_range.
>> - use that copy when adjusting the match data.
>> Or equivalently, use save-match-data.  IOW go back to your original patch.
>> Duh!
>
> Do we care that using save-match-data in every call to replace-match
> might mean a performance hit?  If it will, then this will again punish
> most of the users for the benefit of those few who (1) have
> buffer-modification hooks, and (2) those hooks call save-match-data.

I care unless there is an easy way to identify which buffer modification
hooks are responsible so I can take steps as a user to mitigate the
problems.

--
Alex Bennée