smartparens icon indicating copy to clipboard operation
smartparens copied to clipboard

Add :no-enclosing-expression message

Open tomdl89 opened this issue 1 year ago • 2 comments

I noticed quite a few commands fail with a generic error when point is not enclosed by a sexp. E.g. call sp-forward-barf-sexp when not inside parens, and you will see:

Wrong type argument: number-or-marker-p, nil

Because many commands call sp-get-enclosing-sexp which can return nil, and then subsequent operations try to do e.g. math with nil resulting in the above sort of error.

This PR adds a new sp-message error and tries to display that rather than the generic error, where possible. I gave up trying to run tests locally, but happy to be instructed if it's not too much effort :)

tomdl89 avatar May 08 '23 14:05 tomdl89

The code which is wrapped with -when-let or when ok shouldn't give any errors because the body won't execute. Where it does, it probably misses the check and there it might be reasonable to handle it.

In non-interactive commands, we shouldn't display any errors because they might not be called by the user. Some of the functions can be called both interactively and non-interactively and this is quite a mess to handle.

Rather than adding the messages, I would handle the nil errors only.

Fuco1 avatar May 29 '23 09:05 Fuco1

Hey @Fuco1 thanks for your reply. I'm not sure I understand it entirely.

  • I agree that the code wrapped with -when-let or when ok shouldn't give any errors because the body won't execute, but it does, so that's why I added this PR. Are you suggesting the underlying error be fixed rather than showing an error message? I think calling these commands not inside an enclosing expression is kind of undefined behaviour, so an error does seem to make sense. In fact, this seems more like an argument against error messages in general, no?
  • All the functions I added this to (apart from sp--next-thing-selection which was displaying this message anyway) are "interactive commands", but I agree messages shouldn't be displayed when not called interactively. Adding to sp-message a check via called-interactively-p could fix that across the board?
  • What do you mean by "I would handle the nil errors only"?

Thanks again

tomdl89 avatar May 29 '23 17:05 tomdl89