Window height is reduced when 'noequalalways' is set

[weirdan]

When having 'noequalalways' set, invoking omnicompletion in top window causes the window height to be reduced, eventually causing below 1new to fail with E36: Not enough room error. This error is silenced, leading to all windows being destroyed with only E839: Completion function changed window message on the message stack. Also, both windows are deleted (probably because after 1new fail, completion operates on the wrong (non-temporary) buffer).

Test case:

:set noequalalways
:split
:resize 1
<C-x> <C-o> # on anything that loads files with 'below 1new'

[complex857]

Thank you very much for reporting this!

I'm not exactly sure that i get this correctly, but it appears to me that the first below 1new works but the second one will fail because splits won't rebalance their sizes and there's no space for one more "1 line sized window" below the first one-line-sized one (vim tries to split a 1 line space into two).

It also appears to me that if you have set noequalalways then every split halves the available "space" so in theory if you would remove the 1s from the 1new places, you would run out of space in O(Log N) which is still better then running out at the second attempt no matter the available space.

I'm not sure how to test this the proper way, adding set noequalalways to the test's vimrc generate a whole bunch of E16: Invalid range: 2wincmd w error (which is most likely related but not sure how at this moment), I'll have to take a look into that.

However I've created a patch that enables and restores the noequalalways value during completion (like you usually do with nocp), could you give it a try?
You should be able to apply it with patch -p1 < noequalalways.patch from the plugin's root.

[weirdan]

Your theory seems plausible, will test your patch today. Re the patch: couldn't it be simplified by restoring the option in :finally?

[weirdan]

The patch seems to be working for me.

[complex857]

Awesome news, I'm a little hesitant about solving the problem this way since it will "ruin" the layout of your splits every time you invoke the completion (it's bad enough already that it inflates the buffer counter).

I'm thinking of just removing the 1s from the 1news as an alternative, so vim would manage the split sizes the way it wants (not forced to split an already 1 line height window) and if you have enough space for the splits it would should work out. Could you please try out this patch too (noequalalways2.patch) (this just removes the 1s)?

I can't seem to trigger the error unless I deliberately create a 1 line split and trigger completion.
What is strange to me about the whole thing is that by looking at the code, I don't see the situation where the plugin would try to open a second split while one already opened by it is open (so no calling two 1new in succession). And if you don't already have a one-line-split to begin the completion in, then opening and closing one-line-splits should not trigger this error as I understand it.

There are two places where during completion splits are opened, one in phpcomplete#GetClassContentsStructure, but between the 1new and the bw % lines I don't see the second how a second split would be created in here.
The other suspect is phpcomplete#GetCurrentNameSpace but that's also self-contained.
Do you frequently use a few-line-height splits and this is why you've run into this or it related to code you are completing? I feel like I'm missing something.

About :finally: Yeah, great suggestion, a try block would more robust in event of errors too (whitespace changes still make the diff hairy but oh well)!

[weirdan]

I'm a little hesitant about solving the problem this way since it will "ruin" the layout of your splits every time you invoke the completion

True, the whole point of having 'noequalalways' on is to have (and keep 😉 ) custom layout. So, disregard that 'working for me' comment.

I'm thinking of just removing the 1s from the 1news as an alternative

That's not an option, I'm afraid. It will still reduce the height of the split. So if you do enough completions, the split will get reduced to 1 line eventually - and then completions will fail. I can live with completions not working in a 1 line split as long as it does not wipe out other splits/buffers though.

The real problem for me is changing the height of the current split every time phpcomplete#getClassContentsStructure is called. As far as I understand, completion function has to run this function on every class in a class hierarchy (e.g. to get method return types). Suppose you have a class with 'fluent interface' and 5 ancestors (assuming 1 class per file coding standard): my understanding is that every time you complete a method call on an instance of this class your current split height is reduced by 5 lines (if you keep that 1new approach). If you started with quite generous 50 lines split you only need to complete 10 methods to reduce the split height to zilch.

What if you did :tab new instead of :1new? Separate tab could provide a kind of 'layout sandbox' where any changes would not affect the user's preferred layout.

[weirdan]

See #60 - this seems to fix both the problem with 1-line splits and splits changing their height on autocomplete invocation.

[complex857]

As far as I understand, completion function has to run this function on every class in a class hierarchy (e.g. to get method return types).

This is absolutely correct, however the way this reading-in-each-and-every-one-class process works is that it opens a buffer, grabs a relevant part and closes the buffer before going to the next class so as far as i could tell, there should be no two open buffer at the same time.

What if you did :tab new instead of :1new? Separate tab could provide a kind of 'layout sandbox' where any changes would not affect the user's preferred layout.

I was wondering the same thing, as i can see from #60 it should work as expected (at least the test didn't catch anything). I'm not sure if it's slower then just opening buffers, if you are on lazyredraw you should not notice it either way.

[weirdan]

opens a buffer, grabs a relevant part and closes the buffer before going to the next class so as far as i could tell, there should be no two open buffer at the same time.

And after this current split height is reduced by 2 lines. So it's only a matter of number of invocations to reduce the split height to nothing.

[complex857]

I was under the impression that vim will reclaim the space after the plugin-opened split is closed, and the buffer - where we were before the spliting/closing started - ends up with the same size.
At least this is what seems to be happening, when I manually open a few split, set noequalalways and run :below 1new and then :bw %.

[weirdan]

Doesn't happen for me: https://www.youtube.com/watch?v=8zIYrev8r04
I used :q in this video, but :bw % behaves in the same way.

[complex857]

Oh, I see. I really have to get into the habit of vim -u NONE testing, I have splitbelow set in my normal vimrc and that's make the difference here.

Thank you for taking the time explaining this to me!

[complex857]

@weirdan: Unfortunately the tab-opening type of fix causing other problems so I've had to revert it.
The good news is that I've added an explicit resize to the plugin to fix your issue and devised a test which should catch window changes with noequalalways enabled, so it should stay fixed.

Thanks to your previous explanation I'm pretty confident that I've got this fix right, but please try out the new master version yourself too.