bug#4136: 23.1; delete-pair

bug#4136: 23.1; delete-pair

[Eli Barzilay]

`delete-pair' is deleting what it documents -- instead of removing the
open paren of the following sexp, it deletes the current one.  So, if
the cursor is on some whitespace that precedes an expression, the
whitespace is deleted, and the open paren is left intact.



In GNU Emacs 23.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.10.14)
 of 2009-08-01 on winooski.ccs.neu.edu
Windowing system distributor `The X.Org Foundation', version 11.0.10300000
configured using `configure  '--prefix=/home/eli/bin/local/emacs-dir''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: POSIX
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US
  value of $XMODIFIERS: nil
  locale-coding-system: iso-latin-1-unix
  default-enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  cua-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  line-number-mode: t
  transient-mark-mode: t

bug#4136: 23.1; delete-pair

[martin rudalics]

 > `delete-pair' is deleting what it documents -- instead of removing the
 > open paren of the following sexp, it deletes the current one.  So, if
 > the cursor is on some whitespace that precedes an expression, the
 > whitespace is deleted, and the open paren is left intact.

`delete-pair' shouldn't delete anything but matching elements of
`insert-pair-alist'.

martin

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> `delete-pair' is deleting what it documents -- instead of removing the
>> open paren of the following sexp, it deletes the current one.  So, if
>> the cursor is on some whitespace that precedes an expression, the
>> whitespace is deleted, and the open paren is left intact.
>
> `delete-pair' shouldn't delete anything but matching elements of
> `insert-pair-alist'.

This is rather vague semantics.  I suggest to keep the current simple
semantics and just to fix it with the patch I sent.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4136: 23.1; delete-pair

[Eli Barzilay]

On Aug 13, martin rudalics wrote:
>  > `delete-pair' is deleting what it documents -- instead of removing the
>  > open paren of the following sexp, it deletes the current one.  So, if
>  > the cursor is on some whitespace that precedes an expression, the
>  > whitespace is deleted, and the open paren is left intact.
> 
> `delete-pair' shouldn't delete anything but matching elements of
> `insert-pair-alist'.

This is a very good point.


On Aug 14, Juri Linkov wrote:
> >> `delete-pair' is deleting what it documents -- instead of removing the
> >> open paren of the following sexp, it deletes the current one.  So, if
> >> the cursor is on some whitespace that precedes an expression, the
> >> whitespace is deleted, and the open paren is left intact.
> >
> > `delete-pair' shouldn't delete anything but matching elements of
> > `insert-pair-alist'.
> 
> This is rather vague semantics.  I suggest to keep the current simple
> semantics and just to fix it with the patch I sent.

I disagree in two different ways: (a) it can be put into code, so it's
not vague at all; (b) it sounds like a very good feature -- as it
stands, this function is nearly useless without human supervision,
which makes it useless for keyboard macros.

Here is a version that checks that does exactly what Martin suggested.
It even fails on something like (...] -- which is very useful for
keyboard macros, since if you have that text in a buffer, then
something is probably messed up enough to require your attention.


(defun delete-pair ()
  "Delete a pair of characters enclosing the sexp that follows point."
  (interactive)
  (save-excursion
    (let* ((end      (progn (forward-sexp  1) (point)))
           (start    (progn (forward-sexp -1) (point)))
           (start-ch (char-after start))
           (end-ch   (char-before end))
           (match-p  nil))
      (let ((p insert-pair-alist))
        (while p
          (let* ((q (car p))
                 (fst (if (= (length q) 3) 1 0))
                 (snd (1+ fst)))
            (if (and (equal start-ch (nth fst q))
                     (equal end-ch   (nth snd q)))
              (setq p nil match-p t)
              (setq p (cdr p))))))
      (if match-p
        (progn (delete-region (1- end) end)
               (delete-region start (1+ start)))
        (error "not a matching pair")))))


-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

bug#4136: 23.1; delete-pair

[Stefan Monnier]

> Here is a version that checks that does exactly what Martin suggested.
> It even fails on something like (...] -- which is very useful for
> keyboard macros, since if you have that text in a buffer, then
> something is probably messed up enough to require your attention.

There's always a tension in such cases.  E.g. for interactive use, I'd
rather we allow such (..] cases, since it may occasionally come in handy
(whereas the beep will come as a major annoyance, where Emacs will
appear to the user as being dogmatic).


        Stefan

bug#4136: 23.1; delete-pair

[Juri Linkov]

> Here is a version that checks that does exactly what Martin suggested.
> It even fails on something like (...] -- which is very useful for
> keyboard macros, since if you have that text in a buffer, then
> something is probably messed up enough to require your attention.

This command is useful even if it doesn't fail on (...] because
forward-sexp and backward-sexp don't fail on (...].

It seems you and Martin prefer checking against the `insert-pair-alist'
because the function name `delete-pair' suggests it should be
a counterpart of `insert-pair'.  In this case I think we sould have
two commands: one more strict version `delete-pair' that checks
`insert-pair-alist', and another loose version with the name e.g.
`delete-parens' with the body of the current `delete-pair'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4136: 23.1; delete-pair

[Eli Barzilay]

On Aug 15, Juri Linkov wrote:
> > Here is a version that checks that does exactly what Martin
> > suggested.  It even fails on something like (...] -- which is very
> > useful for keyboard macros, since if you have that text in a
> > buffer, then something is probably messed up enough to require
> > your attention.
> 
> This command is useful even if it doesn't fail on (...] because
> forward-sexp and backward-sexp don't fail on (...].

Martin's example of '(blah) works better, but in general...

> It seems you and Martin prefer checking against the
> `insert-pair-alist' because the function name `delete-pair' suggests
> it should be a counterpart of `insert-pair'.

...this is exactly the issue: it is much better if `delete-foo' is
always an operation that reverts what `insert-foo' does.


> In this case I think we sould have two commands: one more strict
> version `delete-pair' that checks `insert-pair-alist', and another
> loose version with the name e.g.  `delete-parens' with the body of
> the current `delete-pair'.

The current state of `delete-pair' is so bad that my guess is that
hardly anyone used it, so adding another command doesn't make much
sense.  How about making it do the proper thing (removing only
balanced pairs as specified by `insert-pair-alist'), and ignoring
errors with a prefix argument?


