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.83k stars 194 forks source link

sp-forward-sexp doesn't work on floats in python-mode #863

Open djr7C4 opened 6 years ago

djr7C4 commented 6 years ago

sp-forward-sexp does not treat '.' as part of a float in python-mode (and presumably some others as well). It works as expected in emacs-lisp-mode. I assume that this is because '.' is considered punctuation in python-mode but is not in emacs-lisp-mode.

It may be worth noting that the vanilla forward-sexp function works as expected (apparently because it sets forward-sexp-function to a python-specific value).

Expected behavior

|1.2 --> 1.2|

Actual behavior

|1.2 --> 1|.2

Steps to reproduce the problem

Run M-x sp-forward-sexp before a floating point number

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

N/A

Environment & version information

Fuco1 commented 6 years ago

Yea, can reproduce. I'm not sure what a good general solution would be. Ignore punctuation not followed by a space? Redefine the syntax? I wish every day that the syntax tables were never invented :( Such a mess.

djr7C4 commented 6 years ago

I think that redefining the syntax might break other packages and/or builtin functions. Usually, a '.' in Python is puntuation referring to the membership operator and obj.x should arguably be treated as a single sexp. However, currently the same issue actually occurs for obj.x as well (I just tested it).

I'm not sure what the intended behavior is in that case though. In python-mode, the standard forward-sexp command treats obj.x and also things like 1 + 2 as a single sexp which sp-forward-sexp does not. Of course, in those cases it's ambiguous as to what the top-level expression so one could make an argument for doing things either way.

I would argue that they should be treated as complete sexps. This is the behavior that is useful for example when one is inside a list of such expressions (e.g. [1.23, 1 + 2, obj.x]).

The same bug with floats is also present in c-mode. In that case, the default forward-sexp command is also broken.

If you ignore punctuation not followed by a space, it will cause code like a,b to be treated as a single sexp (which it should not be). Usually, a space is included after the ',', so that might not be too much of an issue.

I guess the correct general solution will depend on if you also want to treat things like 1 + 2 and obj.x as a single sexp in sp-forward-sexp or not. A complication is that ',' is also considered punctuation in Python...

Fuco1 commented 6 years ago

In SP the decision whether the character should stop the traversal or not should be handled in two places and that is sp-forward-symbol (and backward version) and sp--skip-to-symbol-1 macro (which is an abomination). Maybe there are more but this should get us 99% to the goal.

I think providing a way for a major-mode to hook in a custom predicate would solve a lot of issues. We can then implement one for python that would be smart enough in the cases you outlined above (thanks for the analysis by the way!).

djr7C4 commented 6 years ago

I'm glad the comments were useful.

Maybe it would be enough for SP to distinguish between two different types of punctuation characters: operators (like '+', '*', etc.) and separators (like ',', ';' and ':' in Python mode). The logic for sp-forward-sexp would then work something like this:

1) if before an opening delimiter, skip to the corresponding closing delimiter ignoring non-delimiter characters inside the sexp 2) if before an sexp that does not start with an opening delimiter, then skip over operator punctuation characters but do not skip over separator punctuation characters

sp-forward symbol would work as it does now and would not skip over any punctuation characters.

Doing this via a language-specific predicate sounds good. There could also be a default value for the predicate to be used in non-lisp languages that implements the above logic, since this logic seems like it will apply to some other non-lisp languages besides just Python. The distinction between operators and separators could be implemented by adding a couple new language-specific variables (which the default predicate could check to decide what to do). Lisp languages could have a different predicate that corresponds to the current behavior.

I can look into implementing this, if you like.

Fuco1 commented 6 years ago

I think I would prefer not to have the logic in sp-forward-sexp. That function if very simple and uses other "primitives" we have (strings, sexps, paired expressions, symbols etc) via sp-get-thing. It doesn't feel right to bake some special behaviour there.

I think we should put this in sp-forward-symbol (the skip functions won't have to be changed I think after all). Symbol in smartparens is a somewhat loose abstraction over what emacs considers symbols, it is a "unit" that it doesn't make sense to break, so a float 1.2 usually makes no sense as just 1. or .2---although some languages can parse such syntax, if we were to delete one part we usually want to delete it all. There is always delete-word for the other case.

Though the case of 1 + 2 makes sense both ways, you might want to replace an argument or the whole expression.

djr7C4 commented 6 years ago

In the case of 1 + 2, it seems redundant to move point just past the 1 since sp-forward-symbol will do that too. The situation where I could see it being more useful is in an expression like (1 + 2) 3, where you might want to go to the point just before the , but expressions like 1 + 2 + 3 are probably more common, and the same could still be achieved using sp-forward-symbol with a prefix argument of 2.

Another challenge with making sp-forward-sexp behave the way I mentioned is that it would have to understand mode-specific syntax. The case that comes to mind is the lambda expression (e.g. lambda x: x + 1).