-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: enhance view component support #8
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
base: main
Are you sure you want to change the base?
Conversation
The previous implementation conflicts with webTypes.
Taken from the website.
Do we have any formatting requirement that should be enforced? Right now I just formatted the files with the defaults of IntelliJ IDEA. |
Qodana Community for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/[email protected]
with:
upload-result: true Contact Qodana teamContact us at [email protected]
|
If the framework has `x-base` and the project also has `x-base`, the project-level `x-base` will override the framework-level one.
Can be invoked via `gradlew ktfmtCheck` and `gradlew ktfmtFormat`
@xepozz Could you take a look at this when you got time? What do you think? I'd merge it if it looks alright. |
Hey, @xHeaven ! Thanks for the PR
I think it's better to have reference-based platform instead of implementing direct reference calculation. References are more general. It's more convenient to create/pass/resolve a reference instead of creating another GoToDeclarationHandler. Moreover, IDE does navigation by itself if any references under a mouse while clicking on it.
They does not equals by its own features, so there are 2 different mechanisms.
Awesome
Awesome, should we do the same for the dark image? |
# Example: platformPlugins = com.jetbrains.php:203.4449.22, org.intellij.scala:2023.3.27@EAP | ||
#platformPlugins=com.jetbrains.php:243.25659.59,com.jetbrains.hackathon.indices.viewer:1.28 | ||
platformPlugins=com.jetbrains.php:251.25410.129,com.jetbrains.hackathon.indices.viewer:1.28 | ||
platformPlugins=com.jetbrains.php:253.17525.99,com.jetbrains.hackathon.indices.viewer:1.31 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to support at least 4 previous major versions of the IDE.
2025.1 -> 2024.1, 2024.2, 2024.3, 2024.4
2025.2 -> 2024.2, 2024.3, 2024.4, 2025.1
Or as much as possible versions.
Could you please downgrade the plugin to the 242 and check if everything works great?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can try, though there was a bit of a mess with the webTypes
, because below 252.*.*
, there are webSymbols.webTypes
, above 252.*.*
there are polySymbols.webTypes
. I'll take a look. I don't like the way I did the webTypes at all, though, I think it should be entirely dynamic - or at least as much as possible. I just couldn't make it work. Could you maybe look into it? Then I'm fine with any version of the IDE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current web/poly types feature scope is to provide:
- xml tag
- xml attribute
- description
That can be pretty done with custom references providers and description providers.
I have done a dynamic attributes provider. There should be another static provider for the default undeclared values. Like a map of tags and it's attributes.
Descriptions can be done with xml.attributeDescriptorsProvider
and xml.elementDescriptorProvider
extension points.
We can introduce another map, but I'd suggest to have one common map:
- tag:
-- name
-- description
-- content type empty (`<tag />`) / required (`<tag >...</tag>`) / any
-- attributes
--- name
--- description
--- required/optional
--- some helpful stuff
-- some helpful stuff
and use it across the project via several helperes
} | ||
} | ||
|
||
ktfmt { kotlinLangStyle() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understood, kotlinfmt
has very specific, non-configurable, opinionated setups. See: https://github.com/facebook/ktfmt#ktfmt-uses-a-2-space-indent-why-not-4-any-way-to-change-that
I went with the 4 space indent variant - that's what you see right now. The other is Google's opinionated version with 2 space indenting. Shall we try that?
I also don't necessarily want to opt-out of a specific formatter, since different IDEs and IDE setups will result in different formatting styles over the project, which can get nasty. We could say that we push a .idea-based formatting to the repo, but not every contributor will use JB IDEs so it won't enforce anything for them. I'm open to different formatting solutions as well, let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave code style "as is", there are more cons than pros using this formatter.
I usually use "optimize imports" option from the commit dialog in my IDEA.
To format a file a do cmd+option+l from time to time and that's totally fine for me.
To share a codestyle guides I usually use https://www.jetbrains.com/help/idea/editorconfig.html. VScode does support it as well.
but not every contributor will use JB IDEs
CI? :)
Anyway, I'd leave all as is. If you want to collapse/expand lines or anything else I don't mind.
"version": "1.0.0", | ||
"description-markup": "markdown", | ||
"default-icon": "icon", | ||
"description-markup": "html", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any features in HTML that cannot be done with md?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newlines, mostly. Couldn't make it them work with markdown since we can't press Enter in JSON and /n
/<br>
wouldn't want to behave either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use \n\n
as a line delimiter
"doc-url": "https://tempestphp.com/1.x/internals/view-spec#x-base" | ||
"description": "<p>A base template you can install into your own project as a starting point. This one includes the <strong>Tailwind CDN</strong> for quick prototyping.</p><p><strong>Available named slots:</strong></p><ul><li><code>head</code></li><li><code>scripts</code></li></ul>", | ||
"doc-url": "https://tempestphp.com/1.x/internals/view-spec#x-base", | ||
"attributes": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand Tempest, all these components have flexible structure with dynamic attributes.
So there should be another way to provide the attributes based on global variables lookup in the template files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, components should be inferred entirely dynamically instead of relying on static webTypes - we should use the webTypes json only for static things.
We can either go with the tempest meta:view-component
command that I mentioned earlier on Telegram, or if we want to avoid IO for now, we should implement our own component for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I mentioned above, there should be dynamic and static attributes providers. I've implemented a dynamic attributes provider. Feel free to implement a static one
"name": "x-vite-tags", | ||
"description": "Tempest has built-in support for Vite, the most popular front-end development server and build tool. This component simply inject registered entrypoints where it is called.", | ||
"doc-url": "https://tempestphp.com/1.x/internals/view-spec#x-vite-tags" | ||
"description": "<p>Tempest has built-in support for Vite, the most popular front-end development server and build tool. This component simply inject registered entrypoints where it is called.</p>", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure how to kill off this message - webTypes only provides support for the tag itself, so <x-vite-tags></x-vite-tags>
won't show inspection issues. We'll need to look into how to mark specific tags as "empty-taggable".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like there should be another custom xml structure provider
Hey @xepozz! Thanks for the review!
I agree, however, the reference contributor seems to conflict with webTypes for me - having the reference contributor in place wouldn't work together for me with webTypes. Could you confirm it?
Thanks, however I think we should opt for dynamically finding and registering components and attributes - the docs are very hidden for this one. I could use some help here.
The transparent icon seems to work fine in both light and dark mode - not sure if we need any other icon other than the transparent one, so we could keep just that. What do you think? |
Nope, it works.
There
Probably, yes. |
Improves the component view support within the IDE by: