neomake icon indicating copy to clipboard operation
neomake copied to clipboard

Height of Location List is Determined By Output of First Maker to Finish

Open jfelchner opened this issue 7 years ago • 22 comments

If I have multiple makers which each spit out their own output, if the first maker to finish only has one error, but the next maker has 5 errors, the height of the locationlist window is 1 rather than 6.

I would like to see the quickfix/locationlist window be adjusted after each maker is finished to make sure the height is correct.

Additionally, if the scenario above plays out and the initial error returned by the first maker is fixed, running :Neomake again does not resize the quickfix/locationlist window either.

jfelchner avatar Jun 07 '18 21:06 jfelchner

I feel like this is more of a bug than an enhancement.

jfelchner avatar Jun 07 '18 21:06 jfelchner

GitMate.io thinks possibly related issues are https://github.com/neomake/neomake/issues/1004 (Prevent automatically jumping to location list when errors are returned by a maker), https://github.com/neomake/neomake/issues/741 (Quickfix/location list should not be cleared when starting makers), https://github.com/neomake/neomake/issues/39 (Add Quickfix/Location List Settings), https://github.com/neomake/neomake/issues/1133 (Move list of makers from wiki to help), and https://github.com/neomake/neomake/issues/182 (Location list populated, but quickfix list not populated).

neomake-bot avatar Jun 07 '18 21:06 neomake-bot

You must be using some plugin for resizing the quickfix window already, vim-qf maybe? Try https://github.com/blueyed/vim-qf_resize instead / additionally.

blueyed avatar Jun 18 '18 16:06 blueyed

@blueyed nope. I cleared out all plugins except neomake, went to a bare init.vim and it still does it.

jfelchner avatar Jun 18 '18 17:06 jfelchner

It seems clear from the code that this is how it's written to behave.

jfelchner avatar Jun 18 '18 17:06 jfelchner

Ah, I see now - forgot that we had that code there: https://github.com/neomake/neomake/blob/62d33650a07c5a4291946defe86dab9f19056d8c/autoload/neomake.vim#L910-L926

Thanks for looking at the code also - I think https://github.com/neomake/neomake/pull/1974 should improve this. Can you test/review it, please?

blueyed avatar Jun 18 '18 19:06 blueyed

@blueyed I completely missed this! But thank you for fixing so quickly and I’ll respond back here if there are any problems! 👍🏻❤️

jfelchner avatar Jun 26 '18 22:06 jfelchner

@jfelchner I've just noticed that this causes an issue with https://github.com/blueyed/vim-qf_resize, which handles automatic resizing. I'm now using Neomake's method myself (had something custom before).

E.g. after lwindow 2 for two lines of output from the first job, vim-qf_resize is triggered and adjusts the size (or keeps it), but then later Neomake now would do resize 6 with 4 new entries, and there appears to be no autocmd triggered for this where vim-qf_resize could react upon.

The result is a qf window of height 6, whereas it should have been 2.

Some good fix already might be for Neomake to only do the resize if the size is still the same, but that would not help when vim-qf_resize likes the initial height of 2 lines.

The ultimate solution would be for Vim to gain a WinResized autocommand probably.

What do you think?

blueyed avatar Jul 17 '18 23:07 blueyed

As for Neomake and the common use case I could also imagine to delay the initial lwindow: it could be done unconditionally only at the end of all jobs, but since that might take some time there could be a timer to trigger it after e.g. 1s at the latest. And as for vim-qf_resize: it could hook into NeomakeFinished itself, to handle the final resizing.

blueyed avatar Jul 17 '18 23:07 blueyed

Basically as long as it works and I don’t have to install yet another plugin, I’m fine with whatever solution you come up with. 👍🏻

jfelchner avatar Jul 18 '18 17:07 jfelchner

Yeah.. Would be good if you could try https://github.com/neomake/neomake/pull/2011 and provide feedback there.

blueyed avatar Jul 18 '18 21:07 blueyed

@blueyed giving it a shot. I'll ping back if I have any problems.

jfelchner avatar Jul 20 '18 05:07 jfelchner

