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.81k stars 193 forks source link

Don't insert matching pair if inserting before word #965

Open dolorsitatem opened 5 years ago

dolorsitatem commented 5 years ago

Expected behavior

Don't insert matching ) when point is before a word and a ( is inserted.

Actual behavior

A ) is inserted.

Steps to reproduce the problem

Start emacs -Q.

Configure MELPA:

(package-initialize)
(require 'package)
(add-to-list 'package-archives
         '("melpa" . "http://melpa.org/packages/") t)
(package-refresh-contents)

Then install package using M-x list-packages.

In the *scratch* buffer, call

(smartparens-global-mode 1)
(sp-pair "(" ")" :unless '(sp-point-before-word-p))

or

(smartparens-global-mode 1)
(sp-pair "(" nil :unless '(sp-point-before-word-p))

Type word Put point before word Press ( A matching ) is inserted.

The steps followed here apply the solutions given in #717 and #645. The call (sp-pair "(" ")" :unless '(sp-point-before-word-p)) comes verbatim from #717. Issue #645 says to use :unless and to use escapes for brackets. The :unless keyword is used here to no effect and escaping the parens with \\( also does not produce the expected behavior.

Environment & version information

dolorsitatem commented 5 years ago

You must call (require 'smartparens) and (sp-pair "(" ")" :unless '(sp-point-before-word-p)) needs to be called before toggling the mode on. Calling it after the mode has been toggled doesn't seem to work.

After installing the package, do

(require 'smartparens)
(sp-pair "(" ")" :unless '(sp-point-before-word-p))
(smartparens-global-mode 1)

The require statement is needed despite smartparens-global-mode toggling the mode on, as demonstrated in the original question. Also, toggling smartparens-global-mode prior to redefining sp-pair doesn't work. I don't know why, but whatever.

Also confusing is when enabling smartparens through use-package. As far as my understanding of use-package goes, it is meant to replace require statements. However, unlike any other package I have loaded with use-package, the require is still needed:

(use-package smartparens
  :config
  (require 'smartparens)
  (sp-pair "(" ")" :unless '(sp-point-before-word-p))
  (smartparens-global-mode 1))

Hopefully this saves someone else a few hours of fiddling.

Fuco1 commented 5 years ago

The definition of the global mode is as follows

;;;###autoload
(define-globalized-minor-mode smartparens-global-mode
  smartparens-mode
  turn-on-smartparens-mode)

so it should trigger autoloading of the package. The function sp-pair is not autoloaded, so if the package is not loaded at that point it would error on undefined function. Which means the package is loaded and the require should be unnecessary.

dolorsitatem commented 5 years ago

Thank you for taking the time to respond, especially after I'd closed the issue!

I'm confused because when the package is loaded via (smartparens-global-mode 1), calling sp-pair seems to have no effect.

The only way I could prevent a matching pair from being inserted before a word was through the sequence:

  1. load package
  2. call sp-pair
  3. toggle mode

Is this the intended behavior?

Fuco1 commented 5 years ago

sp-pair should reload the settings in all active buffers but maybe that is where the problem lies.

What you can try to do is evaluate sp-local-pairs in the buffer and paste the result here, there we can see if your setting was merged into the buffer's settings or not.

dolorsitatem commented 5 years ago

I'm not sure I have what you want.

Running emacs -Q with the following:

(package-initialize)
(require 'smartparens)
(smartparens-global-mode 1)

sp-local-pairs

(sp-pair "(" ")" :unless '(sp-point-before-word-p))

sp-local-pairs

produced the following output in *Messages* (I put a newline between the outputs):

Mark set nil smartparens t ((:open "" :close "" :actions (insert wrap autoskip navigate)) (:open "{" :close "}" :actions (insert wrap autoskip navigate)) (:open "[" :close "]" :actions (insert wrap autoskip navigate)) (:open "(" :close ")" :actions (insert wrap autoskip navigate)) (:open "'" :close "'" :actions (insert wrap autoskip navigate escape) :unless (sp-in-string-quotes-p sp-point-after-word-p) :post-handlers (sp-escape-wrapped-region sp-escape-quotes-after-insert)) (:open "\"" :close "\"" :actions (insert wrap autoskip navigate escape) :unless (sp-in-string-quotes-p) :post-handlers (sp-escape-wrapped-region sp-escape-quotes-after-insert)) (:open "\\"" :close "\\"" :actions (insert wrap autoskip navigate)) (:open "\(" :close "\)" :actions (insert wrap autoskip navigate)) (:open "\{" :close "\}" :actions (insert wrap autoskip navigate)) (:open "\\(" :close "\\)" :actions (insert wrap autoskip navigate)))

((t (:open "\\(" :close "\\)" :actions (insert wrap autoskip navigate)) (:open "\{" :close "\}" :actions (insert wrap autoskip navigate)) (:open "\(" :close "\)" :actions (insert wrap autoskip navigate)) (:open "\\"" :close "\\"" :actions (insert wrap autoskip navigate)) (:open "\"" :close "\"" :actions (insert wrap autoskip navigate escape) :unless (sp-in-string-quotes-p) :post-handlers (sp-escape-wrapped-region sp-escape-quotes-after-insert)) (:open "'" :close "'" :actions (insert wrap autoskip navigate escape) :unless (sp-in-string-quotes-p sp-point-after-word-p) :post-handlers (sp-escape-wrapped-region sp-escape-quotes-after-insert)) (:open "(" :close ")" :actions (wrap insert autoskip navigate) :unless (sp-point-before-word-p)) (:open "[" :close "]" :actions (insert wrap autoskip navigate)) (:open "{" :close "}" :actions (insert wrap autoskip navigate)) (:open "" :close "" :actions (insert wrap autoskip navigate))))

((:open "\\(" :close "\\)" :actions (insert wrap autoskip navigate)) (:open "\{" :close "\}" :actions (insert wrap autoskip navigate)) (:open "\(" :close "\)" :actions (insert wrap autoskip navigate)) (:open "\\"" :close "\\"" :actions (insert wrap autoskip navigate)) (:open "\"" :close "\"" :actions (insert wrap autoskip navigate escape) :unless (sp-in-string-quotes-p) :post-handlers (sp-escape-wrapped-region sp-escape-quotes-after-insert)) (:open "'" :close "'" :actions (insert wrap autoskip navigate escape) :unless (sp-in-string-quotes-p sp-point-after-word-p) :post-handlers (sp-escape-wrapped-region sp-escape-quotes-after-insert)) (:open "(" :close ")" :actions (insert wrap autoskip navigate)) (:open "[" :close "]" :actions (insert wrap autoskip navigate)) (:open "{" :close "}" :actions (insert wrap autoskip navigate)) (:open "" :close "" :actions (insert wrap autoskip navigate)))

Fuco1 commented 5 years ago

Yea, you can see in the second invocation

(:open "(" :close ")" :actions (insert wrap autoskip navigate))

there is no predicate for :unless so something funny is going on there.

I'll reopen this to track the issue.

Fuco1 commented 4 months ago

I think I just managed to accidentally reproduce this today. It seems that you have to change the major mode after calling sp-pair, or simply revert the buffer so things get reinitialized.

This kind of always makes it pop up during a repro case because usually people just do it in scratch, whereas during normal use you might be opening other buffers inbetween.

Funnily enough, in my personal config or any smartparens config we actually never use sp-pair, only the local config.