Skip to content

Conversation

@Arrowbox
Copy link
Contributor

For issue #168
This fixes some of the mouse scrolling issues with Window. And adds some unit tests to check them.

  • Can't scroll to end with wrap_lines
  • Can't scroll past end with allow_scroll_beyond_bottom

Still needs work to get scrolling up to work. It currently stops scrolling up when the amount of lines needed to add a line from the top would force the cursor off screen after moving up 1 line. Or if the cursor is not on the last line of the window but on the last line of the visible lines.

@Arrowbox
Copy link
Contributor Author

My mistake, this is actually for issue #686

@codecov-io
Copy link

Codecov Report

Merging #693 into master will increase coverage by 0.75%.
The diff coverage is 99.4%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #693      +/-   ##
==========================================
+ Coverage   71.21%   71.97%   +0.75%     
==========================================
  Files         142      139       -3     
  Lines       13287    13062     -225     
==========================================
- Hits         9462     9401      -61     
+ Misses       3825     3661     -164
Impacted Files Coverage Δ
prompt_toolkit/layout/containers.py 69.78% <100%> (+2.66%) ⬆️
tests/test_mouse_scroll.py 99.35% <99.35%> (ø)
prompt_toolkit/eventloop/select.py 60.34% <0%> (-12.07%) ⬇️
prompt_toolkit/input/defaults.py 45.45% <0%> (-6.07%) ⬇️
prompt_toolkit/utils.py 85.4% <0%> (-5.84%) ⬇️
prompt_toolkit/eventloop/defaults.py 65% <0%> (-5%) ⬇️
prompt_toolkit/output/color_depth.py 86.95% <0%> (-4.35%) ⬇️
prompt_toolkit/eventloop/future.py 72.34% <0%> (-2.13%) ⬇️
prompt_toolkit/eventloop/async_generator.py 83.58% <0%> (-1.5%) ⬇️
prompt_toolkit/output/base.py 95.94% <0%> (-1.36%) ⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a22b88...3594662. Read the comment docs.

@Arrowbox
Copy link
Contributor Author

@jonathanslenders It looks like there is some error with Appveyor and Python 2.6. It looks like some sort of SSL error with respect to pypi. My rudimentary googling seems to indicate an issue with python 2.6 and pip. Any thoughts? I don't think my changes would cause it.

@jonathanslenders
Copy link
Member

Don't worry about Python 2.6, especially on Windows there's no reason to use it (IMHO). It shouldn't be because of these changes.

Thanks for the pull request, I hope to get some time soon to review and merge it!

@Arrowbox
Copy link
Contributor Author

I've been working on the _scroll_up implementation and can't seem to find a work around for one particular issue. The problem is that the Window._mouse_handler can be called multiple times between calls toWindow.write_to_screen. The _mouse_handler uses Window.render_info that is generated for each rendering pass. Included in the render_info is the cursor_position and ui_content.cursor_position.

Both _scroll_down and _scroll_up use the positions to determine if a call to self.content.move_cursor_up|down should be made. However, any call to those are not reflected in self.render_info until after the next render. Multiple calls to the one of the scroll functions only end up with a modified self.vertical_scroll.

I think a possible long term path would be to make the mouse scrolling functions set a self.window_scroll flag to True. The Window._scroll function could then check the flag to decide if the scroll offsets should be adjusted to fit the cursor position or if the cursor position should be adjusted to fit the scroll window. What do you think?

@jonathanslenders
Copy link
Member

Great analysis! Thanks a lot. I haven't had the time yet to dive into this issue, I will. It's something I think needs to be fixed.

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

Successfully merging this pull request may close these issues.

3 participants