alexherbo2 / auto-pairs.kak

Auto-pairing of characters for Kakoune
https://kakoune.org
77 stars 18 forks source link

Modify `auto-pairs-insert-new-line` implementation to remain consistent with associated comment #40

Closed johnisom closed 4 years ago

johnisom commented 4 years ago

The comment right above the implementation shows behavior different than the actual:

     # main() {
     #   ▌
     # }

This is the actual behavior:

# main() {
#   ▌
#   }

This is described in #39.

It looks to be the responsability of the filetypes to produce a correct indenting when pressing Return. I’m afraid all filetypes are affected to not handle indenting in-between content, assuming new lines are inserted at the end of the line.

While I agree that this is valid reasoning, it's still annoying as hell to deal with the indentation.

It is hard, because all filetypes are wrong. I feel like if I fix it in auto-pairs, upstream will never get fixed.

With this proposed implementation, it will still behave exactly the same even if/when upstream eventually gets fixed. Additionally, if you're averse to merging this to master, I would suggest creating a new branch. People can then use it if they get too fed up with the default filetypes behavior.

alexherbo2 commented 4 years ago

Maybe entirely disable hooks during the execution, and a last i action entering in insert mode with hooks.

The illustration misses a possible indenting before the closing pair, which could be represented with a single chevron for “less strong”?

johnisom commented 4 years ago

By a last i action do you mean for it to be the i command? Or perhaps any insertion command, such as o?

alexherbo2 commented 4 years ago

It looks we want to totally handle the indenting, so a last i action after the changes for leaving the user in insert mode with hooks enabled.

Maybe a single chevron too before the opening pair to denote the same indent level than the closing pair.

johnisom commented 4 years ago

It looks we want to totally handle the indenting, so a last i action after the changes for leaving the user in insert mode with hooks enabled.

Considering that many people use <tab> with a hook (for example, when using smarttab.kak) I was thinking that we could also include <tab> with hooks enabled.

After trying to implement only using i (and potentially <tab>) with hooks (as well as a few other combos), it seems too difficult for auto-pairs to completely handle the indenting. Indenting is kind of complicated, and everybody has their configs wired up differently. It seems the best option is to go ahead and use o with hooks and let it handle the indenting for us.

I suggest that we keep everything in a single execute-keys -with-hooks '...' call, like was the case in my first commit. From what I've seen in my tests, out of all the alternatives I explored, this one is the most concise. It also provides the best user experience. If you'd like, I'd be happy to go through the different approaches I took if you want to see the challenges I faced.

Maybe a single chevron too before the opening pair to denote the same indent level than the closing pair.

I'm trying it out and I think at this point it's just making the whole thing less clear with chevrons everywhere. I'll upload it, and if you think it's looking too ugly, I can just revert those commits.

johnisom commented 4 years ago

Actually, hold on that thought for just a bit

johnisom commented 4 years ago

Awesome, I've got a solution that fits your provided criteria that provides a good user experience whilst being somewhat simple

alexherbo2 commented 4 years ago

Can you squash the commits into a single one (except the copyright waiver)?

alexherbo2 commented 4 years ago

Can you prefer Alt + > to > for handling indent on empty lines?

alexherbo2 commented 4 years ago

glj<a-gt> will not work (enter in insert mode with a trailing whitespace) because the cursor is not on the newline character. Prefer j<a-gt>A<esc>. Adding l to glj<a-gt> would also work but is less idiomatic.

alexherbo2 commented 4 years ago

I agree about the chevrons. I let you handle what you find clearer.

johnisom commented 4 years ago

glj<a-gt> will not work (enter in insert mode with a trailing whitespace) because it is not on the newline character. Prefer j<a-gt>A<esc>. Adding l to glj<a-gt> would also work but is less idiomatic.

Thanks for catching this! When I made the update last night (when I was very tired) I didn't consider the possibility of something like this:

{
    |
}

This is very obvious in hindsight and I'll have a fix out soon.

johnisom commented 4 years ago

Do you think that the vertical padding example on the README takes up too much space? I like the clarity it provides, but do recognize it takes up lots of space.

Once I get approval for both of these changes I'll squash all of my commits.

alexherbo2 commented 4 years ago

It looks good. o/

johnisom commented 4 years ago

Ok, those changes have been squashed

alexherbo2 commented 4 years ago

@johnisom There is a typo for “vertical” and | instead of .

EDIT: Can you also edit the commit message without “squash”?

johnisom commented 4 years ago

Ok, I amended the commit message

alexherbo2 commented 4 years ago

@johnisom Thanks! o/