-
-
Notifications
You must be signed in to change notification settings - Fork 21.2k
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
Tooltips for exported variables #35716
base: 3.x
Are you sure you want to change the base?
Conversation
This looks great! I'm kind of amazed it was that simple to implement. The Still, we may want to look into adding a basic Markdown -> BBCode formatter. Writing BBCode in comments makes it less readable, whereas Markdown source is still easy to read. Supporting just |
I'm not sure it is a good idea to add it to PropertyInfo. In fact you can get the documentations of the exported variables from the LSP extended parser with multi-lines supported.
godot/modules/gdscript/language_server/lsp.hpp Lines 1093 to 1115 in 6fcb58f
|
Yeah, I guess it would be better to wrap this in TOOLS_ENABLED block as well.
I get it, but the problem solved here is different - to provide the tool to show helpful info in editor, especially useful for non-programmers. As I tried your parser with VS Code it gets the whole comment above the variable. You can't explicitly mark something as documentation, right? |
Could this be expanded to be a general system for inline documentation, similar to the |
@aaronfranke The proposal you're referring to is godotengine/godot-proposals#177. |
@@ -142,6 +142,7 @@ class GDScriptTokenizer { | |||
TK_ERROR, | |||
TK_EOF, | |||
TK_CURSOR, //used for code completion | |||
TK_TOOLTIP, |
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.
creating a new token would break the parser unless every corner cases were handled.
ex: if TK_TOOLTIP
inside a function (or just a comment starting with ##) the parser complains Expected end of statement after expression, got ...
inside _parse_block()
and we check the next and previous tokens for parsing so we need to tell the parser that if it's a tool tip token just skip it.
Is this something that @vnen should be considering as another type of GDScript annotation? |
Yeah, this should be added as an annotation. @ThakeeNathees is working on showing GDScript documentation in the editor as a GSoC project, so a variable description can also be used for the tooltips. |
Eagerly waiting for this to get merged and usable! |
@SHIVAMMUKHERJEE This pull request will most likely conflict with #39359 and the GDScript rewrite (with the annotation syntax), so I doubt it'll be merged. |
@Calinou should we consider export variable doc comments as tool tips ? |
I'll be Waiting for Godot then! As long as I get to use tooltips like a sane person again. |
@ThakeeNathees only if the export variable's doc comments are also rendered as editor tooltips. Has that been confirmed by @vnen? |
@willnationsdev No, I'm just suggesting that to use it as a tooltip yet no decision has made. |
@willnationsdev , @vnen agreed to use the documentation of export variable as it's tooltip, |
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 think this would be a welcome change for 3.4 since the syntax is forwards-compatible with 4.0. However, https://github.com/godotengine/godot/pull/35716/files#r389275266 needs to be addressed before this can be merged.
@Calinou I fixed auto-rebase issues and the comment with trailing comma in the same line. I've also added the solution for multiline tooltip comments, which you asked about at the very beginning and I missed this earlier. |
man i would really like this feature. i'm designing an interface for our writers to use without touching much code and tooltips would be massively helpful. |
71cb8d3
to
c58391c
Compare
Apparently this implementation works with built-in scripts, unlike the 4.0 one. However, it breaks compatibility, due to the token issue mentioned before. I tend to put |
@KoBeWi could you please test those corner cases on newer version of my PR? I found some issues in the release build too, so it should be covered now. Right now it will generate token only if |
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.
Looks ok feature-wise, but GDScript changes need review.
Also we should probably document this somewhere, including the BBCode syntax (although I assume it's the same as in RTL). Especially the multiline comments might be confusing, as you need to use [br]
.
How difficult is it to make a markdown or even standard XML comment parser? |
There are many Markdown parsing libraries out there, but you'd still have to write the part that outputs BBCode for use in RichTextLabel. That said, I wouldn't add support for Markdown syntax here, as BBCode is what we went for in 4.0. It's not as human-readable, but it's far more powerful than Markdown could ever pretend to be 🙂 |
If this is to be commented, it will likely cause a similar if not the exact same issue as #66339. I am currently making a fix for the header problem(linked issue) but if I wanna do anything bigger than that it's gonna be a bit of a pain to backport. Also, this may break single hash docs since the LSP originally used that format. |
Does anyone know if tooltips from documentation (e.g. XMLDoc) are available for C# If not, where is an issue/PR I can watch to track this? Sorry if this is the wrong place to ask. I tried to find a specific issue for C#, but I only found this. Should I open a new issue? For example, something like this: ///<summary>Tooltip blah blah </summary>
[Export] string myProp; Or even something like this, if you want to be more flexible? ///<tooltip><summary>Tooltip blah blah </summary></tooltip>
///<remarks>Not in tooltip.</remarks>
[Export] string myProp; |
Thanks Calinou! I started writing one, but turns out there is a proposal already: godotengine/godot-proposals#5675 |
There's also properties exported via Looks like something that should be tightly related, but I'm not familiar with the code. I wonder if implementing this for those should better be done in another PR, |
adds tooltips to exportd vars
This reverts commit 4d44318.
Backward compatible solution to add custom tooltips for exported variables.
Uses comments syntax, so it won't throw errors on previous versions. Tooltips are parsed only in editor. Supports BBCode.
Related issue #6204