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

Improve css error reporting #3582

Merged
merged 9 commits into from
Nov 1, 2023
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- DataTable now has a max-height of 100vh rather than 100%, which doesn't work with auto
- Breaking change: empty rules now result in an error https://github.com/Textualize/textual/pull/3566
- Improved startup time by caching CSS parsing https://github.com/Textualize/textual/pull/3575
- CSS error reporting will no longer provide links to the files in question https://github.com/Textualize/textual/pull/3582
- inline CSS error reporting will report widget/class variable where the CSS was read from https://github.com/Textualize/textual/pull/3582

### Added

Expand Down
26 changes: 12 additions & 14 deletions src/textual/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1753,20 +1753,19 @@ def _load_screen_css(self, screen: Screen):

update = False
for path in screen.css_path:
if not self.stylesheet.has_source(path):
if not self.stylesheet.has_source((str(path), "")):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to veto the tuple here. It doesn't read well. Could the second value be a keyword arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that sounds good. See 6c6eecf.

self.stylesheet.read(path)
update = True
if screen.CSS:
try:
screen_css_path = (
f"{inspect.getfile(screen.__class__)}:{screen.__class__.__name__}"
)
screen_path = inspect.getfile(screen.__class__)
except (TypeError, OSError):
screen_css_path = f"{screen.__class__.__name__}"
if not self.stylesheet.has_source(screen_css_path):
screen_path = ""
read_from = (screen_path, f"{screen.__class__.__name__}.CSS")
if not self.stylesheet.has_source(read_from):
self.stylesheet.add_source(
screen.CSS,
path=screen_css_path,
read_from=read_from,
is_default_css=False,
scope=screen._css_type_name if screen.SCOPED_CSS else "",
)
Expand Down Expand Up @@ -2127,23 +2126,22 @@ async def _process_messages(
try:
if self.css_path:
self.stylesheet.read_all(self.css_path)
for path, css, tie_breaker, scope in self._get_default_css():
for read_from, css, tie_breaker, scope in self._get_default_css():
self.stylesheet.add_source(
css,
path=path,
read_from=read_from,
is_default_css=True,
tie_breaker=tie_breaker,
scope=scope,
)
if self.CSS:
try:
app_css_path = (
f"{inspect.getfile(self.__class__)}:{self.__class__.__name__}"
)
app_path = inspect.getfile(self.__class__)
except (TypeError, OSError):
app_css_path = f"{self.__class__.__name__}"
app_path = ""
read_from = (app_path, f"{self.__class__.__name__}.CSS")
self.stylesheet.add_source(
self.CSS, path=app_css_path, is_default_css=False
self.CSS, read_from=read_from, is_default_css=False
)
except Exception as error:
self._handle_exception(error)
Expand Down
13 changes: 0 additions & 13 deletions src/textual/css/_styles_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,6 @@
from .types import BoxSizing, Display, EdgeType, Overflow, Visibility


def _join_tokens(tokens: Iterable[Token], joiner: str = "") -> str:
"""Convert tokens into a string by joining their values
Args:
tokens: Tokens to join
joiner: String to join on.
Returns:
The tokens, joined together to form a string.
"""
return joiner.join(token.value for token in tokens)


Comment on lines -68 to -80
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was dead code.

class StylesBuilder:
"""
The StylesBuilder object takes tokens parsed from the CSS and converts
Expand Down
23 changes: 11 additions & 12 deletions src/textual/css/parse.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

from functools import lru_cache
from pathlib import PurePath
from typing import Iterable, Iterator, NoReturn

from ..suggestions import get_suggestion
Expand All @@ -19,7 +18,7 @@
from .styles import Styles
from .tokenize import Token, tokenize, tokenize_declarations, tokenize_values
from .tokenizer import EOFError, ReferencedBy
from .types import Specificity3
from .types import CSSLocation, Specificity3

SELECTOR_MAP: dict[str, tuple[SelectorType, Specificity3]] = {
"selector": (SelectorType.TYPE, (0, 0, 1)),
Expand All @@ -38,7 +37,7 @@ def parse_selectors(css_selectors: str) -> tuple[SelectorSet, ...]:
if not css_selectors.strip():
return ()

tokens = iter(tokenize(css_selectors, ""))
tokens = iter(tokenize(css_selectors, ("", "")))

get_selector = SELECTOR_MAP.get
combinator: CombinatorType | None = CombinatorType.DESCENDENT
Expand Down Expand Up @@ -180,18 +179,18 @@ def parse_rule_set(
yield rule_set


def parse_declarations(css: str, path: str) -> Styles:
def parse_declarations(css: str, read_from: CSSLocation) -> Styles:
"""Parse declarations and return a Styles object.

Args:
css: String containing CSS.
path: Path to the CSS, or something else to identify the location.
read_from: The location where the CSS was read from.

Returns:
A styles object.
"""

tokens = iter(tokenize_declarations(css, path))
tokens = iter(tokenize_declarations(css, read_from))
styles_builder = StylesBuilder()

declaration: Declaration | None = None
Expand Down Expand Up @@ -245,7 +244,7 @@ def _unresolved(variable_name: str, variables: Iterable[str], token: Token) -> N
message += f"; did you mean '${suggested_variable}'?"

raise UnresolvedVariableError(
token.path,
token.read_from,
token.code,
token.start,
message,
Expand Down Expand Up @@ -341,7 +340,7 @@ def substitute_references(
def parse(
scope: str,
css: str,
path: str | PurePath,
read_from: CSSLocation,
variables: dict[str, str] | None = None,
variable_tokens: dict[str, list[Token]] | None = None,
is_default_rules: bool = False,
Expand All @@ -351,9 +350,9 @@ def parse(
and generating rule sets from it.

Args:
scope: CSS type name
css: The input CSS
path: Path to the CSS
scope: CSS type name.
css: The input CSS.
read_from: The source location of the CSS.
variables: Substitution variables to substitute tokens for.
is_default_rules: True if the rules we're extracting are
default (i.e. in Widget.DEFAULT_CSS) rules. False if they're from user defined CSS.
Expand All @@ -363,7 +362,7 @@ def parse(
if variable_tokens:
reference_tokens.update(variable_tokens)

tokens = iter(substitute_references(tokenize(css, path), variable_tokens))
tokens = iter(substitute_references(tokenize(css, read_from), variable_tokens))
while True:
token = next(tokens, None)
if token is None:
Expand Down
2 changes: 1 addition & 1 deletion src/textual/css/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ def set_styles(
node.set_styles(**update_styles)
if css is not None:
try:
new_styles = parse_declarations(css, path="set_styles")
new_styles = parse_declarations(css, read_from=("set_styles", ""))
except DeclarationError as error:
raise DeclarationError(error.name, error.token, error.message) from None
for node in self:
Expand Down
9 changes: 6 additions & 3 deletions src/textual/css/styles.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
if TYPE_CHECKING:
from .._layout import Layout
from ..dom import DOMNode
from .types import CSSLocation


class RulesMap(TypedDict, total=False):
Expand Down Expand Up @@ -534,20 +535,22 @@ def is_animatable(cls, rule: str) -> bool:

@classmethod
@lru_cache(maxsize=1024)
def parse(cls, css: str, path: str, *, node: DOMNode | None = None) -> Styles:
def parse(
cls, css: str, read_from: CSSLocation, *, node: DOMNode | None = None
) -> Styles:
"""Parse CSS and return a Styles object.

Args:
css: Textual CSS.
path: Path or string indicating source of CSS.
read_from: Location where the CSS was read from.
node: Node to associate with the Styles.

Returns:
A Styles instance containing result of parsing CSS.
"""
from .parse import parse_declarations

styles = parse_declarations(css, path)
styles = parse_declarations(css, read_from)
styles.node = node
return styles

Expand Down
Loading