sjbach / lusty

LustyExplorer / LustyJuggler for Vim
http://www.vim.org/scripts/script.php?script_id=1890
256 stars 30 forks source link

Don't resize $curwin when setting max window height #76

Closed dlobraico closed 12 years ago

dlobraico commented 12 years ago

This should fix the problem reported in Issue #52, where the bottom-most of 2 or more horizontal splits is resized to fit the whole screen minus 1 line for each of the splits above it. It seems like the issue was simple that the calculation of the max window height was setting the current window size to the max size. I dropped all of the variable shuffling in Display.max_height and simple set it to VIM::MOST_POSITIVE_INTEGER outright. I don't think this should be a problem, unless you were doing Display.max_height to do something other than I expect.

Sorry it took me so long to do this, I didn't have a chance to look at it until a few minutes ago.

Cheers!

sjbach commented 12 years ago

The variable acrobatics in Display.max_height do serve a purpose, though it's egregiously undocumented - it computes an accurate maximum display window height so that the truncation indicator will display and so that only a subset of files will be shown in the window, i.e. the ones for which we have space to display. It's intended to be a side-effect free function, but obviously it's not (hence issue #52).

Your change does fix issue #52, but it removes the truncation indicator and lists all matches in the explorer window instead of the displayable subset. To see what I mean, navigate to e.g. /usr/bin (or some other directory with many files) before and after your change.

I think the right fix is to modify Display.max_height to use Vim's winrestcmd() or a mix of winsaveview() / winrestview() to properly save and restore the window sizes while computing the displayable height. I didn't know about these functions when I first wrote the plugin.

dlobraico commented 12 years ago

Ah. I see that now. Sorry for that, and thanks for the info. I'll make another pull request that addresses this later today.

On Jun 24, 2012, at 6:51 PM, Stephen Bachreply@reply.github.com wrote:

The variable acrobatics in Display.max_height do serve a purpose, though it's egregiously undocumented - it computes an accurate maximum display window height so that the truncation indicator will display and so that only a subset of files will be shown in the window, i.e. the ones for which we have space to display. It's intended to be a side-effect free function, but obviously it's not (hence issue #52).

Your change does fix issue #52, but it removes the truncation indicator and lists all matches in the explorer window instead of the displayable subset. To see what I mean, navigate to e.g. /usr/bin (or some other directory with many files) before and after your change.

I think the right fix is to modify Display.max_height to use Vim's winrestcmd() or a mix of winsaveview() / winrestview() to properly save and restore the window sizes while computing the displayable height. I didn't know about these functions when I first wrote the plugin.


Reply to this email directly or view it on GitHub: https://github.com/sjbach/lusty/pull/76#issuecomment-6537134

dlobraico commented 12 years ago

I just added two commits that seem to fix the issue for real. Take a look and let me know what you think or if you have any questions.

sjbach commented 12 years ago

Looks good, thanks. I left a question in one of your commits.

sjbach commented 12 years ago

This has been merged. Thanks Dominick!