Skip to content

Fix some small list -> Iterable/Sequence typing issues #1590

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.

Sign up for GitHub

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Fix some small list -> Iterable/Sequence typing issues #1590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7209c72
Change to covariant typing
CarliJoy Mar 10, 2022
bb055b6
Accept any Iterable of words
CarliJoy Mar 10, 2022
862875a
Use Iterable only for typing
CarliJoy Mar 10, 2022
8bdaf35
Update base.py
CarliJoy Mar 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions prompt_toolkit/completion/word_completer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Callable, Iterable, List, Mapping, Optional, Pattern, Union
from typing import Callable, Iterable, Mapping, Optional, Pattern, Union

from prompt_toolkit.completion import CompleteEvent, Completer, Completion
from prompt_toolkit.document import Document
Expand Down Expand Up @@ -31,7 +31,7 @@ class WordCompleter(Completer):

def __init__(
self,
words: Union[List[str], Callable[[], List[str]]],
words: Union[Iterable[str], Callable[[], Iterable[str]]],
Copy link
Member

Choose a reason for hiding this comment

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

Please make this at least a Sequence.

Otherwise, users are going to pass in a generator of strings, and wonder why the second time, they're not getting any completions.

Is there a reason it should be Iterable here?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review @jonathanslenders

Nope. I was just going to use the lowest possible. You are right a Sequence would be more fitting.

I stopped working on this PR as some other troubles occured (see failing tests)
Once I found time to resolve them, I will undraft the PR and ping you.

ignore_case: bool = False,
display_dict: Optional[Mapping[str, AnyFormattedText]] = None,
meta_dict: Optional[Mapping[str, AnyFormattedText]] = None,
Expand Down
11 changes: 7 additions & 4 deletions prompt_toolkit/formatted_text/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import TYPE_CHECKING, Any, Callable, Iterable, List, Tuple, Union, cast
from typing import TYPE_CHECKING, Any, Callable, List, Iterable, Sequence, Tuple, Union, cast

from prompt_toolkit.mouse_events import MouseEvent

Expand All @@ -8,6 +8,7 @@
__all__ = [
"OneStyleAndTextTuple",
"StyleAndTextTuples",
"StyleAndTextTuplesType",
"MagicFormattedText",
"AnyFormattedText",
"to_formatted_text",
Expand All @@ -21,9 +22,11 @@
Tuple[str, str], Tuple[str, str, Callable[[MouseEvent], None]]
]

# List of (style, text) tuples.
# List (covariant version of list) of (style, text) tuples
# Not there is a subclass of this
StyleAndTextTuples = List[OneStyleAndTextTuple]

# For typing stuff, we prefer an Iterable as it is more flexible
StyleAndTextTuplesType = Iterable[OneStyleAndTextTuple]
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that Iterable is desirable, because it means it's not indexable and possibly, can only be consumed once.
Without reading again through all of the prompt_toolkit codebase, I think Sequence is at least what we need.


if TYPE_CHECKING:
from typing_extensions import TypeGuard
Expand All @@ -41,7 +44,7 @@ def __pt_formatted_text__(self) -> StyleAndTextTuples:
AnyFormattedText = Union[
str,
"MagicFormattedText",
StyleAndTextTuples,
StyleAndTextTuplesTypes,
# Callable[[], 'AnyFormattedText'] # Recursive definition not supported by mypy.
Callable[[], Any],
None,
Expand Down