BUG REPORT: "delsel.el"

BUG REPORT: "delsel.el"

[Le Wang]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

Hi,

I gave the `kill-line' symbol a property of 'kill for 'delete-selection.

After the region is killed, "delsel.el" attempts to deactivate the region, but 
of course in transient-mark-mode buffer modification deactivates the region 
automatically, leading to an error.  I've got a patch (see atached), I don't 
know if it's too simplistic, though.  I'm no lisp expert.

--
Le

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: delsel.el.diff --]
[-- Type: text/x-diff; name="delsel.el.diff", Size: 449 bytes --]

--- temp/delsel_old.el	2003-02-26 08:54:24.000000000 -0500
+++ c:/cygwin/usr/emacs-cvs/lisp/delsel.el	2003-02-26 08:55:08.000000000 -0500
@@ -75,8 +75,9 @@
   (if killp
       (kill-region (point) (mark))
     (delete-region (point) (mark)))
-  (setq mark-active nil)
-  (run-hooks 'deactivate-mark-hook)
+  (unless transient-mark-mode
+    (setq mark-active nil)
+    (run-hooks 'deactivate-mark-hook))
   t)
 
 (defun delete-selection-pre-hook ()

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: BUG REPORT: "delsel.el"

[Stefan Monnier]

> Hi,
> 
> I gave the `kill-line' symbol a property of 'kill for 'delete-selection.
> 
> After the region is killed, "delsel.el" attempts to deactivate the region, but 
> of course in transient-mark-mode buffer modification deactivates the region 
> automatically, leading to an error.

What error do you get ?

> I've got a patch (see atached), I don't 
> know if it's too simplistic, though.  I'm no lisp expert.

I don't understand the actual problem (I need the answer to the above
question for that), but a better patch along the lines of what you
sent is the one attached.


	Stefan


--- delsel.el.~1.29.~	Mon Nov 19 01:36:36 2001
+++ delsel.el	Sun Mar  2 15:31:34 2003
@@ -75,8 +75,7 @@
   (if killp
       (kill-region (point) (mark))
     (delete-region (point) (mark)))
