search

expez / evil-smartparens

Evil integration for Smartparens
GNU General Public License v3.0
134 stars 17 forks source link

Problem with evil-sp-change indent-according-to-mode #30

Closed andyleejordan closed 8 years ago

andyleejordan commented 8 years ago

Bug

I have a strange (new-ish) issue. When using an operator such as ce in certain modes (so far, markdown-mode and ledger-mode), upon entering insert state, two unexpected things happen: a) the line indents up completely unindented, and b), the cursor is left at the beginning of the line.

Trace

When I remove the call to indent-according-to-mode at the end of evil-sp-change, the issue no longer reproduces (of course, this isn't a solution).

Examples

In ledger-mode, where <P> is the location of the point (the point being on the next letter):

2016/02/13 * Whole Foods                                                        
     Expenses:<P>Food:Groceries                   $49.93                            
     Liabilities:Credit:Citi

When I ce at any point in that snippet (for example, on the F of Food), here's what happens:

2016/02/13 * Whole Foods                                                        
Expenses:<P>:Groceries                   $49.93                                    
     Liabilities:Credit:Andy:Citi

In markdown-mode:

## Net <P>profit

Again, ce:

<P>## Net

Versions

I have a lot of minor modes:

Enabled minor modes: Async-Bytecomp-Package Auto-Compile-On-Load                 
Auto-Compile-On-Save Auto-Composition Auto-Compression Auto-Encryption           
Company Company-Flx Dtrt-Indent Electric-Indent Evil Evil-Commentary             
Evil-Escape Evil-Local Evil-Matchit Evil-Smartparens Evil-Surround               
File-Name-Shadow Flx-Ido Flycheck Font-Lock Fortune-Cookie Git-Gutter            
Global-Auto-Revert Global-Company Global-Evil-Matchit                            
Global-Evil-Surround Global-Flycheck Global-Font-Lock                            
Global-Git-Commit Global-Git-Gutter Global-Undo-Tree                             
Global-Whitespace-Cleanup Ido-Grid Ido-Ubiquitous Line-Number Menu-Bar           
Override-Global Popwin Recentf Shell-Dirtrack Show-Smartparens                   
Show-Smartparens-Global Smartparens Smartparens-Global                           
Smartparens-Global-Strict Smartparens-Strict Tooltip Transient-Mark              
Undo-Tree Which-Key Whitespace-Cleanup Winner

But I've tested with and without smartparens-strict-mode, smartparens-mode, electric-indent-mode, dtrt-indent-mode, etc. It's evil-smartparens, but I bet it's interacting with something else odd (as there haven't been any changes to this package recently).

expez commented 8 years ago

Thanks for a very detailed bug report! Much appreciated!

Of the modes you have enabled electric-indent immediately stands out as a candidate for a bad interaction. Does turning that mode off affect the issue?

andyleejordan commented 8 years ago

No, I tested without it, turning it off had no effect :(

expez commented 8 years ago

markdown-indent-line moves point, I'm pretty sure it's not supposed to do that.

andyleejordan commented 8 years ago

Hm. I have a fix for the bug in markdown-mode. But it didn't fix ledger-mode.

andyleejordan commented 8 years ago

So, for ledger-mode, indent-line-function is set to indent-relative, which is what's aligning it to the previous non-blank line (as it is supposed to, I guess). Why this is set such is a question for Ledger, not for evil-smartparens.

I see two options:

1) wrap indent-according-to-mode with save-excursion (I have a branch doing this that I can PR) 2) remove the calls to indent-according-to-mode

The latter might be a better choice. Indenting when using overridden Evil operators seems to be an undesired side-effect. Why was this call originally added?

andyleejordan commented 8 years ago

It was added in ff9c4b4bc7edd5d5a220d5e26b6001bc56ff02b4.

expez commented 8 years ago

I'd prefer 1) if those are the only possible solutions, to avoid messing up the buffer's indentation.

andyleejordan commented 8 years ago

Ah, but one only solved half the problem when testing.

andyleejordan commented 8 years ago

Another fix would be to modify evil-sp--new-beginning to use the start of the current sexp as the beginning of the motion. This is probably a bit smarter, as the current feature feels a bit hairy, though it works.

When I get some time I'll give this alternative a shot.

andyleejordan commented 8 years ago

@ane I've been playing with this. We don't necessarily want it to use the start of the current sexp, because then an unbalanced attempted delete is no longer a no-op (do we maybe want this as a feature?). To keep a no-op and respect indentation, what I have is an evil-sp--safe-beginning function which takes the max of beg and back-to-indentation (as if at beg).

This seems to do what we want, except now a dd ignores the indentation of a line, and so doesn't delete the whole line.

What behavior do we really want for dd in balanced and unbalanced expressions?

andyleejordan commented 8 years ago

I've got it fixed, with just one last issue to deal with (point moves, but indentation is unaffected :smile:).