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

JS arrow functions are difficult to type in strict mode #823

Open nickmccurdy opened 6 years ago

nickmccurdy commented 6 years ago

Expected behavior

I can type () => {} or any other valid JS ES6 arrow function and strict mode will allow me to type the > without matching it with a <.

Actual behavior

I'm prevented from typing > on its own so I have to type C-u > every time. Basically, smartparents doesn't know this is valid syntax so it assumes it's an unmatched angle bracket.

Steps to reproduce the problem

  1. Open any supported JS mode
  2. Enable smartparens-strict-mode
  3. Type () => {} or any other valid JS ES6 arrow function

Environment & version information

Fuco1 commented 6 years ago

Is > a standard pair in javascript? If so we could configure it out of the box.

I can see how this could be handled with js2-mode where we could access the AST. I don't think we can do much better than some sort of regex hack for js-mode.

nickmccurdy commented 6 years ago

I'm not sure exactly what you mean by "standard pair". As far as I'm aware, the spec itself doesn't have any matching angle brackets, but JSX (which is a non-standard syntax frequently seen in the same file and supported by js2-mode and web-mode) does allow them to represent HTML tags. So it would depend on is smartparens has JSX represented as a separate syntax or a part of the JS syntax. Either way I doubt => would be valid for anything other than an arrow even in JSX, so a regex hack should hopefully be easy.

LilyMGoh commented 6 years ago

+1 I'm experiencing the same problem and would love to see this fixed.

Ionshard commented 6 years ago

I have solved this problem by removing the < and > pairs from smartparens with the following line:

(sp-local-pair '(react-mode) "<" ">" :actions nil)

I am specifically doing this for react-mode because I am using spacemacs and that's the major mode that enables JSX handling. This completely removes the < and > handling, which I am mostly okay with because I just use emmet mode anyways.

I tried following some information from https://github.com/Fuco1/smartparens/wiki/Permissions#filters to try and allow > to be typed only after an = but could not figure it out. Any advice would be helpful!

Fuco1 commented 6 years ago

There are two things that need to be done. First, you need to specify a predicate in :unless filter to only enable the insertion when there is no= in front.

Second is to set up :skip-match to ignore the > in case there is a = in front of it so as not to try to pair it with a (non-existing) opening <.

The logic is pretty much the same as in smartparens-text.el which adds support for emoticons, such that :-) the paren is not being matched to ( because there is none.

Ionshard commented 6 years ago

Thanks that was the push I needed. For others the following worked for me:

  ;;;; Allow ES6 arrow '=>' in react-mode
  (defun sp-after-equals-p (_id action _context)
    (when (memq action '(insert navigate))
      (sp--looking-back-p "=>" 2)))

  (defun sp-after-equals-skip (ms mb _me)
    (when (eq ms ">")
      (save-excursion
        (goto-char mb)
        (sp--looking-back-p "=" 1))))

  (sp-local-pair '(react-mode) "<" nil
                 :unless '(:add sp-after-equals-p)
                 :skip-match 'sp-after-equals-skip-p)

There's still a weird issue where if I try to put => in the JSX it will put = (e.g. replace the > with a space) but I shouldn't be doing that so I will ignore it. I am pretty sure that's something in web-mode

Fuco1 commented 6 years ago

@Kasuko Sweet job! Would you be interested in making a pull request?

I don't understand your last comment about "trying to put => in JSX", can you show an example?

Ionshard commented 6 years ago

Sure, I could make a pull request, I just don't know where this would go. I actually use Spacemacs so this issue arose when installing the react layer, which adds a react major mode, but I don't know if that's accurate to the emacs situation as a whole. If I am just editing javascript then the < > pairs are not even present so adding a => works just fine. If you tell me where this should go, I'd be happy to put it there.

As for the strange => JSX thing I have captured a gif

kapture 2018-07-06 at 13 11 34

So when I enter the > after an = inside of the JSX parts (e.g. after the <div> tag) it just doesn't show up and seems to be replaced with a space. I'm not sure what the sp--post-self-insert-hook-handler error indicates.

I am aware that placing the => in that location is invalid syntax, which is why I am not super worried about it.

If I attempt to place the => in a proper JS block in the JSX e.g.

<div>
  {() => (<span>"Something"</span>}
</div>

it works fine, which is what seems to me to indicate it's some kind of web-mode interaction that is causing the odd behaviour.

Fuco1 commented 6 years ago

Thanks for the gif, that is superb! I think there is some issue in smartparens and the post insert handler. As you say it's probably not super high priority.

The code can go to smartparens-javascript.el. Am I right to assume react-mode is in fact this: https://github.com/felipeochoa/rjsx-mode ? In which case it will get autoloaded properly because that mode extends js2-mode.

You should then change the prefix of the methods to sp-react-- to avoid possible conflicts.

Ionshard commented 6 years ago

I don't think it's rjsx-mode this is defined here https://github.com/syl20bnr/spacemacs/blob/master/layers/%2Bframeworks/react/packages.el#L66

I am quite unfamiliar with emacs-lisp but I am learning. I think that react-mode is declared directly in that layer (which is a spacemacs only concept) and it is derived from the web-mode major mode.

Does this change where/if I would add that code?

Fuco1 commented 6 years ago

In that case it would be better to create a separate smartparens-react.el, put the code there and then add an eval-after-load to smartparens-config.el to require this file when react-mode is loaded.

As a side note I think it would make sense for spacemacs to adapt rjsx mode as it works directly with js2 mode and supports all the react syntax also. But that's a separate issue.

ROCKTAKEY commented 2 years ago

Hi, @Fuco1, @Ionshard. The code https://github.com/Fuco1/smartparens/issues/823#issuecomment-403019519 uses wrong function name on :skip-match (-p suffix is not needed), so I fix it. In addition I added web-mode. It seems to work well. How is it?

;;;;;;;;;;;;;;;;;outdated;;;;;;;;;;;;;;;;;;;;;;;
;;;; Allow ES6 arrow '=>' in react-mode
(defun sp-after-equals-p (_id action _context)
  (when (memq action '(insert navigate))
    (sp--looking-back-p "=>" 2)))

(defun sp-after-equals-skip (ms mb _me)
  (when (eq ms ">")
    (save-excursion
      (goto-char mb)
      (sp--looking-back-p "=" 1))))

(sp-local-pair '(web-mode react-mode) "<" nil ; add web-mode
               :unless '(:add sp-after-equals-p)
               :skip-match 'sp-after-equals-skip) ; remove -p suffix

Actually, another problem is not fixed. Although insertion of => can be done, when highlighting react JSX, => is recognized as the end of tag. For example: image

Edit: I should use string= instead of eq. It works well.

;;;; Allow ES6 arrow '=>' in react-mode
(defun sp-after-equals-p (_id action _context)
  (when (memq action '(insert navigate))
    (sp--looking-back-p "=>" 2)))

(defun sp-after-equals-skip (ms mb _me)
  (when (string= ms ">")
    (save-excursion
      (goto-char mb)
      (sp--looking-back-p "=" 1))))

(sp-local-pair '(web-mode react-mode) "<" nil ; add web-mode
               :unless '(:add sp-after-equals-p)
               :skip-match 'sp-after-equals-skip) ; remove -p suffix