From mboxrd@z Thu Jan 1 00:00:00 1970 Path: main.gmane.org!not-for-mail From: Karl Chen Newsgroups: gmane.emacs.devel Subject: Re: [cvs] bug when using pc-selection-mode/transient-mark-mode Date: Fri, 20 Sep 2002 19:38:12 -0700 (PDT) Sender: emacs-devel-admin@gnu.org Message-ID: References: Reply-To: q.edg875310@quarl.org NNTP-Posting-Host: localhost.gmane.org Mime-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Trace: main.gmane.org 1032605418 14932 127.0.0.1 (21 Sep 2002 10:50:18 GMT) X-Complaints-To: usenet@main.gmane.org NNTP-Posting-Date: Sat, 21 Sep 2002 10:50:18 +0000 (UTC) Cc: emacs-devel@gnu.org Return-path: Original-Received: from quimby.gnus.org ([80.91.224.244]) by main.gmane.org with esmtp (Exim 3.35 #1 (Debian)) id 17shpx-0003sL-00 for ; Sat, 21 Sep 2002 12:50:09 +0200 Original-Received: from monty-python.gnu.org ([199.232.76.173]) by quimby.gnus.org with esmtp (Exim 3.12 #1 (Debian)) id 17siVU-0001z4-00 for ; Sat, 21 Sep 2002 13:33:15 +0200 Original-Received: from localhost ([127.0.0.1] helo=monty-python.gnu.org) by monty-python.gnu.org with esmtp (Exim 4.10) id 17shXa-0006Ft-00; Sat, 21 Sep 2002 06:31:10 -0400 Original-Received: from list by monty-python.gnu.org with tmda-scanned (Exim 4.10) id 17sa9p-0003iD-00 for emacs-devel@gnu.org; Fri, 20 Sep 2002 22:38:09 -0400 Original-Received: from mail by monty-python.gnu.org with spam-scanned (Exim 4.10) id 17sa9m-0003i1-00 for emacs-devel@gnu.org; Fri, 20 Sep 2002 22:38:08 -0400 Original-Received: from hkn.eecs.berkeley.edu ([128.32.47.228]) by monty-python.gnu.org with esmtp (Exim 4.10) id 17sa9k-0003hj-00; Fri, 20 Sep 2002 22:38:04 -0400 Original-Received: from quarl (helo=localhost) by hkn.eecs.berkeley.edu with local-esmtp id 17sa9s-0001ol-00; Fri, 20 Sep 2002 19:38:12 -0700 Original-To: Richard Stallman In-Reply-To: Errors-To: emacs-devel-admin@gnu.org X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.0.11 Precedence: bulk List-Help: List-Post: List-Subscribe: , List-Id: Emacs development discussions. List-Unsubscribe: , List-Archive: Xref: main.gmane.org gmane.emacs.devel:8073 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:8073 > In the C code there are just two places that deactivate the mark: > > editfns.c:802: current_buffer->mark_active = tem; > keyboard.c:1755: current_buffer->mark_active = Qnil; > > Once you are at that point, could you try enabling breakpoints at > those two places and see if they go off? It wasn't easy because the part in keyboard.c:1755 gets called whenever deactivate-mark is non-nil, which is set in multiple places. The ultimate source of deactivate-mark getting set is insdel.c:1967 prepare_to_modify_buffer() { ... Vdeactivate_mark = Qt; // line 1967 } So the bug indeed has to do with x-own-selection and sit-for (non-nil post-command-idle-hook). Emacs will get X selection events and in sit_for (sec=0, ...) will call swallow_events() which will call x_handle_selection_request() which will call x_get_local_selection() which will go through selection-converter-alist and end up calling (xselect-convert-to-string ...) which will call (encode-coding-string ...) code_convert_string1() encode_coding_string() run_pre_post_conversion_on_str() <--- (erase-buffer /* buffer = "*code-converting-work*" */ ) del_range() del_range_1() prepare_to_modify_buffer() I'm giving you all this backtracing because I'm not sure where to fix the bug. I can comment out insdel.c:1967 and it will go away but that was probably there for a reason. My best guess is that when run_pre_post_conversion_on_str() calls buffer modification functions such as (erase-buffer) and insert_from_string() on its temporary buffer, it doesn't mean to set the global deactivate-mark. This patch fixes the problem. --- coding.c.~1.257.~ 2002-09-07 21:30:13.000000000 -0700 +++ coding.c 2002-09-20 19:19:31.000000000 -0700 @@ -5819,6 +5819,7 @@ int multibyte = STRING_MULTIBYTE (str); Lisp_Object buffer; struct buffer *buf; + Lisp_Object old_deactivate_mark = Vdeactivate_mark; record_unwind_protect (Fset_buffer, Fcurrent_buffer ()); record_unwind_protect (code_convert_region_unwind, Qnil); @@ -5854,6 +5855,9 @@ } inhibit_pre_post_conversion = 0; str = make_buffer_string (BEG, Z, 1); + + Vdeactivate_mark = old_deactivate_mark; + return unbind_to (count, str); } --- I haven't looked yet in cvs what change was responsible for the behavior manifesting. HOWEVER, this is an ugly hack and I've noticed a lot of similar "save deactivate_mark; do crap; restore deactivate_mark" code in Emacs. It just smells of bad design when global variables have to be pushed and popped all the time and someone calling a function needs to know which variables to push/pop, especially when this is not documented. Some ideas for a more elegant (i.e. a lot more troublesome to implement) solution: - per-buffer `deactivate-mark' variable. - maybe prepare_to_modify_buffer doesn't need to set deactivate-mark (probably does need to). - a "temporary buffer" status for buffers that are such. Could be either passed to get-buffer-create or set as a buffer-local variable. - notice whether the buffer being modified by in prepare_to_MB is the "active" one (current buffer? or any visible buffer) that the user has a selection in (and if not assume it's a temporary buffer) In the meantime there should be a documented warning to emacs developers that whenever they create and manipulate temporary buffers that they have to save the deactivate-mark variable. -- Karl Chen / quarl@quarl.org