Optimize query_one#5948
Merged
Merged
Conversation
Member
Author
|
@TomJGooding Want to take a gander? |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a special-case optimization for query_one when using a simple ID selector, routing those lookups through an indexed BFS lookup instead of a full selector parse.
- Add
is_id_selectorhelper (with caching) in the CSS parser. - Implement
walk_breadth_search_idin the DOM walker for fast ID-based lookups. - Integrate the new search path into
query_oneand update imports. - Add unit tests for the new
is_id_selectorfunction.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/css/test_parse.py | Import is_id_selector and add parametrized tests for ID selectors |
| src/textual/walk.py | Add walk_breadth_search_id for efficient BFS ID search |
| src/textual/dom.py | Hook ID-search path into query_one and update related imports |
| src/textual/css/parse.py | Define is_id_selector (regex + LRU cache) and adjust imports |
Comments suppressed due to low confidence (4)
tests/css/test_parse.py:1278
- [nitpick] Consider adding test cases for ID selectors including hyphens or underscores (e.g. '#foo-bar', '#foo_bar') and uppercase letters to ensure full coverage of valid IDENTIFIER patterns.
"selector, expected",
src/textual/walk.py:142
- [nitpick] The function name 'walk_breadth_search_id' is somewhat verbose; consider renaming to something more concise like 'find_by_id_bfs' to improve clarity.
def walk_breadth_search_id(
src/textual/walk.py:147
- In the docstring, correct the comparative 'that' to 'than': 'more efficient than ...' for grammatical accuracy.
This is more efficient that [walk_breadth_first][textual.walk.walk_breadth_first] for this special case, as it can use an index.
src/textual/css/parse.py:43
- [nitpick] The escape of '#' in a raw string (r"\#") is unnecessary; it can be simplified to r"#" to improve readability.
RE_ID_SELECTOR = re.compile(r"\#" + IDENTIFIER)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A special case optimization for
query_onewith a simple id selector.Previously
widget.query_one("#foo")would search each node in turn until it found a matching ID. In most cases this is quick enough, but here are potential pathological cases where it could be slow.This update uses the index on the node list to make this much faster.