search

Fuco1 / smartparens

Minor mode for Emacs that deals with parens pairs and tries to be smart about it.
GNU General Public License v3.0
1.82k stars 195 forks source link

sp-clone-sexp : mis-behaving in presence of quotes (') and sharp-quotes (#') #938

Closed bhrgunatha closed 6 months ago

bhrgunatha commented 5 years ago

Expected behavior

sp-clone-sexp should recognise quoted and sharp-quoted identifiers within an sexp and clone that sexp.

Actual behavior

The clone operation leaves a malformed sexp split up over 2 lines and sometimes clones the "grandparent" sexp instead of the "parent".

Steps to reproduce the problem

Entire buffer contents:

(defun my-smartparens-hook ()
  (setq sp-base-key-bindings 'paredit)
  (define-key smartparens-mode-map (kbd "C-, C-y")  #'sp-clone-sexp)
  (define-key smartparens-mode-map (kbd "C-k")      #'kill-sexp)
  (define-key smartparens-mode-map (kbd "C-M-t")    #'sp-transpose-sexp)
  )

Cursor is represented by "|"

First example:

  1. Place cursor inside the first define-key sexp e.g.
    (define-key smartparens-|mode-map (kbd "C-, C-y") #'sp-clone-sexp)

  2. M-x sp-clone-sexp

  3. Result: 

(defun my-smartparens-hook ()
  (setq sp-base-key-bindings 'paredit)
  (define-key smartparens-mode-map (kbd "C-, C-y")  #'
    'paredit)
  (define-key smartparens-|mode-map (kbd "C-, C-y")  #'sp-clone-sexp)
  (define-key smartparens-mode-map (kbd "C-k")      #'kill-sexp)
  (define-key smartparens-mode-map (kbd "C-M-t")    #'sp-transpose-sexp)
  )

Note the #' to end the line and on the next line 'paredit

Second example: Same starting buffer state as above:

  1. Place cursor inside the first setq sexp e.g.
    (setq |sp-base-key-bindings 'paredit)

  2. M-x sp-clone-sexp

  3. Result: 

    (defun my-smartparens-hook ()
(setq sp-base-key-bindings 'paredit)
(define-key smartparens-mode-map (kbd "C-, C-y")  #'sp-clone-sexp)
(define-key smartparens-mode-map (kbd "C-k")      #'kill-sexp)
(define-key smartparens-mode-map (kbd "C-M-t")    #'sp-transpose-sexp)
)
(defun my-smartparens-hook ()
(setq |sp-base-key-bindings 'paredit)
(define-key smartparens-mode-map (kbd "C-, C-y")  #'sp-clone-sexp)
(define-key smartparens-mode-map (kbd "C-k")      #'kill-sexp)
(define-key smartparens-mode-map (kbd "C-M-t")    #'sp-transpose-sexp)
)

Backtraces if necessary (M-x toggle-debug-on-error)

None

Environment & version information

In recent enough smartparens you can call M-x sp-describe-system to generate this report. Please fill manually what we could not detect automatically. Edit the output as you see fit to protect your privacy.

Fuco1 commented 5 years ago

I could not reproduce this on emacs 25. I think E26 redefined some of the syntax properties of these characters and it breaks the code.

Can you confirm this only happens when there are quotes in the sexp and not at other times so we can narrow this down as much as possible? Thanks!

bhrgunatha commented 5 years ago

I haven't noticed problems with no quoted symbols but obviously I can't prove it. In fact experimenting I think it's when there are 2 (or maybe more quotes).

I called sp-clone-exp at every position on the line and recorded the results.

The result remains the same up to a certain position, then the result changes and that changed result is consistent up to the next position where another change occurs. That seems correct to me as the cursor moves along the structure of the sexp.

Example 1

No quoted characters

This seems correct to me. I get the same behaviour with more complex examples (foo (bar) baz qux) - (foo (bar (baz qux)) quux) - (foo (bar) "baz" qux)

Before: 

|(foo (bar) baz)

After: 

(foo (bar) baz)
|(foo (bar) baz)

.. same results until:

Before: 

(foo| (bar) baz)

After: 

(foo (bar)
     |(bar) baz)

.. same results until:

Before: 

(foo (bar)| baz)

After: 

(foo (bar) baz)  
(foo (bar)| baz)

.. same results until:

Before: 

(foo (bar)| baz)|

After: 

(foo (bar) baz)|

Example 2

One Quoted symbols

(foo (bar) 'baz qux) This also seems correct to me.

Two Quoted symbols

It seems to go wrong when there are 2 or more quoted symbols. I get the same incorrect result whether the sexp is one one line or split up onto 2.

Before:

(foo (bar)| 'baz 'qux)

After:

(foo (bar) 'baz '
    | 'baz 'qux)

.. same results until:

Before:

(foo (bar) 'baz '|qux)

After:

(foo (bar) 'baz 'qux)
(foo (bar) 'baz '|qux)

which now seems correct.

Fuco1 commented 5 years ago

Thanks for the detailed report, this will be useful!

Fuco1 commented 6 months ago

I have added test cases from the OP and the later post (7 in total) and they all pass now. We will have them there for future regressions. As I can't reproduce this now I will close this since I can't do anything more at this moment.