-  (setq mark-active nil)
-  (run-hooks 'deactivate-mark-hook)
+  (deactivate-mark)
   t)
 
 (defun delete-selection-pre-hook ()

Re: BUG REPORT: "delsel.el"

[Le Wang]

Stefan Monnier wrote:

>> Hi,
>> 
>> I gave the `kill-line' symbol a property of 'kill for 'delete-selection.
>> 
>> After the region is killed, "delsel.el" attempts to deactivate the region,
>> but of course in transient-mark-mode buffer modification deactivates the
>> region automatically, leading to an error.
> 
> What error do you get ?

1. emacs -q

2. <M-x> transient-mark-mode

3. <M-x> delete-selection-mode

4. insert into scratch, and evaluate:

(put 'kill-line 'delete-selection 'kill)

5. Error: the mark is not active now.


>> I've got a patch (see atached), I don't
>> know if it's too simplistic, though.  I'm no lisp expert.
> 
> I don't understand the actual problem (I need the answer to the above
> question for that), but a better patch along the lines of what you
> sent is the one attached.

The problem is (I believe) that buffer modification already causes the mark to 
deactivate in transient-mark-mode.  So mark shouldn't be deactivated at all.  
As such, your patch doesn't address this specific problem.  But it is better 
coding style to deactivate the mark through the API.

Actually, I just tried to use delete-selection mode w/o transient-mark-mode, 
and it doesnt work at all.  Maybe this is best?

cd /usr/local/share/emacs/21.3.50/lisp/
diff -c /usr/local/share/emacs/21.3.50/lisp/delsel.el 
/tmp/buffer-content-278329Vo
*** /usr/local/share/emacs/21.3.50/lisp/delsel.el       Mon Nov 19 01:21:11 2001
--- /tmp/buffer-content-278329Vo        Sun Mar  2 17:28:13 2003
***************
*** 75,82 ****
    (if killp
        (kill-region (point) (mark))
      (delete-region (point) (mark)))
-   (setq mark-active nil)
-   (run-hooks 'deactivate-mark-hook)
    t)
  
  (defun delete-selection-pre-hook ()
--- 75,80 ----

Diff finished at Sun Mar  2 17:28:13


--
Le

Re: BUG REPORT: "delsel.el"

[Richard Stallman]

    +  (unless transient-mark-mode
    +    (setq mark-active nil)
    +    (run-hooks 'deactivate-mark-hook))

To quote the doc string of delete-selection-mode:

    When Delete Selection mode is enabled, Transient Mark mode is also
    enabled and typed text replaces the selection if the selection is
    active.

So putting (unless transient-mark-mode ...) around that code in
delete-active-region is equivalent to deleting it.  That can't be
right.

I do not actually use delsel mode.
Does this replacement function work right?

(defun delete-active-region (&optional killp)
  (if killp
      (kill-region (point) (mark))
    (delete-region (point) (mark)))
  (when mark-active
    (setq mark-active nil)
    (run-hooks 'deactivate-mark-hook))
  t)

Re: BUG REPORT: "delsel.el"

[Le Wang]

Richard Stallman wrote:

>     +  (unless transient-mark-mode
>     +    (setq mark-active nil)
>     +    (run-hooks 'deactivate-mark-hook))
> 
> To quote the doc string of delete-selection-mode:
> 
>     When Delete Selection mode is enabled, Transient Mark mode is also
>     enabled and typed text replaces the selection if the selection is
>     active.
> 
> So putting (unless transient-mark-mode ...) around that code in
> delete-active-region is equivalent to deleting it.  That can't be
> right.

It's not right.  Quoting the doc string from transient-mark-mode:

,----[ C-h f transient-mark-mode RET ]
|
| [...]  
| 
| In Transient Mark mode, when the mark is active, the region is highlighted.
| Changing the buffer "deactivates" the mark.
|
| [...]
| 
`----

So `delete-active-region' by virtue of `transient-mark-mode' already 
deactivates the region.  And as I mentioned in another post in this thread, 
this patch is appropriate:

*** /usr/local/share/emacs/21.3.50/lisp/delsel.el       Mon Nov 19 01:21:11 
2001
--- /tmp/buffer-content-278329Vo        Sun Mar  2 17:28:13 2003
***************
*** 75,82 ****
    (if killp
        (kill-region (point) (mark))
      (delete-region (point) (mark)))
-   (setq mark-active nil)
-   (run-hooks 'deactivate-mark-hook)
    t)
  
  (defun delete-selection-pre-hook ()
--- 75,80 ----

Diff finished at Sun Mar  2 17:28:13


--
Le

Re: BUG REPORT: "delsel.el"

[Richard Stallman]

    I don't understand the actual problem (I need the answer to the above
    question for that), but a better patch along the lines of what you
    sent is the one attached.

Your patch seems to be a good one.

Re: BUG REPORT: "delsel.el"

[Le Wang]

 --- Richard Stallman <rms@gnu.org> wrote: 

>     I don't understand the actual problem (I need the answer to the above
>     question for that), but a better patch along the lines of what you
>     sent is the one attached.
> 
> Your patch seems to be a good one. 

Do you mean this patch?

--- delsel.el.~1.29.~	Mon Nov 19 01:36:36 2001
+++ delsel.el	Sun Mar  2 15:31:34 2003
@@ -75,8 +75,7 @@
   (if killp
       (kill-region (point) (mark))
     (delete-region (point) (mark)))
-  (setq mark-active nil)
-  (run-hooks 'deactivate-mark-hook)
+  (deactivate-mark)
   t)
 
 (defun delete-selection-pre-hook ()

It doesn't fix the problem.  I still get "Mark is not active now" error.  If
we don't touch the mark at all, transient-mark-mode takes care of everything:

*** /usr/local/share/emacs/21.3.50/lisp/delsel.el       Mon Nov 19 01:21:11 
2001
--- /tmp/buffer-content-278329Vo        Sun Mar  2 17:28:13 2003
***************
*** 75,82 ****
    (if killp
        (kill-region (point) (mark))
      (delete-region (point) (mark)))
-   (setq mark-active nil)
-   (run-hooks 'deactivate-mark-hook)
    t)
  
  (defun delete-selection-pre-hook ()
--- 75,80 ----

Diff finished at Sun Mar  2 17:28:13

--
Le

______________________________________________________________________ 
Post your free ad now! http://personals.yahoo.ca

Re: BUG REPORT: "delsel.el"

[Richard Stallman]

    So `delete-active-region' by virtue of `transient-mark-mode' already 
    deactivates the region.  And as I mentioned in another post in this thread, 
    this patch is appropriate:

Maybe you are right.  If both kill-region and delete-region deactivate
the mark, then there is no need for delete-active-region to do so
explicitly.