Seems to be working for me thus far? It's hard to tell because it's hard to notice if it's only one maker returning results.

jfelchner avatar Jul 28 '18 20:07 jfelchner

@jfelchner Thanks for the feedback. I am a bit uncertain about if the complexity of it is good, and thoiught about using g:neomake_open_list_resize_existing = 0 by default, too. Also https://github.com/neomake/neomake/commit/3d89088f70699947f5b091face520136d269bfff#r29870808.

blueyed avatar Jul 28 '18 21:07 blueyed

I want to throw one other thing into the mix here, although I can create a new issue if we want. Having this behavior supersede the value of g:neomake_list_height makes reading long errors difficult. They can end up looking like this:

image

Where there is much more context on the wrapped lines. I'd prefer a user set value of g:neomake_list_height take precedence here.

keith avatar Aug 21 '18 17:08 keith

@blueyed I'm recently continuing to have issues now even when there's only one maker that returns values. I have no idea what's going on.

Is the complexity here that you're trying to make it work for people who have both this and qf_resize installed at the same time?

I'd really like to just get this working and then deal with the (I'm assuming much) smaller subset of people who have both plugins installed.

jfelchner avatar Sep 01 '18 17:09 jfelchner

@jfelchner You are not using https://github.com/neomake/neomake/pull/2011 then, are you? (but have tried it back then, right?)

I think it adds too much complexity in the first place (i.e. two new settings), and we should rather maybe find a better for this in general. The initial fix (https://github.com/neomake/neomake/pull/1974) could have been done through some custom callback/config instead. But I also see that having this in the core is good after all.

I'm recently continuing to have issues now even when there's only one maker that returns values. I have no idea what's going on.

It would be good to get the logs for when that happens. I.e. please use let g:neomake_logfile = '/tmp/neomake.log and then look at it / forward it here for when something goes wrong.

@keith So your problem is that you have wrapping enabled for the quickfix list, but the resizing does not honor visible lines it seems. I've created https://github.com/neomake/neomake/issues/2073 for this.

blueyed avatar Sep 01 '18 20:09 blueyed

@blueyed I tried out your branch.

It works for the initial opening of the location list -- both makers affect the split height.

However, I continue making syntax errors, the height of the split remains the same. Is it possible to resize the split automatically upon each update of the location list?

FWIW, I would also welcome a change that g:neomake_list_height is respected even if the number of errors is less than that.

jcao219 avatar Nov 02 '18 08:11 jcao219

I have this behaviour even using one linter. Procedure:

  • I edit a python file, save it, flake8 is run automatically, no errors
  • I code some lines, introducing an issue
  • Save the file, flake8 is run, the window below opens with one line height, showing me the issue
  • I keep coding, introducing three more issues
  • Save the file, flake8 is run, the window below stays with its height in 1, it should be 4 lines high!

BTW, having a setting or some for the window height to always be the same size when opened would solve this (and, incidentally, is the behaviour I prefer... shall I open other bug for it?).

facundobatista avatar Feb 14 '20 11:02 facundobatista

@facundobatista The related code is here: https://github.com/neomake/neomake/blob/acbbd0e0ce2277c33926c189d0f54825e2ed59d3/autoload/neomake.vim#L981-L1051 (and quite complex already ;)) I suggest debugging/looking at it via the logfile / adding echoms to see what is going on in your case. IIRC your case should be covered/work.

blueyed avatar Feb 14 '20 16:02 blueyed

These are the logs for the exposing case I described above: http://linkode.org/#cjcIUq1SqCYajqYHjjK0j2 (starting with a clean file, adding one lint issue, then adding two more)

facundobatista avatar Feb 15 '20 12:02 facundobatista

BTW, I just hacked that code and in line 985 I left:

let height = height

Which opens the window at full height if there is a problem (no window if there is not). I really like this behaviour.

Maybe we could have another config parameter for this, or force to the full size if height is negative? Like:

let g:neomake_list_height = 5
let g:neomake_list_full_height = true

or just

let g:neomake_list_height = -5

facundobatista avatar Feb 15 '20 12:02 facundobatista