Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add setting for dragging to select text. #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nriley
Copy link
Contributor

@nriley nriley commented Feb 28, 2023

For use in apps (e.g., some PDF viewers) that do not support clicking to set an insertion point position, nor using the keyboard to select.

For use in apps (e.g., some PDF viewers) that do not support clicking to
set an insertion point position, nor using the keyboard to select.
Copy link
Owner

@wolfmanstout wolfmanstout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for putting this together!

@@ -267,6 +267,8 @@ def move_text_cursor_to_words_generator(
timestamp: Optional[float] = None,
click_offset_right: int = 0,
hold_shift: bool = False,
start_drag: bool = False,
end_drag: bool = False,
Copy link
Owner

@wolfmanstout wolfmanstout Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unused. I suspect the reason why it is still working is brittle: click() is probably implemented as "click_down(); click_up()", and so the click_down() is a no-op and click() has the effect of click_up(). I don't think we want to rely on that; it's not hard to imagine that calling click() when the button is already down could become an error, or it could call click_up() at the start, etc.

I'd suggest adding an end_drag arg to _move_text_cursor_to_word_locations, having that perform click_up(), and then you can pass this along to that.

@@ -553,6 +580,8 @@ def select_text_generator(
return None
finally:
self.keyboard.shift_up()
if drag:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be pulled out into a separate & larger try/finally block, so that any early returns in the function won't leave the mouse in a bad (down) state. Since this risk is introduced very early in the function with the call to move_text_cursor_to_words_generator, the simplest might be to simply rename this function with an "_unsafe" suffix and then reimplement this whole function as simply a try block that wraps the call to the unsafe() function and a finally block that calls click_up().

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.

2 participants