On Aug 15, Juri Linkov wrote:
> >> This is rather vague semantics.  I suggest to keep the current
> >> simple semantics and just to fix it with the patch I sent.
> >
> > Before a quoted object your fixed version deletes the last
> > character of the object and the first quote character.  Handling
> > quote or prefix syntax characters via (delete-char 1) doesn't
> > strike me as very clean.
> 
> This is fixed in the following version:
> [...]

This version doesn't make much sense as an operation you'd want to do
on code:

  (foo '(x y z))
  -->
  (foo 'x y z)

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> It seems you and Martin prefer checking against the
>> `insert-pair-alist' because the function name `delete-pair' suggests
>> it should be a counterpart of `insert-pair'.
>
> ...this is exactly the issue: it is much better if `delete-foo' is
> always an operation that reverts what `insert-foo' does.

The exact reverse is impossible.  For instance, put the cursor
on the letter `f' in:

    foo "" bar

and type `M-3 M-"' where M-" is bound to `insert-pair'.
The result will be:

    "foo "" bar"

Now what `delete-pair' should do to revert this back to the original?

> The current state of `delete-pair' is so bad that my guess is that
> hardly anyone used it, so adding another command doesn't make much
> sense.  How about making it do the proper thing (removing only
> balanced pairs as specified by `insert-pair-alist'), and ignoring
> errors with a prefix argument?

I've been using `delete-pair' for many years several times a day
without any problem because my lists are always correctly balanced
thanks to `insert-pair' that I exclusively use to create balanced
lists and strings.

That's why even in the current state of `delete-pair' it is the useful
reverse of `insert-pair' because the latter creates balanced lists
and the former deletes them.

>> This is fixed in the following version:
>> [...]
>
> This version doesn't make much sense as an operation you'd want to do
> on code:
>
>   (foo '(x y z))
>   -->
>   (foo 'x y z)

It makes sense when `foo' is a multi-argument function like `list', e.g.

    (list 'x y z)

So I see no reason to introduce more restrictions to decide what
parens the user is allowed to delete in his/her code.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4136: 23.1; delete-pair

[Eli Barzilay]

On Aug 16, Juri Linkov wrote:
> >> It seems you and Martin prefer checking against the
> >> `insert-pair-alist' because the function name `delete-pair'
> >> suggests it should be a counterpart of `insert-pair'.
> >
> > ...this is exactly the issue: it is much better if `delete-foo' is
> > always an operation that reverts what `insert-foo' does.
> 
> The exact reverse is impossible.

Of course it isn't, but the operation is one that negates in what it
"generally does" the thing that `insert-foo' does.  Just like
`scroll-down' and `scroll-up' are opposites even though they don't
always cancel out.


> > The current state of `delete-pair' is so bad that my guess is that
> > hardly anyone used it, so adding another command doesn't make much
> > sense.  How about making it do the proper thing (removing only
> > balanced pairs as specified by `insert-pair-alist'), and ignoring
> > errors with a prefix argument?
> 
> I've been using `delete-pair' for many years several times a day
> without any problem because my lists are always correctly balanced
> thanks to `insert-pair' that I exclusively use to create balanced
> lists and strings.

Believe me, I've written a paren or two, and I use `insert-pair'
enough to have my own improved version of it.  (In fact, guess why I
almost never use smileys in emails.)  Still, the first time I tried
this, the cursor was at the beginning of an indented line.  But the
point is still the same: having two nearly identical functions is
exactly what prefix arguments are for.


> That's why even in the current state of `delete-pair' it is the
> useful reverse of `insert-pair' because the latter creates balanced
> lists and the former deletes them.

... unless you happen to have your cursor on a non-paren.


> >> This is fixed in the following version:
> >> [...]
> >
> > This version doesn't make much sense as an operation you'd want to do
> > on code:
> >
> >   (foo '(x y z))
> >   -->
> >   (foo 'x y z)
> 
> It makes sense when `foo' is a multi-argument function like `list',
> e.g.
> 
>     (list 'x y z)
> 
> So I see no reason to introduce more restrictions to decide what
> parens the user is allowed to delete in his/her code.

You've missed my point: the difference between "y" and "'y" is *huge*,
changing one to the other is something that you don't want to do by
mistake.


On Aug 16, Juri Linkov wrote:
> >> This is fixed in the following version:
> >>
> >> (defun delete-pair ()
> >>   "Delete a pair of characters enclosing the sexp that follows point."
> >>   (interactive)
> >>   (save-excursion
> >>     (forward-sexp 1)
> >>     (save-excursion
> >>       (backward-sexp 1)
> >>       (skip-syntax-forward "'")
> >>       (delete-char 1))
> >>     (delete-char -1)))
> >
> > Still doesn't seem TDTRT with `point' before something like
> >
> >        `foo'
> 
> I know, I know, after I fix this, you'll come up with another
> test case like
> 
>          `foo bar'

Those examples are very good IMO -- it's not being picky for nothing,
it's an attempt to avoid nasty surprises that make you end up with
erroneous code.  Emacs is usually good at being a careful editor for
code, `delete-pair' is very exceptional in this aspect.

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

bug#4136: 23.1; delete-pair

[Juri Linkov]

> Believe me, I've written a paren or two, and I use `insert-pair'
> enough to have my own improved version of it.  (In fact, guess why I
> almost never use smileys in emails.)

You can write smileys using the closing parenthesis (but we should
improve `delete-pair' to delete smileys completely after deleting the
closing parenthesis ;-)

> Still, the first time I tried this, the cursor was at the beginning of
> an indented line.

Where did you get this weird idea that `delete-pair' should delete
a character far away from the current cursor position?  Look,
`insert-char' inserts a character at point and `delete-char' deletes
a character at point.  `insert-pair' inserts an opening character
at point.  So why `delete-pair' shouldn't do the same?  Why it shouldn't
delete an opening character at point instead of using some additional
heuristics to find the position of the opening character?

>> That's why even in the current state of `delete-pair' it is the
>> useful reverse of `insert-pair' because the latter creates balanced
>> lists and the former deletes them.
>
> ... unless you happen to have your cursor on a non-paren.

Why would I want to put cursor on a non-paren when I want to delete a paren?

>> > This version doesn't make much sense as an operation you'd want to do
>> > on code:
>> >
>> >   (foo '(x y z))
>> >   -->
>> >   (foo 'x y z)
>>
>> It makes sense when `foo' is a multi-argument function like `list',
>> e.g.
>>
>>     (list 'x y z)
>>
>> So I see no reason to introduce more restrictions to decide what
>> parens the user is allowed to delete in his/her code.
>
> You've missed my point: the difference between "y" and "'y" is *huge*,
> changing one to the other is something that you don't want to do by
> mistake.

99% of time when you write a program it is in the erroneous state until
you finish editing.  But this doesn't mean we should disallow the user
to have an intermediate erroneous state.  So of course, in the above
example you can add necessary quotes after deleting a pair of parentheses.

>> I know, I know, after I fix this, you'll come up with another
>> test case like
>>
>>          `foo bar'
>
> Those examples are very good IMO -- it's not being picky for nothing,
> it's an attempt to avoid nasty surprises that make you end up with
> erroneous code.  Emacs is usually good at being a careful editor for
> code, `delete-pair' is very exceptional in this aspect.

Emacs is good at following the KISS principle to not over-engineer
simple functions.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4136: 23.1; delete-pair

[Eli Barzilay]

On Aug 17, Juri Linkov wrote:
> > Still, the first time I tried this, the cursor was at the
> > beginning of an indented line.
> 
> Where did you get this weird idea that `delete-pair' should delete a
> character far away from the current cursor position?  Look,
> `insert-char' inserts a character at point and `delete-char' deletes
> a character at point.

Only when there is a character there.


> `insert-pair' inserts an opening character at point.  So why
> `delete-pair' shouldn't do the same?  Why it shouldn't delete an
> opening character at point instead of using some additional
> heuristics to find the position of the opening character?

We're going back in circles here: "delete *pair*" shouldn't delete an
opening character unless it is part of a *pair*.  If by the above you
mean make it delete the character pair starting at the current point,
then that seems fine -- as long as it verifies that there really is a
pair.


> >> That's why even in the current state of `delete-pair' it is the
> >> useful reverse of `insert-pair' because the latter creates
> >> balanced lists and the former deletes them.
> >
> > ... unless you happen to have your cursor on a non-paren.
> 
> Why would I want to put cursor on a non-paren when I want to delete
> a paren?

You wouldn't, which is why throwing an error is the right thing.  If
you apply this exact same argument to `delete-char':

  Why would I want to put the cursor at the end of the buffer when I
  want to delete a character?

it can justify making `delete-char' delete the preceding character
when the cursor is at the eob.


> >> > This version doesn't make much sense as an operation you'd want to do
> >> > on code:
> >> >
> >> >   (foo '(x y z))
> >> >   -->
> >> >   (foo 'x y z)
> >>
> >> It makes sense when `foo' is a multi-argument function like `list',
> >> e.g.
> >>
> >>     (list 'x y z)
> >>
> >> So I see no reason to introduce more restrictions to decide what
> >> parens the user is allowed to delete in his/her code.
> >
> > You've missed my point: the difference between "y" and "'y" is *huge*,
> > changing one to the other is something that you don't want to do by
> > mistake.
> 
> 99% of time when you write a program it is in the erroneous state
> until you finish editing.  But this doesn't mean we should disallow
> the user to have an intermediate erroneous state.  So of course, in
> the above example you can add necessary quotes after deleting a pair
> of parentheses.

That makes absolutely no sense.  Yes, a file is usually at a bad state
while it's being edited -- but that's not a reason to make things
worse by an operation that hardly ever does what you (some random
hacker) want to do.  (And you don't want to do that because 'foo is
unrelated to foo.)


> >> I know, I know, after I fix this, you'll come up with another
> >> test case like
> >>
> >>          `foo bar'
> >
> > Those examples are very good IMO -- it's not being picky for
> > nothing, it's an attempt to avoid nasty surprises that make you
> > end up with erroneous code.  Emacs is usually good at being a
> > careful editor for code, `delete-pair' is very exceptional in this
> > aspect.
> 
> Emacs is good at following the KISS principle to not over-engineer
> simple functions.

Are you serious??  In v23 I'm counting about 500 lines of just elisp
code for next/previous-line.  It is the resulting behavior that should
be simple -- and that sometimes requires considerable amount of code.
Making `delete-pair' does what its name suggest *is* keeping it
simple.

The current version of the function is pretty much swapping the last
two characters in "KISS".

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> `insert-pair' inserts an opening character at point.  So why
>> `delete-pair' shouldn't do the same?  Why it shouldn't delete an
>> opening character at point instead of using some additional
>> heuristics to find the position of the opening character?
>
> We're going back in circles here: "delete *pair*" shouldn't delete an
> opening character unless it is part of a *pair*.  If by the above you
> mean make it delete the character pair starting at the current point,
> then that seems fine -- as long as it verifies that there really is a
> pair.

We are in violent agreement.  I didn't claim that `delete-pair'
shouldn't check `insert-pair-alist'.  On the contrary, I think
`delete-pair' should verify if the character pair starting at the
current point is part of a pair according to `insert-pair-alist'.
"At the current point" - that's my point.  It shouldn't try
finding the opening character somewhere else.  So in your original
test case it should throw an error when the cursor is on some
whitespace that precedes an expression.

>> Why would I want to put cursor on a non-paren when I want to delete
>> a paren?
>
> You wouldn't, which is why throwing an error is the right thing.

I agree.  So I'd like to close this issue with the following patch:

Index: lisp/emacs-lisp/lisp.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/emacs-lisp/lisp.el,v
retrieving revision 1.102
diff -u -r1.102 lisp.el
--- lisp/emacs-lisp/lisp.el	22 Jul 2009 02:45:37 -0000	1.102
+++ lisp/emacs-lisp/lisp.el	17 Aug 2009 21:16:32 -0000
@@ -530,7 +530,15 @@
 (defun delete-pair ()
   "Delete a pair of characters enclosing the sexp that follows point."
   (interactive)
-  (save-excursion (forward-sexp 1) (delete-char -1))
+  (let ((open-char (char-after)))
+    (save-excursion
+      (forward-sexp 1)
+      (unless (member (list open-char (char-before))
+		      (mapcar (lambda (p)
+				(if (= (length p) 3) (cdr p) p))
+			      insert-pair-alist))
+	(error "Not a matching pair"))
+      (delete-char -1)))
   (delete-char 1))
 
 (defun raise-sexp (&optional arg)

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4136: 23.1; delete-pair

[martin rudalics]

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

 > We are in violent agreement.  I didn't claim that `delete-pair'
 > shouldn't check `insert-pair-alist'.  On the contrary, I think
 > `delete-pair' should verify if the character pair starting at the
 > current point is part of a pair according to `insert-pair-alist'.
 > "At the current point" - that's my point.  It shouldn't try
 > finding the opening character somewhere else.  So in your original
 > test case it should throw an error when the cursor is on some
 > whitespace that precedes an expression.

Ne hlebom jedinnym.  Why can't we be generous and provide something like
the attached?

martin

[-- Attachment #2: lisp.el.diff --]
[-- Type: text/plain, Size: 1719 bytes --]

*** emacs-lisp/lisp.el.~1.102.~	2009-07-27 08:10:54.124527300 +0200
--- emacs-lisp/lisp.el	2009-08-18 08:52:42.734375000 +0200
***************
*** 527,537 ****
    (interactive "P")
    (insert-pair arg ?\( ?\)))
  
! (defun delete-pair ()
!   "Delete a pair of characters enclosing the sexp that follows point."
!   (interactive)
!   (save-excursion (forward-sexp 1) (delete-char -1))
!   (delete-char 1))
  
  (defun raise-sexp (&optional arg)
    "Raise ARG sexps higher up the tree."
--- 527,566 ----
    (interactive "P")
    (insert-pair arg ?\( ?\)))
  
! (defun delete-pair (&optional arg)
!   "Delete a pair of characters enclosing ARG sexps that follow point.
! A negative ARG deletes a pair around the preceding ARG sexps instead."
!   (interactive "P")
! 
!   (if arg
!       (setq arg (prefix-numeric-value arg))
!     (setq arg 1))
! 
!   (if (< arg 0)
!       (save-excursion
! 	(skip-chars-backward " \t")
! 	(save-excursion
! 	  (let ((close-char (char-before)))
! 	    (forward-sexp arg)
! 	    (unless (member (list (char-after) close-char)
! 			    (mapcar (lambda (p)
! 				      (if (= (length p) 3) (cdr p) p))
! 				    insert-pair-alist))
! 	      (error "Not after matching pair"))
! 	    (delete-char 1)))
! 	(delete-char -1))
!     (save-excursion
!       (skip-chars-forward " \t")
!       (save-excursion
! 	(let ((open-char (char-after)))
! 	  (forward-sexp arg)
! 	  (unless (member (list open-char (char-before))
! 			  (mapcar (lambda (p)
! 				    (if (= (length p) 3) (cdr p) p))
! 				  insert-pair-alist))
! 	    (error "Not before matching pair"))
! 	  (delete-char -1)))
!       (delete-char 1))))
  
  (defun raise-sexp (&optional arg)
    "Raise ARG sexps higher up the tree."

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> We are in violent agreement.  I didn't claim that `delete-pair'
>> shouldn't check `insert-pair-alist'.  On the contrary, I think
>> `delete-pair' should verify if the character pair starting at the
>> current point is part of a pair according to `insert-pair-alist'.
>> "At the current point" - that's my point.  It shouldn't try
>> finding the opening character somewhere else.  So in your original
>> test case it should throw an error when the cursor is on some
>> whitespace that precedes an expression.
>
> Ne hlebom jedinnym.  Why can't we be generous and provide something like
> the attached?
>
> martin
> *** emacs-lisp/lisp.el.~1.102.~	2009-07-27 08:10:54.124527300 +0200
> --- emacs-lisp/lisp.el	2009-08-18 08:52:42.734375000 +0200
> ***************
> *** 527,537 ****
>     (interactive "P")
>     (insert-pair arg ?\( ?\)))
>   
> ! (defun delete-pair ()
> !   "Delete a pair of characters enclosing the sexp that follows point."
> !   (interactive)
> !   (save-excursion (forward-sexp 1) (delete-char -1))
> !   (delete-char 1))
>   
>   (defun raise-sexp (&optional arg)
>     "Raise ARG sexps higher up the tree."
> --- 527,566 ----
>     (interactive "P")
>     (insert-pair arg ?\( ?\)))
>   
> ! (defun delete-pair (&optional arg)
> !   "Delete a pair of characters enclosing ARG sexps that follow point.
> ! A negative ARG deletes a pair around the preceding ARG sexps instead."
> !   (interactive "P")
> ! 
> !   (if arg
> !       (setq arg (prefix-numeric-value arg))
> !     (setq arg 1))
> ! 
> !   (if (< arg 0)
> !       (save-excursion
> ! 	(skip-chars-backward " \t")
> ! 	(save-excursion
> ! 	  (let ((close-char (char-before)))
> ! 	    (forward-sexp arg)
> ! 	    (unless (member (list (char-after) close-char)
> ! 			    (mapcar (lambda (p)
> ! 				      (if (= (length p) 3) (cdr p) p))
> ! 				    insert-pair-alist))
> ! 	      (error "Not after matching pair"))
> ! 	    (delete-char 1)))
> ! 	(delete-char -1))
> !     (save-excursion
> !       (skip-chars-forward " \t")
> !       (save-excursion
> ! 	(let ((open-char (char-after)))
> ! 	  (forward-sexp arg)
> ! 	  (unless (member (list open-char (char-before))
> ! 			  (mapcar (lambda (p)
> ! 				    (if (= (length p) 3) (cdr p) p))
> ! 				  insert-pair-alist))
> ! 	    (error "Not before matching pair"))
> ! 	  (delete-char -1)))
> !       (delete-char 1))))
>   
>   (defun raise-sexp (&optional arg)
>     "Raise ARG sexps higher up the tree."

Hmm, I think this makes sense.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4136: 23.1; delete-pair

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> Ne hlebom jedinnym.  Why can't we be generous and provide something like
> the attached?

The patch no longer applied, so I've respun it for Emacs 28 (included
below).

The thread here ended with Martin and Juri both agreeing that this was a
good idea, but it was never applied?

I don't use delete-pair myself regularly, but giving it some testing
now, this new version seems to work fine.

Any comments?  (The patch is 11 years old.)

diff --git a/lisp/emacs-lisp/lisp.el b/lisp/emacs-lisp/lisp.el
index 8c18557c79..ac4ba78897 100644
--- a/lisp/emacs-lisp/lisp.el
+++ b/lisp/emacs-lisp/lisp.el
@@ -735,12 +735,37 @@ insert-parentheses
   (insert-pair arg ?\( ?\)))
 
 (defun delete-pair (&optional arg)
-  "Delete a pair of characters enclosing ARG sexps following point.
-A negative ARG deletes a pair of characters around preceding ARG sexps."
-  (interactive "p")
-  (unless arg (setq arg 1))
-  (save-excursion (forward-sexp arg) (delete-char (if (> arg 0) -1 1)))
-  (delete-char (if (> arg 0) 1 -1)))
+  "Delete a pair of characters enclosing ARG sexps that follow point.
+A negative ARG deletes a pair around the preceding ARG sexps instead."
+  (interactive "P")
+  (if arg
+      (setq arg (prefix-numeric-value arg))
+    (setq arg 1))
+  (if (< arg 0)
+      (save-excursion
+	(skip-chars-backward " \t")
+	(save-excursion
+	  (let ((close-char (char-before)))
+	    (forward-sexp arg)
+	    (unless (member (list (char-after) close-char)
+			    (mapcar (lambda (p)
+				      (if (= (length p) 3) (cdr p) p))
+				    insert-pair-alist))
+	      (error "Not after matching pair"))
+	    (delete-char 1)))
+	(delete-char -1))
+    (save-excursion
+      (skip-chars-forward " \t")
+      (save-excursion
+	(let ((open-char (char-after)))
+	  (forward-sexp arg)
+	  (unless (member (list open-char (char-before))
+			  (mapcar (lambda (p)
+				    (if (= (length p) 3) (cdr p) p))
+				  insert-pair-alist))
+	    (error "Not before matching pair"))
+	  (delete-char -1)))
+      (delete-char 1))))
 
 (defun raise-sexp (&optional arg)
   "Raise ARG sexps higher up the tree."


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#4136: 23.1; delete-pair

[Juri Linkov]

> I don't use delete-pair myself regularly, but giving it some testing
> now, this new version seems to work fine.
>
> Any comments?  (The patch is 11 years old.)

I don't know, I use the current version of delete-pair many times every day,
and happy with it.  If the patch doesn't break the current behavior, that
would be fine.

bug#4136: 23.1; delete-pair

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

>> I don't use delete-pair myself regularly, but giving it some testing
>> now, this new version seems to work fine.
>>
>> Any comments?  (The patch is 11 years old.)
>
> I don't know, I use the current version of delete-pair many times every day,
> and happy with it.  If the patch doesn't break the current behavior, that
> would be fine.

Could you test the patch and see whether there's any problems?  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#4136: 23.1; delete-pair

[Juri Linkov]

>>> I don't use delete-pair myself regularly, but giving it some testing
>>> now, this new version seems to work fine.
>>>
>>> Any comments?  (The patch is 11 years old.)
>>
>> I don't know, I use the current version of delete-pair many times every day,
>> and happy with it.  If the patch doesn't break the current behavior, that
>> would be fine.
>
> Could you test the patch and see whether there's any problems?  :-)

Now I tested that at least it doesn't cause regression.
Hopefully, its new features work equally well :-)

bug#4136: 23.1; delete-pair

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

> Now I tested that at least it doesn't cause regression.
> Hopefully, its new features work equally well :-)

Thanks for checking.

The new feature is just that it only deletes things that are part of
insert-pair-alist, if I understood Martin correctly...

I've now pushed it to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> Now I tested that at least it doesn't cause regression.
>> Hopefully, its new features work equally well :-)
>
> Thanks for checking.
>
> The new feature is just that it only deletes things that are part of
> insert-pair-alist, if I understood Martin correctly...
>
> I've now pushed it to Emacs 28.

I see that you reverted it now because it breaks on "foo" in text-mode.
But the previous version is worse: it deletes wrong text on "foo"
in text-mode.

bug#4136: 23.1; delete-pair

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

> I see that you reverted it now because it breaks on "foo" in text-mode.
> But the previous version is worse: it deletes wrong text on "foo"
> in text-mode.

Heh, I was misreading the tests -- I thought that the first one of these
was the one that failed, and not the second, kinda weird one:

(ert-deftest lisp-delete-pair-quotation-marks ()
  "Test \\[delete-pair] with quotation marks."
  (with-temp-buffer
    (insert "\"foo\"")
    (goto-char (point-min))
    (delete-pair)
    (should (string-equal "foo" (buffer-string)))))

(ert-deftest lisp-delete-pair-quotes-in-text-mode ()
  "Test \\[delete-pair] against string in Text Mode for #15014."
  (with-temp-buffer
    (text-mode)
    (insert "\"foo\"")
    (goto-char (point-min))
    (delete-pair)
    (should (string-equal "fo\"" (buffer-string)))))

So in text mode, delete-pair just deletes two characters, and that
doesn't seem helpful.  The patch I reverted made it bug out instead,
which seems better.

However, bug#15014 says:

> I'm not arguing here that we font lock double quoted strings (though
> I'm not convinced either). Without font locking, I can't think of a
> concrete case that yields the "pretty annoying behavior". What do you
> have in mind?
>
> I added
>
>   (modify-syntax-entry ?\" "$")
>
> to my configuration and find that forward-sexp and delete-pair seem to
> work without confusion, even in the presence of odd double quotes. It
> seems forward-sexp just scans forward from point.

And the patch wouldn't allow this, either, so I'm not sure this
behavioural change would be welcome -- the user would have to alter
insert-pair-alist in text mode, too.

So...

I think the current thing delete-pair does in text-mode is bad, so I'm
inclined to re-apply the patch, and announce this as an incompatible
change in NEWS (and explain how to get the behaviour described above
back).

Any opinions?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#4136: 23.1; delete-pair

[Juri Linkov]

> (ert-deftest lisp-delete-pair-quotes-in-text-mode ()
>   "Test \\[delete-pair] against string in Text Mode for #15014."
>   (with-temp-buffer
>     (text-mode)
>     (insert "\"foo\"")
>     (goto-char (point-min))
>     (delete-pair)
>     (should (string-equal "fo\"" (buffer-string)))))
>
> So in text mode, delete-pair just deletes two characters, and that
> doesn't seem helpful.  The patch I reverted made it bug out instead,
> which seems better.

Yes, it's better to bug out than to delete text instead of deleting quotes.

> I think the current thing delete-pair does in text-mode is bad, so I'm
> inclined to re-apply the patch, and announce this as an incompatible
> change in NEWS (and explain how to get the behaviour described above
> back).
>
> Any opinions?

If it's really impossible to delete a pair in text-mode, then better
to signal an error.

bug#4136: 23.1; delete-pair

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

> If it's really impossible to delete a pair in text-mode, then better
> to signal an error.

OK; I've reapplied the patch and fixed the test, and I'm now re-closing
this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> If it's really impossible to delete a pair in text-mode, then better
>> to signal an error.
>
> OK; I've reapplied the patch and fixed the test, and I'm now re-closing
> this bug report.

Thanks.  Every time I delete a pair of quotes in a long string,
or a pair of parens in a long sexp, it makes me feel uneasy since
I worry whether it deleted the intended characters or not.

How about using the same delay blink-matching-delay that is used
after showing a matching paren, but in this case to show
a character at the other end of the pair with a delay
before deleting it?

This is achievable with this patch:

diff --git a/lisp/emacs-lisp/lisp.el b/lisp/emacs-lisp/lisp.el
index ac4ba78897..88e1b26af3 100644
--- a/lisp/emacs-lisp/lisp.el
+++ b/lisp/emacs-lisp/lisp.el
@@ -752,6 +752,8 @@ delete-pair
 				      (if (= (length p) 3) (cdr p) p))
 				    insert-pair-alist))
 	      (error "Not after matching pair"))
+	    (when blink-matching-paren
+	      (sit-for blink-matching-delay))
 	    (delete-char 1)))
 	(delete-char -1))
     (save-excursion
@@ -764,6 +766,8 @@ delete-pair
 				    (if (= (length p) 3) (cdr p) p))
 				  insert-pair-alist))
 	    (error "Not before matching pair"))
