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

🔨 Include both <sth> and <element name="sth"> in hover #299

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

babakks
Copy link
Contributor

@babakks babakks commented Sep 17, 2022

@babakks
Copy link
Contributor Author

babakks commented Sep 19, 2022

@aeschli I used | (pipe) as the joiner for possible element signatures, like:

<something>|<element name="something">

Of course, this change of behavior of the MarkedStringPrinter class resulted in other tests failing. I wasn't sure that you're okay with the changes that I made in this PR. So, I left other test cases as they were, until hearing your opinion on this.

@aeschli
Copy link
Contributor

aeschli commented Nov 9, 2022

Thanks @babakks. I'm sorry, but I don't think that's the right fix.
[name='something'] should give the hover <element name="something">
<something> is wrong and is the bug.

Note that <element name="something"> is not something you can write in HTML. The element is just used as a placeholder for any element name.

The bug is that the Element class uses the name property to store the element name, but name is a valid attribute, so that conflicts. Instead of name, the element name should be stored in a separate property, or something like <>` should be used which is not a valid attribute name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[scss] Wrongly shown selector in SCSS
2 participants