Allow soft-wrapping of lines in the signature help window #4252
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4252 +/- ##
==========================================
+ Coverage 89.80% 89.88% +0.07%
==========================================
Files 37 37
Lines 4758 4763 +5
==========================================
+ Hits 4273 4281 +8
+ Misses 485 482 -3 |
bstaletic
force-pushed
the
sig_help_wrap
branch
from
987b72e
to
a08e683
Compare
There was a problem hiding this comment.
I tried this out and I quite liked it but there were cases where it was I think net worse than what we have now.
This is a shame because it's just really hard to solve for both problems of horizontal positioning being "sane" and inducing as much info on the screen as possible. tricky tricky.
Reviewable status: 0 of 2 LGTMs obtained
|
Right, the shifting to the left is broken in this pull request. |
|
Thank you for working on this. Today I was searching for how to enable this feature only to find out that it doesn't exist yet. |
|
I'll be getting back to this pull request, this week. |
bstaletic
force-pushed
the
sig_help_wrap
branch
from
a08e683
to
c2155a6
Compare
|
I think I got this working. |
bstaletic
force-pushed
the
sig_help_wrap
branch
from
c2155a6
to
5c4d427
Compare
|
This latest update works well for me. I tested it with those overly long Python function signatures, which now wrap properly. It works like I would have expected YouCompleteMe's signature help window to work out of the box. |
bstaletic
force-pushed
the
sig_help_wrap
branch
2 times, most recently
from
d6e059f
to
710acf5
Compare
This looks ugly, but at least no data is lost due to lines going off screen.
|
There's one minor cosmetic issue left: when the text wraps, there should be a space appended at the end of each wrapping line. At least that's the style all other pop-up windows have. I'm not sure if that is possible here though. In the example screenshot, one space would be inserted after "Opt" and "flus" each to have the right pop-up border width match the left one.
|
|
Never mind, I just noticed that this is normal wrapping behavior even for docstrings. Which brings me to the question if there is anything else left that is stopping this pull request from being merged. I have been trying this feature for quite some time now and I would love to see it included in YCM by default. |
There was a problem hiding this comment.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)
python/ycm/signature_help.py line 162 at r4 (raw file):
col = int( screen_pos[ 'curscol' ] ) - 2 # Vim stops shifting the popup to the left if we turn on soft-wrapping.
I think this is just lazy programming by the absolute waster that implemented this left-shifting in Vim.
https://github.com/vim/vim/blob/master/src/popupwin.c#L1437-L1446
Part of me is like "we should change the Vim behaviour rather than work it out here because, well, that was the whole reason that I implemented it in Vim rather than here"....
Basically by enabling 'wrap' we have to implement that PR's logic in here instead: https://github.com/vim/vim/blob/master/src/popupwin.c#L1447-L1466
python/ycm/signature_help.py line 165 at r4 (raw file):
# Instead, we want to first shift the popup to the left and then # and then turn on wrapping. max_line_length = len( max(
marginally more efficient (and simpler) to write:
max( len( item[ 'text' ] ) for item in buf_lines )
python/ycm/signature_help.py line 169 at r4 (raw file):
key = lambda item: len( item[ 'text' ] ) )[ 'text' ] ) vim_width = vimsupport.GetIntValue( '&columns' ) line_available = vim_width - int( screen_pos[ 'col' ] )
Shouldn't this be col here rather than screen_pos[ 'col' ] ? (assuming that col > 0).
in the previous stanza we offset the col position according to some padding and positioning logic. I think that this calculation needs to account for both sides of the padding.
PR Prelude
Thank you for working on YCM! :)
Please complete these steps and check these boxes (by putting an
xinsidethe brackets) before filing your PR:
rationale for why I haven't.
actually perform all of these steps.
Why this change is necessary and useful
Fixes #4171
Or at least proposes a fix. Here's a short demo:
https://asciinema.org/a/UJlR80blt6OvuwUIqb4swnWUW
This looks ugly, but at least no data is lost due to lines going off screen.
This change is