+	  (when blink-matching-paren
+	    (sit-for blink-matching-delay))
 	  (delete-char -1)))
       (delete-char 1))))
 

bug#4136: 23.1; delete-pair

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

> How about using the same delay blink-matching-delay that is used
> after showing a matching paren, but in this case to show
> a character at the other end of the pair with a delay
> before deleting it?

Makes sense to me, but I'm not a regular user of delete-pair, so I don't
know whether that'd be annoying or reassuring...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#4136: 23.1; delete-pair

[Juri Linkov]

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

>> How about using the same delay blink-matching-delay that is used
>> after showing a matching paren, but in this case to show
>> a character at the other end of the pair with a delay
>> before deleting it?
>
> Makes sense to me, but I'm not a regular user of delete-pair, so I don't
> know whether that'd be annoying or reassuring...

After a month of using it the following patch proved to be useful.
It adds anew new defcustom delete-pair-blink-delay based on the default
value of blink-matching-delay.  Exactly the same defcustom is also
added copy-region-blink-delay for the feature request in bug#42865
not to blink when copying a region:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: blink-delay.patch --]
[-- Type: text/x-diff, Size: 3426 bytes --]

diff --git a/lisp/emacs-lisp/lisp.el b/lisp/emacs-lisp/lisp.el
index 35590123ee..be2adfa848 100644
--- a/lisp/emacs-lisp/lisp.el
+++ b/lisp/emacs-lisp/lisp.el
@@ -784,9 +784,17 @@ insert-parentheses
   (interactive "P")
   (insert-pair arg ?\( ?\)))
 
