Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inserting newline places closing brace at wrong indentation level #9

Closed
andreyorst opened this issue Aug 27, 2018 · 37 comments
Closed

Comments

@andreyorst
Copy link

pairs
only [] and various quotes are expanding normally with newline.

@alexherbo2
Copy link
Owner

alexherbo2 commented Aug 27, 2018

Auto-pairs doesn’t handle the indentation by itself, but triggers the normal hooks.

When inserting a new-line, it goes up and inserts a new-line.

@andreyorst
Copy link
Author

hm, indeed, I didn't noticed that before, because mainly inserted closing pair at right indentation already. Sorry for wrong issue report, but where should I dig then?

@alexherbo2
Copy link
Owner

Hm, maybe on Kakoune directly.

@alexherbo2
Copy link
Owner

Which language is concerned?

@andreyorst
Copy link
Author

andreyorst commented Aug 27, 2018

Which language is concerned?

c-family

@andreyorst
Copy link
Author

So I've noticed that indentation actually works correctly, however the empty line is placed at wrong positon, and closing bracket too.
When auto-pairs are disabled, after I enter { and hit Enter the next line appears indented as it should:
image
(That thin gray line is a tab character)
With auto-pairs turned on, the indented line also exists, however the empty line is added before it, and closing paren is added on the indented line. That's the problem, and it's not issue with kakoune actually:
image

@maximbaz
Copy link
Collaborator

Does it repro with minimal kakrc and latest commit from this master? Doesn't seem to repro for me, also (finally working) unit tests are green and there are a couple of tests for a similar example.

@andreyorst
Copy link
Author

yes. Fully empty kakrc, and manual sourcing auto-pairs still produces this issue for c filetype

@maximbaz
Copy link
Collaborator

What OS do you use?

Can you clone this repo and run unit tests, and tell me if any of them fail?

$ cd /path/to/auto-pairs
$ test/run /path/to/cloned/kakoune-repo/

@andreyorst
Copy link
Author

I've re-cloned the repo, and issue is still here.
My os is Arch Linux, login shell is bash, user shell is zsh. sh is linked to dash.

tests log:

auto-pairs
    delete-closer
    delete-new-line
    delete-opener
    delete-opener-closer
    delete-single-quote
    delete-space
    insert-closer
    insert-nested-pairing
    insert-new-line
    insert-new-line-below
    insert-opener
    insert-opener-closer
    insert-single-quote
    insert-space
    surround-delete-opener
    surround-delete-space
    surround-insert-opener
    surround-insert-space

Resume: 18 tests, 0 failures

@maximbaz
Copy link
Collaborator

Weird! My setup is very similar to yours and I don't repro.

If you want to help investigate, consider adding a test for your exact example and confirming that this unit test fails, because it's reeeeeally suspicious.

@andreyorst
Copy link
Author

strangely enough the test I've wrote passes just fine.

@andreyorst
Copy link
Author

andreyorst commented Nov 21, 2018

Here's me launching freshly compiled Kakoune from master branch, with -n and sourcing everything manually:
problem

@maximbaz
Copy link
Collaborator

I wish I could help but I really have no clue how to investigate this if it doesn't repro via unit tests and on my machine with the same OS and shell as you 😞

@andreyorst
Copy link
Author

this also happens for me in termux. I'll try to do fresh install of something like ubuntu in VB and try there to see if it is reproduces.

@andreyorst
Copy link
Author

andreyorst commented Nov 25, 2018

So. I did just downloaded Ubuntu 18.10 iso from official Ubuntu page.
Installed it in virtualbox.
Cloned kakoune and auto-pairs
Installed build-essential, libncurses-dev, and pkg-config or something like that.
Compiled kakoune, and done this:
bug

All test are passing just fine.

@bugworm
Copy link

bugworm commented Nov 26, 2018

screenshot_telegram_x_20181126-122409
Kak in termux, rust lang

@maximbaz
Copy link
Collaborator

Managed to reproduce! So it happens only if you run auto-pairs-enable manually, and doesn't happen when it runs from WinCreate hook!

hook global WinCreate .* %{ auto-pairs-enable }

If I have this, it works fine. If I comment this and run auto-pairs-enable myself like on gifs above, the issue appears.

I have no clue as to why. @alexherbo2? But presumably plug.kak runs the command not from WinCreate hook and that's why you experience the issue?

@alexherbo2
Copy link
Owner

Maybe a race condition with the execution of hooks between the languages and auto-pairing.

@andreyorst
Copy link
Author

@maximbaz personally I don't run this command in plug command, but you're right, plug.kak doesn't use hooks for commands it executes them with execute-command one by one, so this will still be reproducible with plug.kak.

Using your hook it works fine. But I tend to disable this automatic pair insertion time to time, as sometimes it just not what I want. In rare cases, but still.

@andreyorst
Copy link
Author

Also using auto-pairs-toggle after successful hook breaks everything again.

@andreyorst
Copy link
Author

andreyorst commented Nov 26, 2018

Maybe a race condition with the execution of hooks between the languages and auto-pairing.

Yeah, so if I turn auto pairs on and then set filetype option to c it works fine. If I set filetype option to c and toggle auto pairs after it doesn't.

@maximbaz
Copy link
Collaborator

Not sure if this can be solved in a meaningful way in the plugin itself. Was thinking about something like this to ensure auto-pairs' hooks are first:

  • save current filetype
  • set filetype to '' to
  • execute auto-pairs-enable
  • restore filetype

But it feels hacky, also with c filetype it's just a lucky coincidence that c hooks don't misbehave after auto-pairs does its magic.

@alexherbo2
Copy link
Owner

It’s a problem that should be addressed upstream.

@dpc
Copy link

dpc commented Nov 27, 2018

I cam here to report this exact issue. Can anyone describe in more detail what is the problem?

@andreyorst
Copy link
Author

Can anyone describe in more detail what is the problem?

Seems like if plugin defines it's hooks before filetype hooks the order for executing hooks is correct. Solution for now is to activate plugin by such hook: hook global WinCreate .* %{ auto-pairs-enable }

It’s a problem that should be addressed upstream.

Well I guess then you should adress it upstream as a maintainer? I'm not sure how to do it properly since I don't know deep enough how your plugin is working, and what's actually causing an issue here.

@alexherbo2
Copy link
Owner

alexherbo2 commented Nov 27, 2018

Even without auto-pairs, the indentation is wrong.

The indentation is false when inserting a new line between braces.

The indentation is not corrected when inserting the closing brace at the beginning of the line.

@andreyorst
Copy link
Author

I know that there are problems with lisp filetype indentation (it adds extra space on every line for some reason), but for c-family I was pretty much pleased with indentation handling most of the time.

@alexherbo2
Copy link
Owner

I can add a fix, but there is two bugs that should be corrected in C-family before.

@dgrisham
Copy link

I assume this is still waiting on mawww/kakoune#2590?

@andreyorst
Copy link
Author

I don't think that this issue is related

@alexherbo2
Copy link
Owner

mawww/kakoune#2806

@alexherbo2
Copy link
Owner

@andreyorst 6abc67f Does it help?

@andreyorst
Copy link
Author

it looses indentation. Cursor is placed correctly though.

With this hook hook global WinCreate .* %{ auto-pairs-enable }:

int main() {|}
RET
int main() {
    |
}

after 2 auto-pairs-toggle:

int main() {|}
RET
int main() {
|
}

@alexherbo2
Copy link
Owner

I don’t know how to solve the hook execution order.

@andreyorst
Copy link
Author

Actually, because of this long standing issue I've stopped using auto pairs in all editors :D

But your solution for surrounding is outstanding and I use this plugin for it every day.

@alexherbo2
Copy link
Owner

Wow thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants