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

Tooltips for exported variables #35716

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from
Open

Conversation

Listwon
Copy link
Contributor

@Listwon Listwon commented Jan 29, 2020

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.

## [b]Tooltips[/b] for exported variables. [br][img]icon.png[/img] [br]With [color=green]BBCode[/color] support. [br]Enjoy!
export var with_tooltip = 2

tooltips1
tooltips2

Related issue #6204

@Calinou
Copy link
Member

Calinou commented Jan 29, 2020

This looks great! I'm kind of amazed it was that simple to implement. The ## syntax makes sense to me. What does it look like if you wrap the comment over several lines?

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 [b], [i] and [code] is probably sufficient. Many other languages like Rust also support Markdown formatting in doc comments.

@Geequlim
Copy link
Contributor

Geequlim commented Jan 30, 2020

I'm not sure it is a good idea to add it to PropertyInfo.
As it is not used by properties which are not exported. And it is useless for release version of our games.

In fact you can get the documentations of the exported variables from the LSP extended parser with multi-lines supported.

const lsp::DocumentSymbol *get_member_symbol(const String &p_name, const String &p_subclass = "") const;

/**
* Represents programming constructs like variables, classes, interfaces etc. that appear in a document. Document symbols can be
* hierarchical and they have two ranges: one that encloses its definition and one that points to its most interesting range,
* e.g. the range of an identifier.
*/
struct DocumentSymbol {
/**
* The name of this symbol. Will be displayed in the user interface and therefore must not be
* an empty string or a string only consisting of white spaces.
*/
String name;
/**
* More detail for this symbol, e.g the signature of a function.
*/
String detail;
/**
* Documentation for this symbol
*/
String documentation;

@Listwon
Copy link
Contributor Author

Listwon commented Jan 30, 2020

@Geequlim

I'm not sure it is a good idea to add it to PropertyInfo.

Yeah, I guess it would be better to wrap this in TOOLS_ENABLED block as well.

In fact you can get the documentations of the exported variables from the LSP extended parser with multi-lines supported.

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?

@aaronfranke
Copy link
Member

Could this be expanded to be a general system for inline documentation, similar to the /// <summary> XML comments in C#? There's an issue somewhere about generating in-editor documentation for user-created scripts.

@Calinou
Copy link
Member

Calinou commented Mar 6, 2020

@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,
Copy link
Contributor

@ThakeeNathees ThakeeNathees Mar 7, 2020

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.

@willnationsdev
Copy link
Contributor

Is this something that @vnen should be considering as another type of GDScript annotation?

@vnen
Copy link
Member

vnen commented May 29, 2020

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.

@Vivraan
Copy link
Contributor

Vivraan commented Aug 2, 2020

Eagerly waiting for this to get merged and usable!

@Calinou
Copy link
Member

Calinou commented Aug 2, 2020

@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.

@ThakeeNathees
Copy link
Contributor

@Calinou should we consider export variable doc comments as tool tips ?

@Vivraan
Copy link
Contributor

Vivraan commented Aug 2, 2020

I'll be Waiting for Godot then! As long as I get to use tooltips like a sane person again.

@willnationsdev
Copy link
Contributor

@ThakeeNathees only if the export variable's doc comments are also rendered as editor tooltips. Has that been confirmed by @vnen?

@ThakeeNathees
Copy link
Contributor

@willnationsdev No, I'm just suggesting that to use it as a tooltip yet no decision has made.

@ThakeeNathees
Copy link
Contributor

ThakeeNathees commented Aug 6, 2020

@willnationsdev , @vnen agreed to use the documentation of export variable as it's tooltip,

here is a working example
Capture
Screenshot (28)

@Calinou Calinou added this to the 4.0 milestone Jan 5, 2021
@Calinou
Copy link
Member

Calinou commented Jan 5, 2021

This feature is now available in master (= what will become 4.0 in the future) thanks to #41095.

Should we aim to support this in 3.2? We should aim to use a compatible syntax so that projects can be upgraded smoothly without having to modify the comments.

cc @vnen

@Calinou Calinou changed the base branch from master to 3.2 January 5, 2021 00:54
@Calinou Calinou modified the milestones: 4.0, 3.2 Jan 5, 2021
@aaronfranke aaronfranke modified the milestones: 3.2, 3.3, 3.4 Mar 16, 2021
Copy link
Member

@Calinou Calinou left a 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.

misc/dist/linux/godot.6 Outdated Show resolved Hide resolved
misc/dist/osx_tools.app/Contents/Info.plist Outdated Show resolved Hide resolved
misc/dist/osx_tools.app/Contents/Info.plist Outdated Show resolved Hide resolved
editor/inspector_dock.cpp Outdated Show resolved Hide resolved
modules/gdscript/gdscript_tokenizer.cpp Outdated Show resolved Hide resolved
version.py Outdated Show resolved Hide resolved
version.py Outdated Show resolved Hide resolved
@Listwon
Copy link
Contributor Author

Listwon commented Mar 22, 2021

@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.

@Chaosus Chaosus modified the milestones: 3.4, 3.5 Nov 8, 2021
@DaGamingWolf
Copy link

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.

@KoBeWi
Copy link
Member

KoBeWi commented Jan 10, 2022

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 ## in comments for stuff I want to check layer, so it's easier to find. If this was merged, all scripts using ## "incorrectly" will break.

@Listwon
Copy link
Contributor Author

Listwon commented Feb 21, 2022

@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 ## comment is just before export variable.

Copy link
Member

@KoBeWi KoBeWi left a 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].

@Vivraan
Copy link
Contributor

Vivraan commented Jun 19, 2022

How difficult is it to make a markdown or even standard XML comment parser?

@Calinou
Copy link
Member

Calinou commented Jun 19, 2022

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 🙂

@atirut-w
Copy link
Contributor

atirut-w commented Oct 4, 2022

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.

@geekley
Copy link

geekley commented Jun 2, 2023

Does anyone know if tooltips from documentation (e.g. XMLDoc) are available for C# [Export] members in 4.0? I tried a simple test and it doesn't seem like it's supported (yet?).

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;

@Calinou
Copy link
Member

Calinou commented Jun 2, 2023

@geekley It's only supported in GDScript in 4.0. This is not the place to discuss C# support though; please open a proposal for that.

@geekley
Copy link

geekley commented Jun 2, 2023

Thanks Calinou! I started writing one, but turns out there is a proposal already: godotengine/godot-proposals#5675
Linking here in case people want to upvote it.

@turbohz
Copy link

turbohz commented Aug 7, 2023

There's also properties exported via _get_property_list().

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,
or could just be done in one go.

Valla-Chan added a commit to Valla-Chan/Godot-3.5.1-Ghodost that referenced this pull request Jan 20, 2024
adds tooltips to exportd vars
Valla-Chan added a commit to Valla-Chan/Godot-3.5.1-Ghodost that referenced this pull request Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.