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

PR: Select full floating point numbers by double-clicking them #22728

Merged
merged 7 commits into from
Nov 1, 2024

Conversation

athompson673
Copy link
Contributor

Improve double click to select functionality from default to allow selecting floating point numbers, etc by parsing the line with the tokenize stdlib module and selecting entire NUMBER tokens.

Description of Changes

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Included a screenshot or animation (if affecting the UI, see Licecap)

Issue(s) Resolved

Fixes #22207

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: athompson673

Improve double click to select functionality from default to allow selecting floating point numbers, etc by parsing the line with the tokenize stdlib module and selecting entire NUMBER tokens.
@pep8speaks
Copy link

pep8speaks commented Oct 25, 2024

Hello @athompson673! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-11-01 17:18:51 UTC

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @athompson673 for opening your again PR against master!

I only left some minor style suggestions for you. In case you haven't used Github reviews before, you can submit all of them in a single commit by adding them to the batch, as described here.

spyder/widgets/mixins.py Outdated Show resolved Hide resolved
spyder/widgets/mixins.py Outdated Show resolved Hide resolved
spyder/widgets/mixins.py Outdated Show resolved Hide resolved
spyder/widgets/mixins.py Outdated Show resolved Hide resolved
spyder/widgets/mixins.py Show resolved Hide resolved
@ccordoba12 ccordoba12 changed the title Select full floating point number by double-clicking PR: Select full floating point numbers by double-clicking them Oct 25, 2024
@ccordoba12 ccordoba12 added this to the v6.1.0 milestone Oct 25, 2024
@athompson673 athompson673 marked this pull request as draft October 25, 2024 16:03
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
@athompson673
Copy link
Contributor Author

athompson673 commented Oct 25, 2024

@ccordoba12 just ran into an issue with double clicking a docstring... I'll evaluate how to fix

image

I'm potentially trying to use tokenize in a way it is not intended (for a single line rather than an entire document)

Strip quotes to prevent tokenizer from trying to emit STRING tokens because we want to interpret numbers inside strings. This solves an EOF error trying to double click a line with opening or closing triple quotes as well.
Handle more exceptions raised by tokenize._tokenize
@athompson673 athompson673 marked this pull request as ready for review October 25, 2024 17:16
@athompson673
Copy link
Contributor Author

I figured after finding and solving that last issue, it would be a good idea to try to actually confirm there wouldn't be any more hidden problems. I wasn't sure how to add actual tests for automated testing, but I broke out some of the logic of the newly edited function, and ran a test against every possible double click position in all the .py files in a spyder 6.0.1 installation. No errors raised:

from pathlib import Path

from io import StringIO
from token import NUMBER
from tokenize import generate_tokens, TokenError

from time import perf_counter

def get_number_selection(line: str, pos: int) -> tuple[int, int]|tuple[None, None]:
    """
    Return start and end position of number literal at pos in line.
    If text at pos in line is not a number literal, return (None, None)
    """
    # Strip quotes to prevent tokenizer from trying to emit STRING tokens
    #   because we want to interpret numbers inside strings. This solves
    #   an EOF error trying to double click a line with opening or closing
    #   triple quotes as well.
    text = line.replace('"', ' ').replace("'", ' ')
    readline = StringIO(text).read
    
    try:
        for t_type, _, start, end, _ in generate_tokens(readline):
            if t_type == NUMBER and start[1] <= pos <= end[1]:
                return start[1], end[1]
            elif start[1] > pos:
                break

    except TokenError:
        # Ignore 'EOF in multi-line statement' from tokenize._tokenize
        # IndentationError should be impossible from tokenizing one line
        pass
    
    return None, None  # start and end of NUMBER not found

count = 0
start_time = perf_counter()
for p in Path(r'D:\python\py311\Lib\site-packages\spyder').glob("**/*.py"):
    with p.open(encoding="utf-8") as f:
        print(p)
        for line in f:
            for i in range(len(line)):
                start, end = get_number_selection(line, i)
                if start is None:
                    assert end is None
                elif end is None:
                    raise ValueError("1")
                else:
                    assert type(start) is int
                    assert type(end) is int
                    assert start < end
                    assert end <= len(line)
                count += 1
print("\n{count}, {perf_counter() - start_time}")

>>> %runfile D:/scripts/untitled0.py --wdir --post-mortem
5320059, 81.67973330011591
    

It also shows the function shouldn't slow down double click handling appreciably; about 15μs per call for me.

athompson673 and others added 2 commits November 1, 2024 09:46
This is for the IPython console, but the behavior for the editor is the
same given that this functionality was added to BaseEditMixin.
@ccordoba12
Copy link
Member

I wasn't sure how to add actual tests for automated testing, but I broke out some of the logic of the newly edited function, and ran a test against every possible double click position in all the .py files in a spyder 6.0.1 installation. No errors raised

That's great! Thanks for testing that @athompson673.

I'll give you a hand to implement a simple test for the new behavior you added, but just for the IPython console because it should be the same for the editor.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this @athompson673!! It's a really cool contribution!

@ccordoba12 ccordoba12 merged commit 3db4320 into spyder-ide:master Nov 1, 2024
17 checks passed
@ccordoba12
Copy link
Member

Note: This will be included in 6.1 (to be released in six months or so) because it's a new feature.

@athompson673 athompson673 deleted the patch-1 branch November 2, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Select full floating point numbers by double-clicking on them
3 participants