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.

Sign up for GitHub

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

Improve GDScript documentation generation & behavior #72095

Merged
merged 1 commit into from
Apr 26, 2023
Merged

Improve GDScript documentation generation & behavior #72095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 0 additions & 55 deletions core/doc_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,61 +315,6 @@ class DocData {
}
};

struct EnumDoc {
String name = "@unnamed_enum";
bool is_bitfield = false;
String description;
Vector<DocData::ConstantDoc> values;
static EnumDoc from_dict(const Dictionary &p_dict) {
EnumDoc doc;

if (p_dict.has("name")) {
doc.name = p_dict["name"];
}

if (p_dict.has("is_bitfield")) {
doc.is_bitfield = p_dict["is_bitfield"];
}

if (p_dict.has("description")) {
doc.description = p_dict["description"];
}

Array values;
if (p_dict.has("values")) {
values = p_dict["values"];
}
for (int i = 0; i < values.size(); i++) {
doc.values.push_back(ConstantDoc::from_dict(values[i]));
}

return doc;
}
static Dictionary to_dict(const EnumDoc &p_doc) {
Dictionary dict;

if (!p_doc.name.is_empty()) {
dict["name"] = p_doc.name;
}

dict["is_bitfield"] = p_doc.is_bitfield;

if (!p_doc.description.is_empty()) {
dict["description"] = p_doc.description;
}

if (!p_doc.values.is_empty()) {
Array values;
for (int i = 0; i < p_doc.values.size(); i++) {
values.push_back(ConstantDoc::to_dict(p_doc.values[i]));
}
dict["values"] = values;
}

return dict;
}
};

struct PropertyDoc {
String name;
String type;
Expand Down
124 changes: 86 additions & 38 deletions editor/editor_help.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#include "core/core_constants.h"
#include "core/input/input.h"
#include "core/object/script_language.h"
#include "core/os/keyboard.h"
#include "core/version.h"
#include "core/version_generated.gen.h"
Expand All @@ -45,6 +46,8 @@

#define CONTRIBUTE_URL vformat("%s/contributing/documentation/updating_the_class_reference.html", VERSION_DOCS_URL)

// TODO: this is sometimes used directly as doc->something, other times as EditorHelp::get_doc_data(), which is thread-safe.
// Might this be a problem?
DocTools *EditorHelp::doc = nullptr;
YuriSizov marked this conversation as resolved.
Show resolved Hide resolved

class DocCache : public Resource {
Expand Down Expand Up @@ -74,6 +77,49 @@ class DocCache : public Resource {
void set_classes(const Array &p_classes) { classes = p_classes; }
};

static bool _attempt_doc_load(const String &p_class) {
// Docgen always happens in the outer-most class: it also generates docs for inner classes.
String outer_class = p_class.get_slice(".", 0);
if (!ScriptServer::is_global_class(outer_class)) {
return false;
}

// ResourceLoader is used in order to have a script-agnostic way to load scripts.
// This forces GDScript to compile the code, which is unnecessary for docgen, but it's a good compromise right now.
Comment on lines +87 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me think that perhaps the ScriptServer should be aware of documentation and just as the script language to provide documentation data. This would allow not only GDScript and other languages to optimize the doc gen process.

Of course this is just something to discuss later, no need to worry about it for this PR. I agree that loading the resource is a good compromise for now.

Ref<Script> script = ResourceLoader::load(ScriptServer::get_global_class_path(outer_class), outer_class);
if (script.is_valid()) {
Vector<DocData::ClassDoc> docs = script->get_documentation();
for (int j = 0; j < docs.size(); j++) {
const DocData::ClassDoc &doc = docs.get(j);
EditorHelp::get_doc_data()->add_doc(doc);
}
return true;
}

return false;
}

// Removes unnecessary prefix from p_class_specifier when within the p_edited_class context
static String _contextualize_class_specifier(const String &p_class_specifier, const String &p_edited_class) {
// If this is a completely different context than the current class, then keep full path
if (!p_class_specifier.begins_with(p_edited_class)) {
return p_class_specifier;
}

// Here equal length + begins_with from above implies p_class_specifier == p_edited_class :)
if (p_class_specifier.length() == p_edited_class.length()) {
int rfind = p_class_specifier.rfind(".");
if (rfind == -1) { // Single identifier
return p_class_specifier;
}
// Multiple specifiers: keep last one only
return p_class_specifier.substr(rfind + 1);
}

// Remove prefix
return p_class_specifier.substr(p_edited_class.length() + 1);
}

void EditorHelp::_update_theme_item_cache() {
VBoxContainer::_update_theme_item_cache();

Expand Down Expand Up @@ -131,12 +177,13 @@ void EditorHelp::_class_list_select(const String &p_select) {
}

void EditorHelp::_class_desc_select(const String &p_select) {
if (p_select.begins_with("$")) { //enum
if (p_select.begins_with("$")) { // enum
String select = p_select.substr(1, p_select.length());
String class_name;
if (select.contains(".")) {
class_name = select.get_slice(".", 0);
select = select.get_slice(".", 1);
int rfind = select.rfind(".");
if (rfind != -1) {
class_name = select.substr(0, rfind);
select = select.substr(rfind + 1);
} else {
class_name = "@GlobalScope";
}
Expand Down Expand Up @@ -254,35 +301,35 @@ void EditorHelp::_add_type(const String &p_type, const String &p_enum) {
bool is_enum_type = !p_enum.is_empty();
bool can_ref = !p_type.contains("*") || is_enum_type;

String t = p_type;
String link_t = p_type; // For links in metadata
String display_t = link_t; // For display purposes
if (is_enum_type) {
if (p_enum.get_slice_count(".") > 1) {
t = p_enum.get_slice(".", 1);
} else {
t = p_enum.get_slice(".", 0);
}
link_t = p_enum; // The link for enums is always the full enum description
display_t = _contextualize_class_specifier(p_enum, edited_class);
} else {
display_t = _contextualize_class_specifier(p_type, edited_class);
}

class_desc->push_color(theme_cache.type_color);
bool add_array = false;
if (can_ref) {
if (t.ends_with("[]")) {
if (link_t.ends_with("[]")) {
add_array = true;
t = t.replace("[]", "");
link_t = link_t.replace("[]", "");

class_desc->push_meta("#Array"); //class
class_desc->push_meta("#Array"); // class
class_desc->add_text("Array");
class_desc->pop();
class_desc->add_text("[");
}

if (is_enum_type) {
class_desc->push_meta("$" + p_enum); //class
class_desc->push_meta("$" + link_t); // enum
} else {
class_desc->push_meta("#" + t); //class
class_desc->push_meta("#" + link_t); // class
}
}
class_desc->add_text(t);
class_desc->add_text(display_t);
if (can_ref) {
class_desc->pop(); // Pushed meta above.
if (add_array) {
Expand Down Expand Up @@ -339,7 +386,7 @@ String EditorHelp::_fix_constant(const String &p_constant) const {
class_desc->pop();

void EditorHelp::_add_method(const DocData::MethodDoc &p_method, bool p_overview) {
method_line[p_method.name] = class_desc->get_paragraph_count() - 2; //gets overridden if description
method_line[p_method.name] = class_desc->get_paragraph_count() - 2; // Gets overridden if description

const bool is_vararg = p_method.qualifiers.contains("vararg");

Expand All @@ -353,8 +400,8 @@ void EditorHelp::_add_method(const DocData::MethodDoc &p_method, bool p_overview
_add_type(p_method.return_type, p_method.return_enum);

if (p_overview) {
class_desc->pop(); //align
class_desc->pop(); //cell
class_desc->pop(); // align
class_desc->pop(); // cell
class_desc->push_cell();
} else {
class_desc->add_text(" ");
Expand All @@ -369,7 +416,7 @@ void EditorHelp::_add_method(const DocData::MethodDoc &p_method, bool p_overview
class_desc->pop();

if (p_overview && !p_method.description.strip_edges().is_empty()) {
class_desc->pop(); //meta
class_desc->pop(); // meta
}

class_desc->push_color(theme_cache.symbol_color);
Expand Down Expand Up @@ -448,7 +495,7 @@ void EditorHelp::_add_method(const DocData::MethodDoc &p_method, bool p_overview
}

if (p_overview) {
class_desc->pop(); //cell
class_desc->pop(); // cell
}
}

Expand Down Expand Up @@ -489,8 +536,9 @@ void EditorHelp::_pop_code_font() {
class_desc->pop();
}

Error EditorHelp::_goto_desc(const String &p_class, int p_vscr) {
if (!doc->class_list.has(p_class)) {
Error EditorHelp::_goto_desc(const String &p_class) {
// If class doesn't have docs listed, attempt on-demand docgen
if (!doc->class_list.has(p_class) && !_attempt_doc_load(p_class)) {
anvilfolk marked this conversation as resolved.
Show resolved Hide resolved
return ERR_DOES_NOT_EXIST;
}

Expand Down Expand Up @@ -530,9 +578,9 @@ void EditorHelp::_update_method_list(const Vector<DocData::MethodDoc> p_methods)

if (any_previous && !m.is_empty()) {
class_desc->push_cell();
class_desc->pop(); //cell
class_desc->pop(); // cell
class_desc->push_cell();
class_desc->pop(); //cell
class_desc->pop(); // cell
}

String group_prefix;
Expand All @@ -550,9 +598,9 @@ void EditorHelp::_update_method_list(const Vector<DocData::MethodDoc> p_methods)

if (is_new_group && pass == 1) {
class_desc->push_cell();
class_desc->pop(); //cell
class_desc->pop(); // cell
class_desc->push_cell();
class_desc->pop(); //cell
class_desc->pop(); // cell
}

_add_method(m[i], true);
Expand All @@ -561,7 +609,7 @@ void EditorHelp::_update_method_list(const Vector<DocData::MethodDoc> p_methods)
any_previous = !m.is_empty();
}

class_desc->pop(); //table
class_desc->pop(); // table
class_desc->pop();
_pop_code_font();

Expand Down Expand Up @@ -1197,7 +1245,7 @@ void EditorHelp::_update_doc() {

_add_text(cd.signals[i].arguments[j].name);
class_desc->add_text(": ");
_add_type(cd.signals[i].arguments[j].type);
_add_type(cd.signals[i].arguments[j].type, cd.signals[i].arguments[j].enumeration);
if (!cd.signals[i].arguments[j].default_value.is_empty()) {
class_desc->push_color(theme_cache.symbol_color);
class_desc->add_text(" = ");
Expand Down Expand Up @@ -1768,7 +1816,6 @@ void EditorHelp::_request_help(const String &p_string) {
if (err == OK) {
EditorNode::get_singleton()->set_visible_editor(EditorNode::EDITOR_SCRIPT);
}
//100 palabras
}

void EditorHelp::_help_callback(const String &p_topic) {
Expand Down Expand Up @@ -2179,7 +2226,7 @@ static void _add_text_to_rt(const String &p_bbcode, RichTextLabel *p_rt, Control
tag_stack.push_front("font");

} else {
p_rt->add_text("["); //ignore
p_rt->add_text("["); // ignore
pos = brk_pos + 1;
}
}
Expand Down Expand Up @@ -2328,9 +2375,9 @@ void EditorHelp::go_to_help(const String &p_help) {
_help_callback(p_help);
}

void EditorHelp::go_to_class(const String &p_class, int p_scroll) {
void EditorHelp::go_to_class(const String &p_class) {
_wait_for_thread();
_goto_desc(p_class, p_scroll);
_goto_desc(p_class);
}

void EditorHelp::update_doc() {
Expand Down Expand Up @@ -2461,14 +2508,15 @@ void EditorHelpBit::_go_to_help(String p_what) {
}

void EditorHelpBit::_meta_clicked(String p_select) {
if (p_select.begins_with("$")) { //enum

if (p_select.begins_with("$")) { // enum
String select = p_select.substr(1, p_select.length());
String class_name;
if (select.contains(".")) {
class_name = select.get_slice(".", 0);
int rfind = select.rfind(".");
if (rfind != -1) {
class_name = select.substr(0, rfind);
select = select.substr(rfind + 1);
} else {
class_name = "@Global";
class_name = "@GlobalScope";
}
_go_to_help("class_enum:" + class_name + ":" + select);
return;
Expand Down
4 changes: 2 additions & 2 deletions editor/editor_help.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class EditorHelp : public VBoxContainer {
void _class_desc_resized(bool p_force_update_theme);
int display_margin = 0;

Error _goto_desc(const String &p_class, int p_vscr = -1);
Error _goto_desc(const String &p_class);
//void _update_history_buttons();
void _update_method_list(const Vector<DocData::MethodDoc> p_methods);
void _update_method_descriptions(const DocData::ClassDoc p_classdoc, const Vector<DocData::MethodDoc> p_methods, const String &p_method_type);
Expand Down Expand Up @@ -210,7 +210,7 @@ class EditorHelp : public VBoxContainer {
static String get_cache_full_path();

void go_to_help(const String &p_help);
void go_to_class(const String &p_class, int p_scroll = 0);
void go_to_class(const String &p_class);
void update_doc();

Vector<Pair<String, int>> get_sections();
Expand Down
2 changes: 1 addition & 1 deletion editor/plugins/script_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3317,7 +3317,7 @@ void ScriptEditor::_help_class_open(const String &p_class) {
eh->set_name(p_class);
tab_container->add_child(eh);
_go_to_tab(tab_container->get_tab_count() - 1);
eh->go_to_class(p_class, 0);
eh->go_to_class(p_class);
eh->connect("go_to_help", callable_mp(this, &ScriptEditor::_help_class_goto));
_add_recent_script(p_class);
_sort_list_on_update = true;
Expand Down
Loading