Improve css error reporting #3582
Conversation
We already kept track of the file and widget CSS was read from. Now, we also keep track of the class variable it comes from and we create some structure to transfer that information across the program.
Instead of creating the link explicitly, we let terminal emulators auto-link to the file. This came after a discussion about how/whether we should try to support linking to specific file lines/columns for TCSS files and after some research to see how that would be possible. We decided to drop this feature when we couldn't find information in the standards for 'file://' regarding how to specify line/column numbers and after we found [this iTerm issue](https://gitlab.com/gnachman/iterm2/-/issues/9376) where the creator/maintainer of iTerm says that there is no standard API for opening a file to a particular line number.
rodrigogiraoserrao
added
bug
| 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) | ||
|
|
||
|
|
There was a problem hiding this comment.
This was dead code.
| css: str = base.__dict__.get("DEFAULT_CSS", "").strip() | ||
| css: str = base.__dict__.get("DEFAULT_CSS", "") |
There was a problem hiding this comment.
This messed up the line/column reporting of inline CSS errors.
src/textual/app.py
Outdated
| @@ -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), "")): | |||
There was a problem hiding this comment.
I'd like to veto the tuple here. It doesn't read well. Could the second value be a keyword arg?
There was a problem hiding this comment.
Yeah, that sounds good. See 6c6eecf.
src/textual/css/stylesheet.py
Outdated
| if token.path: | ||
| path = Path(token.path) | ||
| filename = path.name | ||
| if token.read_from: |
There was a problem hiding this comment.
Could this be an explicit is not None check.
There was a problem hiding this comment.
Reworked this a bit better so that the checks make more sense.
|
Giving this spin with this code: from textual.app import App, ComposeResult
from textual.widgets import Label
class CSSErrorApp(App[None]):
CSS = """
:
"""
def compose(self) -> ComposeResult:
yield Label()
if __name__ == "__main__":
CSSErrorApp().run()I get this error: This seems to be saying that the error is on line 1, but also on line 2. Am I reading that correctly? |
You're reading that correctly, yes, but that also seems to be an issue in I'd be keen on getting this PR in despite of that other bug, unless you feel strongly that I should solve #3625 as a part of this PR. |
|
Cool, if it's a pre-existing thing unrelated this makes sense it's handled in a different fix. |
This PR fixes #3569.
This also improves error importing by providing the widget name/class variable from where CSS was read in case the CSS is inline CSS.
This is still a draft because I still need to figure out if we can link to a specific line/column for when the CSS comes from an external file.