+(defcustom delete-pair-blink-delay blink-matching-delay
+  "Time in seconds to delay after showing a pair character to delete.
+No timeout in case of 0."
+  :type 'number
+  :group 'lisp
+  :version "28.1")
+
 (defun delete-pair (&optional arg)
   "Delete a pair of characters enclosing ARG sexps that follow point.
-A negative ARG deletes a pair around the preceding ARG sexps instead."
+A negative ARG deletes a pair around the preceding ARG sexps instead.
+The option `delete-pair-blink-delay' can disable blinking."
   (interactive "P")
   (if arg
       (setq arg (prefix-numeric-value arg))
@@ -802,6 +810,9 @@ delete-pair
 				      (if (= (length p) 3) (cdr p) p))
 				    insert-pair-alist))
 	      (error "Not after matching pair"))
+	    (when (and (natnump delete-pair-blink-delay)
+		       (> delete-pair-blink-delay 0))
+	      (sit-for delete-pair-blink-delay))
 	    (delete-char 1)))
 	(delete-char -1))
     (save-excursion
@@ -814,6 +825,9 @@ delete-pair
 				    (if (= (length p) 3) (cdr p) p))
 				  insert-pair-alist))
 	    (error "Not before matching pair"))
+	  (when (and (natnump delete-pair-blink-delay)
+		     (> delete-pair-blink-delay 0))
+	    (sit-for delete-pair-blink-delay))
 	  (delete-char -1)))
       (delete-char 1))))
 
diff --git a/lisp/simple.el b/lisp/simple.el
index e96c7c9a6e..3d18909f2c 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5089,9 +5089,9 @@ kill-ring-save
 
 (defun indicate-copied-region (&optional message-len)
   "Indicate that the region text has been copied interactively.
-If the mark is visible in the selected window, blink the cursor
-between point and mark if there is currently no active region
-highlighting.
+If the mark is visible in the selected window, blink the cursor between
+point and mark if there is currently no active region highlighting.
+The option `copy-region-blink-delay' can disable blinking.
 
 If the mark lies outside the selected window, display an
 informative message containing a sample of the copied text.  The
@@ -5106,11 +5106,13 @@ indicate-copied-region
 	;; Swap point-and-mark quickly so as to show the region that
 	;; was selected.  Don't do it if the region is highlighted.
 	(unless (and (region-active-p)
-		     (face-background 'region nil t))
+		     (face-background 'region nil t)
+		     (natnump copy-region-blink-delay)
+		     (> copy-region-blink-delay 0))
 	  ;; Swap point and mark.
 	  (set-marker (mark-marker) (point) (current-buffer))
 	  (goto-char mark)
-	  (sit-for blink-matching-delay)
+	  (sit-for copy-region-blink-delay)
 	  ;; Swap back.
 	  (set-marker (mark-marker) mark (current-buffer))
 	  (goto-char point)
@@ -8135,6 +8139,13 @@ blink-paren-post-self-insert-function
           ;; `sit-for'. That's also the reason it get a `priority' prop
           ;; of 100.
           'append)
+
+(defcustom copy-region-blink-delay blink-matching-delay
+  "Time in seconds to delay after showing a pair character to delete.
+No timeout in case of 0."
+  :type 'number
+  :group 'killing
+  :version "28.1")
 \f
 ;; This executes C-g typed while Emacs is waiting for a command.
 ;; Quitting out of a program does not go through here;

bug#4136: 23.1; delete-pair

[Basil L. Contovounesios]

Juri Linkov <juri@jurta.org> writes:

> @@ -802,6 +810,9 @@ delete-pair
>  				      (if (= (length p) 3) (cdr p) p))
>  				    insert-pair-alist))
>  	      (error "Not after matching pair"))
> +	    (when (and (natnump delete-pair-blink-delay)
> +		       (> delete-pair-blink-delay 0))
> +	      (sit-for delete-pair-blink-delay))

