Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
bhrgunatha opened this issue Oct 22, 2018 · 4 comments

Comments

@bhrgunatha
Copy link

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.

  • smartparens version: 20181007.1501
  • Active major-mode: emacs-lisp-mode
  • Smartparens strict mode: nil
  • Emacs version (M-x emacs-version): GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30) of 2018-07-05
  • Starterkit/Distribution: Vanilla
  • OS: gnu/linux
@Fuco1
Copy link
Owner

Fuco1 commented Oct 22, 2018

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
Copy link
Author

bhrgunatha commented Oct 23, 2018

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
Copy link
Owner

Fuco1 commented Oct 23, 2018

Thanks for the detailed report, this will be useful!

@Fuco1 Fuco1 added this to Triage Mar 18, 2024
@Fuco1 Fuco1 moved this to To triage in Triage Mar 18, 2024
@Fuco1
Copy link
Owner

Fuco1 commented Mar 25, 2024

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.

@Fuco1 Fuco1 closed this as completed in 38afe79 Mar 25, 2024
@github-project-automation github-project-automation bot moved this from To triage to Done in Triage Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

2 participants