From 1a43c16c328227c02aed4a7a8dd099d73997d13f Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 14 Sep 2022 09:56:11 +0100 Subject: [PATCH 1/6] fixes for variable css errors --- src/textual/app.py | 11 ++++---- src/textual/cli/previews/borders.py | 2 +- src/textual/css/_help_renderables.py | 6 +++-- src/textual/css/_styles_builder.py | 32 +++++++++++----------- src/textual/css/parse.py | 22 +++++++-------- src/textual/css/stylesheet.py | 40 ++++++++++++++-------------- src/textual/css/tokenizer.py | 2 ++ src/textual/widgets/_button.py | 2 +- 8 files changed, 61 insertions(+), 56 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index 13ab075ee..d11726bf2 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -28,7 +28,7 @@ import rich.repr from rich.console import Console, RenderableType from rich.measure import Measurement from rich.protocol import is_renderable -from rich.segment import Segments +from rich.segment import Segment, Segments from rich.traceback import Traceback from . import ( @@ -1016,10 +1016,11 @@ class App(Generic[ReturnType], DOMNode): is_renderable(renderable) for renderable in renderables ), "Can only call panic with strings or Rich renderables" - pre_rendered = [ - Segments(self.console.render(renderable, self.console.options)) - for renderable in renderables - ] + def render(renderable: RenderableType) -> list[Segment]: + segments = list(self.console.render(renderable, self.console.options)) + return segments + + pre_rendered = [Segments(render(renderable)) for renderable in renderables] self._exit_renderables.extend(pre_rendered) self._close_messages_no_wait() diff --git a/src/textual/cli/previews/borders.py b/src/textual/cli/previews/borders.py index eeb80ce1b..d2d4123fe 100644 --- a/src/textual/cli/previews/borders.py +++ b/src/textual/cli/previews/borders.py @@ -53,7 +53,7 @@ class BorderApp(App): def on_button_pressed(self, event: Button.Pressed) -> None: self.text.styles.border = ( event.button.id, - self.stylesheet.variables["secondary"], + self.stylesheet._variables["secondary"], ) self.bell() diff --git a/src/textual/css/_help_renderables.py b/src/textual/css/_help_renderables.py index b1ba7d24b..ca2307f22 100644 --- a/src/textual/css/_help_renderables.py +++ b/src/textual/css/_help_renderables.py @@ -2,8 +2,8 @@ from __future__ import annotations from typing import Iterable +import rich.repr from rich.console import Console, ConsoleOptions, RenderResult - from rich.highlighter import ReprHighlighter from rich.markup import render from rich.text import Text @@ -42,6 +42,7 @@ class Example: yield _markup_and_highlight(f" [dim]e.g. [/][i]{self.markup}[/]") +@rich.repr.auto class Bullet: """Renderable for a single 'bullet point' containing information and optionally some examples pertaining to that information. @@ -59,10 +60,11 @@ class Bullet: def __rich_console__( self, console: Console, options: ConsoleOptions ) -> RenderResult: - yield _markup_and_highlight(f"{self.markup}") + yield _markup_and_highlight(self.markup) yield from self.examples +@rich.repr.auto class HelpText: """Renderable for help text - the user is shown this when they encounter a style-related error (e.g. setting a style property to an invalid diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index 284d70cc7..b379e85d4 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -1,10 +1,16 @@ from __future__ import annotations from functools import lru_cache -from typing import cast, Iterable, NoReturn, Sequence +from typing import Iterable, NoReturn, Sequence, cast import rich.repr +from .._border import BorderValue, normalize_border_value +from .._duration import _duration_as_seconds +from .._easing import EASING +from ..color import Color, ColorParseError +from ..geometry import Spacing, SpacingDimensions, clamp +from ..suggestions import get_suggestion from ._error_tools import friendly_list from ._help_renderables import HelpText from ._help_text import ( @@ -33,34 +39,28 @@ from .constants import ( VALID_ALIGN_VERTICAL, VALID_BORDER, VALID_BOX_SIZING, - VALID_EDGE, VALID_DISPLAY, + VALID_EDGE, VALID_OVERFLOW, - VALID_VISIBILITY, - VALID_STYLE_FLAGS, VALID_SCROLLBAR_GUTTER, + VALID_STYLE_FLAGS, VALID_TEXT_ALIGN, + VALID_VISIBILITY, ) from .errors import DeclarationError, StyleValueError from .model import Declaration from .scalar import ( Scalar, - ScalarOffset, - Unit, ScalarError, + ScalarOffset, ScalarParseError, + Unit, percentage_string_to_float, ) -from .styles import DockGroup, Styles +from .styles import Styles from .tokenize import Token from .transition import Transition -from .types import BoxSizing, Edge, Display, Overflow, Visibility, EdgeType -from .._border import normalize_border_value, BorderValue -from ..color import Color, ColorParseError -from .._duration import _duration_as_seconds -from .._easing import EASING -from ..geometry import Spacing, SpacingDimensions, clamp -from ..suggestions import get_suggestion +from .types import BoxSizing, Display, Edge, EdgeType, Overflow, Visibility def _join_tokens(tokens: Iterable[Token], joiner: str = "") -> str: @@ -434,6 +434,7 @@ class StylesBuilder: process_padding_left = _process_space_partial def _parse_border(self, name: str, tokens: list[Token]) -> BorderValue: + border_type: EdgeType = "solid" border_color = Color(0, 255, 0) @@ -553,7 +554,7 @@ class StylesBuilder: self.styles._rules["offset"] = ScalarOffset(x, y) def process_layout(self, name: str, tokens: list[Token]) -> None: - from ..layouts.factory import get_layout, MissingLayout + from ..layouts.factory import MissingLayout, get_layout if tokens: if len(tokens) != 1: @@ -602,7 +603,6 @@ class StylesBuilder: if color is not None or alpha is not None: if alpha is not None: - color = (color or Color(255, 255, 255)).with_alpha(alpha) self.styles._rules[name] = color diff --git a/src/textual/css/parse.py b/src/textual/css/parse.py index 7ea876355..c9ba19fd2 100644 --- a/src/textual/css/parse.py +++ b/src/textual/css/parse.py @@ -301,9 +301,7 @@ def substitute_references( for _token in reference_tokens: yield _token.with_reference( ReferencedBy( - name=ref_name, - location=ref_location, - length=ref_length, + ref_name, ref_location, ref_length, token.code ) ) else: @@ -318,13 +316,10 @@ def substitute_references( variable_tokens = variables[variable_name] ref_location = token.location ref_length = len(token.value) - for token in variable_tokens: - yield token.with_reference( - ReferencedBy( - name=variable_name, - location=ref_location, - length=ref_length, - ) + ref_code = token.code + for _token in variable_tokens: + yield _token.with_reference( + ReferencedBy(variable_name, ref_location, ref_length, ref_code) ) else: _unresolved(variable_name, variables.keys(), token) @@ -336,6 +331,7 @@ def parse( css: str, path: str | PurePath, variables: dict[str, str] | None = None, + variable_tokens: dict[str, list[Token]] | None = None, is_default_rules: bool = False, tie_breaker: int = 0, ) -> Iterable[RuleSet]: @@ -349,7 +345,11 @@ def parse( is_default_rules (bool): True if the rules we're extracting are default (i.e. in Widget.DEFAULT_CSS) rules. False if they're from user defined CSS. """ - variable_tokens = tokenize_values(variables or {}) + + reference_tokens = tokenize_values(variables) if variables is not None else {} + if variable_tokens: + reference_tokens.update(variable_tokens) + tokens = iter(substitute_references(tokenize(css, path), variable_tokens)) while True: token = next(tokens, None) diff --git a/src/textual/css/stylesheet.py b/src/textual/css/stylesheet.py index d696525a8..12258fe10 100644 --- a/src/textual/css/stylesheet.py +++ b/src/textual/css/stylesheet.py @@ -2,10 +2,9 @@ from __future__ import annotations import os from collections import defaultdict -from functools import partial from operator import itemgetter from pathlib import Path, PurePath -from typing import Iterable, NamedTuple, cast +from typing import Iterable, NamedTuple, Sequence, cast import rich.repr from rich.console import Console, ConsoleOptions, RenderableType, RenderResult @@ -39,20 +38,15 @@ class StylesheetParseError(StylesheetError): class StylesheetErrors: - def __init__( - self, rules: list[RuleSet], variables: dict[str, str] | None = None - ) -> None: + def __init__(self, rules: list[RuleSet]) -> None: self.rules = rules self.variables: dict[str, str] = {} - self._css_variables: dict[str, list[Token]] = {} - if variables: - self.set_variables(variables) @classmethod def _get_snippet(cls, code: str, line_no: int) -> RenderableType: syntax = Syntax( code, - lexer="scss", + lexer="sass", theme="ansi_light", line_numbers=True, indent_guides=True, @@ -61,11 +55,6 @@ class StylesheetErrors: ) return syntax - def set_variables(self, variable_map: dict[str, str]) -> None: - """Pre-populate CSS variables.""" - self.variables.update(variable_map) - self._css_variables = tokenize_values(self.variables) - def __rich_console__( self, console: Console, options: ConsoleOptions ) -> RenderResult: @@ -105,7 +94,10 @@ class StylesheetErrors: title = Text.assemble(Text("Error at ", style="bold red"), path_text) yield "" yield Panel( - self._get_snippet(token.code, line_no), + self._get_snippet( + token.referenced_by.code if token.referenced_by else token.code, + line_no, + ), title=title, title_align="left", border_style="red", @@ -138,13 +130,20 @@ class Stylesheet: def __init__(self, *, variables: dict[str, str] | None = None) -> None: self._rules: list[RuleSet] = [] self._rules_map: dict[str, list[RuleSet]] | None = None - self.variables = variables or {} + self._variables = variables or {} + self.__variable_tokens: dict[str, list[Token]] | None = None self.source: dict[str, CssSource] = {} self._require_parse = False def __rich_repr__(self) -> rich.repr.Result: yield list(self.source.keys()) + @property + def _variable_tokens(self) -> dict[str, list[Token]]: + if self.__variable_tokens is None: + self.__variable_tokens = tokenize_values(self._variables) + return self.__variable_tokens + @property def rules(self) -> list[RuleSet]: """List of rule sets. @@ -183,7 +182,7 @@ class Stylesheet: Returns: Stylesheet: New stylesheet. """ - stylesheet = Stylesheet(variables=self.variables.copy()) + stylesheet = Stylesheet(variables=self._variables.copy()) stylesheet.source = self.source.copy() return stylesheet @@ -193,7 +192,8 @@ class Stylesheet: Args: variables (dict[str, str]): A mapping of name to variable. """ - self.variables = variables + self._variables = variables + self._variables_tokens = None def _parse_rules( self, @@ -222,7 +222,7 @@ class Stylesheet: parse( css, path, - variables=self.variables, + variable_tokens=self._variable_tokens, is_default_rules=is_default_rules, tie_breaker=tie_breaker, ) @@ -317,7 +317,7 @@ class Stylesheet: """ # Do this in a fresh Stylesheet so if there are errors we don't break self. - stylesheet = Stylesheet(variables=self.variables) + stylesheet = Stylesheet(variables=self._variables) for path, (css, is_defaults, tie_breaker) in self.source.items(): stylesheet.add_source( css, path, is_default_css=is_defaults, tie_breaker=tie_breaker diff --git a/src/textual/css/tokenizer.py b/src/textual/css/tokenizer.py index c7fb9183b..15dc90508 100644 --- a/src/textual/css/tokenizer.py +++ b/src/textual/css/tokenizer.py @@ -118,6 +118,7 @@ class ReferencedBy(NamedTuple): name: str location: tuple[int, int] length: int + code: str @rich.repr.auto @@ -209,6 +210,7 @@ class Tokenizer: message, ) iter_groups = iter(match.groups()) + next(iter_groups) for name, value in zip(expect.names, iter_groups): diff --git a/src/textual/widgets/_button.py b/src/textual/widgets/_button.py index 606cdf371..f13c5294f 100644 --- a/src/textual/widgets/_button.py +++ b/src/textual/widgets/_button.py @@ -36,7 +36,7 @@ class Button(Widget, can_focus=True): height: 3; background: $panel; color: $text; - border: none; + border: none; border-top: tall $panel-lighten-2; border-bottom: tall $panel-darken-3; content-align: center middle; From 314142e16156d741f116755e90d9b7cf95b51ae4 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 14 Sep 2022 10:03:46 +0100 Subject: [PATCH 2/6] test fix --- tests/css/test_parse.py | 68 ++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/tests/css/test_parse.py b/tests/css/test_parse.py index 3a9e43181..fed5db430 100644 --- a/tests/css/test_parse.py +++ b/tests/css/test_parse.py @@ -97,7 +97,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 4), - referenced_by=ReferencedBy(name="x", location=(0, 28), length=2), + referenced_by=ReferencedBy( + name="x", location=(0, 28), length=2, code=css + ), ), Token( name="declaration_end", @@ -191,7 +193,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 3), - referenced_by=ReferencedBy(name="x", location=(0, 27), length=2), + referenced_by=ReferencedBy( + name="x", location=(0, 27), length=2, code=css + ), ), Token( name="declaration_end", @@ -273,7 +277,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 4), - referenced_by=ReferencedBy(name="x", location=(1, 4), length=2), + referenced_by=ReferencedBy( + name="x", location=(1, 4), length=2, code=css + ), ), Token( name="variable_value_end", @@ -337,7 +343,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 4), - referenced_by=ReferencedBy(name="y", location=(2, 17), length=2), + referenced_by=ReferencedBy( + name="y", location=(2, 17), length=2, code=css + ), ), Token( name="whitespace", @@ -446,7 +454,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 4), - referenced_by=ReferencedBy(name="x", location=(1, 6), length=2), + referenced_by=ReferencedBy( + name="x", location=(1, 6), length=2, code=css + ), ), Token( name="whitespace", @@ -454,7 +464,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 5), - referenced_by=ReferencedBy(name="x", location=(1, 6), length=2), + referenced_by=ReferencedBy( + name="x", location=(1, 6), length=2, code=css + ), ), Token( name="number", @@ -462,7 +474,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 6), - referenced_by=ReferencedBy(name="x", location=(1, 6), length=2), + referenced_by=ReferencedBy( + name="x", location=(1, 6), length=2, code=css + ), ), Token( name="whitespace", @@ -542,7 +556,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(1, 4), - referenced_by=ReferencedBy(name="y", location=(2, 17), length=2), + referenced_by=ReferencedBy( + name="y", location=(2, 17), length=2, code=css + ), ), Token( name="whitespace", @@ -550,7 +566,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(1, 5), - referenced_by=ReferencedBy(name="y", location=(2, 17), length=2), + referenced_by=ReferencedBy( + name="y", location=(2, 17), length=2, code=css + ), ), Token( name="number", @@ -558,7 +576,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 4), - referenced_by=ReferencedBy(name="y", location=(2, 17), length=2), + referenced_by=ReferencedBy( + name="y", location=(2, 17), length=2, code=css + ), ), Token( name="whitespace", @@ -566,7 +586,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 5), - referenced_by=ReferencedBy(name="y", location=(2, 17), length=2), + referenced_by=ReferencedBy( + name="y", location=(2, 17), length=2, code=css + ), ), Token( name="number", @@ -574,7 +596,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 6), - referenced_by=ReferencedBy(name="y", location=(2, 17), length=2), + referenced_by=ReferencedBy( + name="y", location=(2, 17), length=2, code=css + ), ), Token( name="whitespace", @@ -582,7 +606,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(1, 8), - referenced_by=ReferencedBy(name="y", location=(2, 17), length=2), + referenced_by=ReferencedBy( + name="y", location=(2, 17), length=2, code=css + ), ), Token( name="number", @@ -590,7 +616,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(1, 9), - referenced_by=ReferencedBy(name="y", location=(2, 17), length=2), + referenced_by=ReferencedBy( + name="y", location=(2, 17), length=2, code=css + ), ), Token( name="whitespace", @@ -715,7 +743,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 4), - referenced_by=ReferencedBy(name="x", location=(1, 20), length=2), + referenced_by=ReferencedBy( + name="x", location=(1, 20), length=2, code=css + ), ), Token( name="declaration_end", @@ -845,7 +875,9 @@ class TestVariableReferenceSubstitution: path="", code=css, location=(0, 7), - referenced_by=ReferencedBy(name="x", location=(0, 26), length=2), + referenced_by=ReferencedBy( + name="x", location=(0, 26), length=2, code=css + ), ), Token( name="declaration_set_end", @@ -1134,7 +1166,9 @@ class TestParsePadding: class TestParseTextAlign: - @pytest.mark.parametrize("valid_align", ["left", "start", "center", "right", "end", "justify"]) + @pytest.mark.parametrize( + "valid_align", ["left", "start", "center", "right", "end", "justify"] + ) def test_text_align(self, valid_align): css = f"#foo {{ text-align: {valid_align} }}" stylesheet = Stylesheet() From 9ed3ce461656f10baceca7c1f3e56e884b186e1a Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 14 Sep 2022 10:42:07 +0100 Subject: [PATCH 3/6] fix excess scroll visible --- src/textual/widget.py | 8 +++++++- src/textual/widgets/_data_table.py | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/textual/widget.py b/src/textual/widget.py index 476b86c3a..3e38e82b5 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -1059,7 +1059,9 @@ class Widget(DOMNode): while isinstance(widget.parent, Widget) and widget is not self: container = widget.parent - scroll_offset = container.scroll_to_region(region, animate=animate) + scroll_offset = container.scroll_to_region( + region, spacing=widget.parent.gutter, animate=animate + ) if scroll_offset: scrolled = True @@ -1096,6 +1098,10 @@ class Widget(DOMNode): window = self.content_region.at_offset(self.scroll_offset) if spacing is not None: window = window.shrink(spacing) + + if window in region: + return Offset() + delta_x, delta_y = Region.get_scroll_to_visible(window, region) scroll_x, scroll_y = self.scroll_offset delta = Offset( diff --git a/src/textual/widgets/_data_table.py b/src/textual/widgets/_data_table.py index 57d7fb27b..5bf59fd54 100644 --- a/src/textual/widgets/_data_table.py +++ b/src/textual/widgets/_data_table.py @@ -557,7 +557,7 @@ class DataTable(ScrollView, Generic[CellType], can_focus=True): def _scroll_cursor_in_to_view(self, animate: bool = False) -> None: region = self._get_cell_region(self.cursor_row, self.cursor_column) - spacing = self._get_cell_border() + spacing = self._get_cell_border() + self.scrollbar_gutter self.scroll_to_region(region, animate=animate, spacing=spacing) def on_click(self, event: events.Click) -> None: From 147ead9d20597a6c1b6110d4b685454d6289e7de Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 14 Sep 2022 10:44:34 +0100 Subject: [PATCH 4/6] docstring --- src/textual/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/app.py b/src/textual/app.py index d11726bf2..2dfe783fc 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -1017,11 +1017,11 @@ class App(Generic[ReturnType], DOMNode): ), "Can only call panic with strings or Rich renderables" def render(renderable: RenderableType) -> list[Segment]: + """Render a panic renderables.""" segments = list(self.console.render(renderable, self.console.options)) return segments pre_rendered = [Segments(render(renderable)) for renderable in renderables] - self._exit_renderables.extend(pre_rendered) self._close_messages_no_wait() From fac1b273e8403024b6ac016ed5df299aae518d22 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 14 Sep 2022 10:45:57 +0100 Subject: [PATCH 5/6] superfluous --- src/textual/css/stylesheet.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/css/stylesheet.py b/src/textual/css/stylesheet.py index 12258fe10..c69147e35 100644 --- a/src/textual/css/stylesheet.py +++ b/src/textual/css/stylesheet.py @@ -4,7 +4,7 @@ import os from collections import defaultdict from operator import itemgetter from pathlib import Path, PurePath -from typing import Iterable, NamedTuple, Sequence, cast +from typing import Iterable, NamedTuple, cast import rich.repr from rich.console import Console, ConsoleOptions, RenderableType, RenderResult From 6852dad4edb40356dea3de928e8c4c9e16381b99 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Wed, 14 Sep 2022 10:46:27 +0100 Subject: [PATCH 6/6] change lexer --- src/textual/css/stylesheet.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/css/stylesheet.py b/src/textual/css/stylesheet.py index c69147e35..b5e964383 100644 --- a/src/textual/css/stylesheet.py +++ b/src/textual/css/stylesheet.py @@ -46,7 +46,7 @@ class StylesheetErrors: def _get_snippet(cls, code: str, line_no: int) -> RenderableType: syntax = Syntax( code, - lexer="sass", + lexer="scss", theme="ansi_light", line_numbers=True, indent_guides=True,