Why can't the delay be a float?

Thanks,

-- 
Basil

bug#4136: 23.1; delete-pair

[Juri Linkov]

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

>> @@ -802,6 +810,9 @@ delete-pair
>>  				      (if (= (length p) 3) (cdr p) p))
>>  				    insert-pair-alist))
>>  	      (error "Not after matching pair"))
>> +	    (when (and (natnump delete-pair-blink-delay)
>> +		       (> delete-pair-blink-delay 0))
>> +	      (sit-for delete-pair-blink-delay))
>
> Why can't the delay be a float?

My mistake was influenced by the lack of the function that
would check for a positive number (e.g. 'positive-number-p').
I tried to find such a function, then noticed that 'natnump'
at least checks for a non-negative number, but I forgot that
this applies only to integers.  So below is a fixed patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: blink-delay-2.patch --]
[-- Type: text/x-diff, Size: 3398 bytes --]

diff --git a/lisp/emacs-lisp/lisp.el b/lisp/emacs-lisp/lisp.el
index 35590123ee..dca132cc7f 100644
--- a/lisp/emacs-lisp/lisp.el
+++ b/lisp/emacs-lisp/lisp.el
@@ -784,9 +784,18 @@ insert-parentheses
   (interactive "P")
   (insert-pair arg ?\( ?\)))
 
+(defcustom delete-pair-blink-delay blink-matching-delay
+  "Time in seconds to delay after showing a pair character to delete.
+The value 0 disables blinking."
+  :type 'number
+  :set-after '(blink-matching-delay)
+  :group 'lisp
+  :version "28.1")
+
 (defun delete-pair (&optional arg)
   "Delete a pair of characters enclosing ARG sexps that follow point.
-A negative ARG deletes a pair around the preceding ARG sexps instead."
+A negative ARG deletes a pair around the preceding ARG sexps instead.
+The option `delete-pair-blink-delay' can disable blinking."
   (interactive "P")
   (if arg
       (setq arg (prefix-numeric-value arg))
@@ -802,6 +811,9 @@ delete-pair
 				      (if (= (length p) 3) (cdr p) p))
 				    insert-pair-alist))
 	      (error "Not after matching pair"))
+	    (when (and (numberp delete-pair-blink-delay)
+		       (> delete-pair-blink-delay 0))
+	      (sit-for delete-pair-blink-delay))
 	    (delete-char 1)))
 	(delete-char -1))
     (save-excursion
@@ -814,6 +826,9 @@ delete-pair
 				    (if (= (length p) 3) (cdr p) p))
 				  insert-pair-alist))
 	    (error "Not before matching pair"))
+	  (when (and (numberp delete-pair-blink-delay)
+		     (> delete-pair-blink-delay 0))
+	    (sit-for delete-pair-blink-delay))
 	  (delete-char -1)))
       (delete-char 1))))
 
diff --git a/lisp/simple.el b/lisp/simple.el
index e96c7c9a6e..d7626e354a 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5087,11 +5087,19 @@ kill-ring-save
   (if (called-interactively-p 'interactive)
       (indicate-copied-region)))
 
+(defcustom copy-region-blink-delay blink-matching-delay
+  "Time in seconds to delay after showing a pair character to delete.
+The value 0 disables blinking."
+  :type 'number
+  :set-after '(blink-matching-delay)
+  :group 'killing
+  :version "28.1")
+
 (defun indicate-copied-region (&optional message-len)
   "Indicate that the region text has been copied interactively.
-If the mark is visible in the selected window, blink the cursor
-between point and mark if there is currently no active region
-highlighting.
+If the mark is visible in the selected window, blink the cursor between
+point and mark if there is currently no active region highlighting.
+The option `copy-region-blink-delay' can disable blinking.
 
 If the mark lies outside the selected window, display an
 informative message containing a sample of the copied text.  The
@@ -5105,12 +5113,14 @@ indicate-copied-region
     (if (pos-visible-in-window-p mark (selected-window))
 	;; Swap point-and-mark quickly so as to show the region that
 	;; was selected.  Don't do it if the region is highlighted.
-	(unless (and (region-active-p)
-		     (face-background 'region nil t))
+	(when (and (numberp copy-region-blink-delay)
+		   (> copy-region-blink-delay 0)
+		   (or (not (region-active-p))
+		       (not (face-background 'region nil t))))
 	  ;; Swap point and mark.
 	  (set-marker (mark-marker) (point) (current-buffer))
 	  (goto-char mark)
-	  (sit-for blink-matching-delay)
+	  (sit-for copy-region-blink-delay)
 	  ;; Swap back.
 	  (set-marker (mark-marker) mark (current-buffer))
 	  (goto-char point)

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> Why can't the delay be a float?
>
> My mistake was influenced by the lack of the function that
> would check for a positive number (e.g. 'positive-number-p').
> I tried to find such a function, then noticed that 'natnump'
> at least checks for a non-negative number, but I forgot that
> this applies only to integers.

The closest function that I found is:

  (cl-plusp NUMBER)
  Return t if NUMBER is positive.

but I'm not sure why it would be better than using just (> number 0).

> So below is a fixed patch:

Now pushed to master.

bug#4136: 23.1; delete-pair

[Drew Adams]

> The closest function that I found is:
> 
>   (cl-plusp NUMBER)
>   Return t if NUMBER is positive.
> 
> but I'm not sure why it would be better than using just (> number 0).

It's there to emulate a Common Lisp predicate, `plusp'.

Why does Common Lisp have `plusp'?  Presumably for
use with higher-order functions, to not have to use
a lambda form etc.

(mapcar #'plusp '(4 -3.1 0 42))
(mapcar (lambda (x) (> x 0)) '(4 -3.1 0 42))

bug#4136: 23.1; delete-pair

[martin rudalics]

 > The exact reverse is impossible.  For instance, put the cursor
 > on the letter `f' in:
 >
 >     foo "" bar
 >
 > and type `M-3 M-"' where M-" is bound to `insert-pair'.
 > The result will be:
 >
 >     "foo "" bar"

Obviously `insert-pair' would have to escape the old quotes here ;-)

martin, who couldn't resist

bug#4136: 23.1; delete-pair

[Stefan Monnier]

> ...this is exactly the issue: it is much better if `delete-foo' is
> always an operation that reverts what `insert-foo' does.

I don't think we shojuld make it hard to delete (...] just because it
may be done by mistake.  That's what `undo' is for.

So, I'm in favor of making delete-pair a bit more picky, but not quite
as much as you suggest.  I suggest we try and make sure delete-pair
indeed only eliminates chars like ([" on the left side and )]" on the
right side (including by moving point a little bit in order to find the
intended pair), but let's not impose that ( can only match ).

> The current state of `delete-pair' is so bad that my guess is that
> hardly anyone used it, so adding another command doesn't make much
> sense.

It works fine, tho it indeed does it in a "blind" way that means that
you may get surprise results which require undo.  No big deal, tho.


        Stefan

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> ...this is exactly the issue: it is much better if `delete-foo' is
>> always an operation that reverts what `insert-foo' does.
>
> I don't think we shojuld make it hard to delete (...] just because it
> may be done by mistake.  That's what `undo' is for.
>
> So, I'm in favor of making delete-pair a bit more picky, but not quite
> as much as you suggest.  I suggest we try and make sure delete-pair
> indeed only eliminates chars like ([" on the left side and )]" on the
> right side (including by moving point a little bit in order to find the
> intended pair), but let's not impose that ( can only match ).

I don't oppose strict checking for only a pair of ( and ) because
a situation with (...] is extremely rare and usually means that
there is some bug.  So in reality there is no problem with
checking against the `insert-pair-alist' list.

What I care about is simplicity.  It should be easy for the user
to understand and remember what the command `delete-pair' does.
Failing on a non-pair character would be natural for a command
that is a counterpart of `insert-pair'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4136: 23.1; delete-pair

[martin rudalics]

 > This command is useful even if it doesn't fail on (...] because
 > forward-sexp and backward-sexp don't fail on (...].

Is this carved in stone?  The TODO file has

** (Controlled by a flag) make open and close syntax match exactly,
   i.e. `(' doesn't match `]'.

I always wondered what that entry aimed at ...

 > It seems you and Martin prefer checking against the `insert-pair-alist'
 > because the function name `delete-pair' suggests it should be
 > a counterpart of `insert-pair'.  In this case I think we sould have
 > two commands: one more strict version `delete-pair' that checks
 > `insert-pair-alist', and another loose version with the name e.g.
 > `delete-parens' with the body of the current `delete-pair'.

In any case, removing the first and last character from a name doesn't
strike me as very "pair-like" ;-)

martin

bug#4136: 23.1; delete-pair

[martin rudalics]

 > This is rather vague semantics.  I suggest to keep the current simple
 > semantics and just to fix it with the patch I sent.

Before a quoted object your fixed version deletes the last character of
the object and the first quote character.  Handling quote or prefix
syntax characters via (delete-char 1) doesn't strike me as very clean.

martin

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> This is rather vague semantics.  I suggest to keep the current simple
>> semantics and just to fix it with the patch I sent.
>
> Before a quoted object your fixed version deletes the last character of
> the object and the first quote character.  Handling quote or prefix
> syntax characters via (delete-char 1) doesn't strike me as very clean.

This is fixed in the following version:

(defun delete-pair ()
  "Delete a pair of characters enclosing the sexp that follows point."
  (interactive)
  (save-excursion
    (forward-sexp 1)
    (save-excursion
      (backward-sexp 1)
      (skip-syntax-forward "'")
      (delete-char 1))
    (delete-char -1)))

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4136: 23.1; delete-pair

[martin rudalics]

 > This is fixed in the following version:
 >
 > (defun delete-pair ()
 >   "Delete a pair of characters enclosing the sexp that follows point."
 >   (interactive)
 >   (save-excursion
 >     (forward-sexp 1)
 >     (save-excursion
 >       (backward-sexp 1)
 >       (skip-syntax-forward "'")
 >       (delete-char 1))
 >     (delete-char -1)))

Still doesn't seem TDTRT with `point' before something like

        `foo'

martin

bug#4136: 23.1; delete-pair

[Juri Linkov]

>> This is fixed in the following version:
>>
>> (defun delete-pair ()
>>   "Delete a pair of characters enclosing the sexp that follows point."
>>   (interactive)
>>   (save-excursion
>>     (forward-sexp 1)
>>     (save-excursion
>>       (backward-sexp 1)
>>       (skip-syntax-forward "'")
>>       (delete-char 1))
>>     (delete-char -1)))
>
> Still doesn't seem TDTRT with `point' before something like
>
>        `foo'

I know, I know, after I fix this, you'll come up with another
test case like

         `foo bar'

;-)

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4136: 23.1; delete-pair

[martin rudalics]

 > I know, I know, after I fix this, you'll come up with another
 > test case like
 >
 >          `foo bar'

I wouldn't.  Doing a `forward-sexp' on `foo' wouldn't get me to the end
of `bar'.  So you could all too easily reject such a test case ;-)

If, however, `delete-pair' were called with a prefix argument of 2 ...

martin

bug#4136: 23.1; delete-pair

[Juri Linkov]

> `delete-pair' is deleting what it documents -- instead of removing the
> open paren of the following sexp, it deletes the current one.  So, if
> the cursor is on some whitespace that precedes an expression, the
> whitespace is deleted, and the open paren is left intact.

This should be fixed with the following patch:

Index: lisp/emacs-lisp/lisp.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/emacs-lisp/lisp.el,v
retrieving revision 1.102
diff -u -r1.102 lisp.el
--- lisp/emacs-lisp/lisp.el	22 Jul 2009 02:45:37 -0000	1.102
+++ lisp/emacs-lisp/lisp.el	13 Aug 2009 23:27:42 -0000
@@ -530,8 +530,12 @@
 (defun delete-pair ()
   "Delete a pair of characters enclosing the sexp that follows point."
   (interactive)
-  (save-excursion (forward-sexp 1) (delete-char -1))
-  (delete-char 1))
+  (save-excursion
+    (forward-sexp 1)
+    (save-excursion
+      (backward-sexp 1)
+      (delete-char 1))
+    (delete-char -1)))
 
 (defun raise-sexp (&optional arg)
   "Raise ARG sexps higher up the tree."

-- 
Juri Linkov
http://www.jurta.org/emacs/