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

Editor Help: Improve appearance of script property setters/getters #80765

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+129 −112
Open

Editor Help: Improve appearance of script property setters/getters #80765

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
216 changes: 114 additions & 102 deletions editor/editor_help.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,41 +459,35 @@ String EditorHelp::_fix_constant(const String &p_constant) const {
_add_text(m_message); \
}

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

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

if (p_overview) {
if (p_method_place == METHOD_PLACE_OVERVIEW) {
class_desc->push_cell();
class_desc->push_paragraph(HORIZONTAL_ALIGNMENT_RIGHT, Control::TEXT_DIRECTION_AUTO, "");
} else {
_add_bulletpoint();
}

_add_type(p_method.return_type, p_method.return_enum, p_method.return_is_bitfield);

if (p_overview) {
_add_type(p_method.return_type, p_method.return_enum, p_method.return_is_bitfield);
class_desc->pop(); // paragraph
class_desc->pop(); // cell
class_desc->push_cell();
} else {
} else if (p_method_place == METHOD_PLACE_DESCRIPTION) {
_add_bulletpoint();
_add_type(p_method.return_type, p_method.return_enum, p_method.return_is_bitfield);
class_desc->add_text(" ");
}

const bool is_documented = p_method.is_deprecated || p_method.is_experimental || !p_method.description.strip_edges().is_empty();

if (p_overview && is_documented) {
if (p_allow_link && is_documented) {
class_desc->push_meta("@method " + p_method.name);
}

class_desc->push_color(theme_cache.headline_color);
class_desc->push_color(p_method_place == METHOD_PLACE_PROPERTY_SETGET ? theme_cache.text_color : theme_cache.headline_color);
class_desc->add_text(p_method.name);
class_desc->pop(); // color

if (p_overview && is_documented) {
if (p_allow_link && is_documented) {
class_desc->pop(); // meta
}

Expand All @@ -511,8 +505,10 @@ void EditorHelp::_add_method(const DocData::MethodDoc &p_method, bool p_overview
}

class_desc->add_text(argument.name);
class_desc->add_text(": ");
_add_type(argument.type, argument.enumeration, argument.is_bitfield);
if (p_method_place != METHOD_PLACE_PROPERTY_SETGET) {
class_desc->add_text(": ");
_add_type(argument.type, argument.enumeration, argument.is_bitfield);
}

if (!argument.default_value.is_empty()) {
class_desc->push_color(theme_cache.symbol_color);
Expand All @@ -527,7 +523,7 @@ void EditorHelp::_add_method(const DocData::MethodDoc &p_method, bool p_overview
class_desc->pop(); // color
}

if (is_vararg) {
if (p_method.qualifiers.contains("vararg")) {
class_desc->push_color(theme_cache.text_color);
if (!p_method.arguments.is_empty()) {
class_desc->add_text(", ");
Expand All @@ -542,7 +538,8 @@ void EditorHelp::_add_method(const DocData::MethodDoc &p_method, bool p_overview
class_desc->push_color(theme_cache.symbol_color);
class_desc->add_text(")");
class_desc->pop(); // color
if (!p_method.qualifiers.is_empty()) {

if (p_method_place != METHOD_PLACE_PROPERTY_SETGET && !p_method.qualifiers.is_empty()) {
class_desc->push_color(theme_cache.qualifier_color);

PackedStringArray qualifiers = p_method.qualifiers.split_spaces();
Expand Down Expand Up @@ -571,7 +568,7 @@ void EditorHelp::_add_method(const DocData::MethodDoc &p_method, bool p_overview
class_desc->pop(); // color
}

if (p_overview) {
if (p_method_place == METHOD_PLACE_OVERVIEW) {
if (p_method.is_deprecated) {
class_desc->add_text(" ");
DEPRECATED_DOC_TAG;
Expand Down Expand Up @@ -707,7 +704,7 @@ void EditorHelp::_update_method_list(MethodType p_method_type, const Vector<DocD
}

// For constructors always point to the first one.
_add_method(m[i], true, (p_method_type != METHOD_TYPE_CONSTRUCTOR || i == 0));
_add_method(m[i], METHOD_PLACE_OVERVIEW, true, p_method_type != METHOD_TYPE_CONSTRUCTOR || i == 0);
}

any_previous = !m.is_empty();
Expand Down Expand Up @@ -758,7 +755,7 @@ void EditorHelp::_update_method_descriptions(const DocData::ClassDoc &p_classdoc

_push_code_font();
// For constructors always point to the first one.
_add_method(method, false, (p_method_type != METHOD_TYPE_CONSTRUCTOR || i == 0));
_add_method(method, METHOD_PLACE_DESCRIPTION, false, p_method_type != METHOD_TYPE_CONSTRUCTOR || i == 0);
_pop_code_font();

class_desc->add_newline();
Expand Down Expand Up @@ -1100,8 +1097,6 @@ void EditorHelp::_update_doc() {
}

// Properties overview
HashSet<String> skip_methods;

bool has_properties = false;
bool has_property_descriptions = false;
for (const DocData::PropertyDoc &prop : cd.properties) {
Expand All @@ -1116,6 +1111,13 @@ void EditorHelp::_update_doc() {
}
}

enum SkipMethodFlag {
SKIP_METHOD_FLAG_SETTER = 1,
SKIP_METHOD_FLAG_GETTER = 2,
SKIP_METHOD_FLAG_PRIVATE = 4,
};
HashMap<String, int> skip_methods;

if (has_properties) {
class_desc->add_newline();
class_desc->add_newline();
Expand All @@ -1139,7 +1141,7 @@ void EditorHelp::_update_doc() {
bool overridden_property_exists = false;

for (const DocData::PropertyDoc &prop : cd.properties) {
// Ignore undocumented private.
// Ignore undocumented private properties.
const bool is_documented = prop.is_deprecated || prop.is_experimental || !prop.description.strip_edges().is_empty();
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're here

Suggested change
const bool is_documented = prop.is_deprecated || prop.is_experimental || !prop.description.strip_edges().is_empty();
bool is_documented = prop.is_deprecated || prop.is_experimental || !prop.description.strip_edges().is_empty();

Copy link
Member

Choose a reason for hiding this comment

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

Not a requirement, personal preference

if (!is_documented && prop.name.begins_with("_")) {
continue;
Expand Down Expand Up @@ -1172,11 +1174,19 @@ void EditorHelp::_update_doc() {
bool describe = false;

if (!prop.setter.is_empty()) {
skip_methods.insert(prop.setter);
if (skip_methods.has(prop.setter)) {
skip_methods[prop.setter] |= SKIP_METHOD_FLAG_SETTER;
} else {
skip_methods[prop.setter] = SKIP_METHOD_FLAG_SETTER;
}
describe = true;
}
if (!prop.getter.is_empty()) {
skip_methods.insert(prop.getter);
if (skip_methods.has(prop.getter)) {
skip_methods[prop.getter] |= SKIP_METHOD_FLAG_GETTER;
} else {
skip_methods[prop.getter] = SKIP_METHOD_FLAG_GETTER;
}
describe = true;
}

Expand Down Expand Up @@ -1295,18 +1305,26 @@ void EditorHelp::_update_doc() {
bool sort_methods = EDITOR_GET("text_editor/help/sort_functions_alphabetically");

Vector<DocData::MethodDoc> methods;

HashMap<String, const DocData::MethodDoc *> method_map;
for (const DocData::MethodDoc &method : cd.methods) {
if (skip_methods.has(method.name)) {
if (method.arguments.is_empty() /* getter */ || (method.arguments.size() == 1 && method.return_type == "void" /* setter */)) {
method_map[method.name] = &method;

const bool is_documented = method.is_deprecated || method.is_experimental || !method.description.strip_edges().is_empty();
// Ignore undocumented setters and getters.
if (!is_documented && skip_methods.has(method.name)) {
const int argc = method.arguments.size();
const int skip_flags = skip_methods[method.name];
Comment on lines +1315 to +1316
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const int argc = method.arguments.size();
const int skip_flags = skip_methods[method.name];
int argc = method.arguments.size();
int skip_flags = skip_methods[method.name];

// Ignore only setters/getters without additional arguments.
if ((argc == 1 && (skip_flags & SKIP_METHOD_FLAG_SETTER)) || (argc == 0 && (skip_flags & SKIP_METHOD_FLAG_GETTER))) {
continue;
}
}
// Ignore undocumented non virtual private.
const bool is_documented = method.is_deprecated || method.is_experimental || !method.description.strip_edges().is_empty();
// Ignore undocumented non-virtual private methods.
if (!is_documented && method.name.begins_with("_") && !method.qualifiers.contains("virtual")) {
skip_methods[method.name] = SKIP_METHOD_FLAG_PRIVATE;
continue;
}
skip_methods.erase(method.name);
methods.push_back(method);
}

Expand Down Expand Up @@ -1571,7 +1589,7 @@ void EditorHelp::_update_doc() {

enums[constant.enumeration].push_back(constant);
} else {
// Ignore undocumented private.
// Ignore undocumented private properties.
const bool is_documented = constant.is_deprecated || constant.is_experimental || !constant.description.strip_edges().is_empty();
if (!is_documented && constant.name.begins_with("_")) {
continue;
Expand Down Expand Up @@ -2009,7 +2027,7 @@ void EditorHelp::_update_doc() {
if (prop.overridden) {
continue;
}
// Ignore undocumented private.
// Ignore undocumented private properties.
const bool is_documented = prop.is_deprecated || prop.is_experimental || !prop.description.strip_edges().is_empty();
if (!is_documented && prop.name.begins_with("_")) {
continue;
Expand Down Expand Up @@ -2052,95 +2070,89 @@ void EditorHelp::_update_doc() {
class_desc->pop(); // color
}

if (cd.is_script_doc && (!prop.setter.is_empty() || !prop.getter.is_empty())) {
class_desc->push_color(theme_cache.symbol_color);
class_desc->add_text(" [" + TTR("property:") + " ");
class_desc->pop(); // color

if (!prop.setter.is_empty()) {
class_desc->push_color(theme_cache.value_color);
class_desc->add_text("setter");
class_desc->pop(); // color
}
if (!prop.getter.is_empty()) {
if (!prop.setter.is_empty()) {
class_desc->push_color(theme_cache.symbol_color);
class_desc->add_text(", ");
class_desc->pop(); // color
}
class_desc->push_color(theme_cache.value_color);
class_desc->add_text("getter");
class_desc->pop(); // color
}

class_desc->push_color(theme_cache.symbol_color);
class_desc->add_text("]");
class_desc->pop(); // color
}

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

// Script doc doesn't have setter, getter.
if (!cd.is_script_doc) {
HashMap<String, DocData::MethodDoc> method_map;
for (int j = 0; j < methods.size(); j++) {
method_map[methods[j].name] = methods[j];
}
if (!prop.setter.is_empty()) {
class_desc->push_cell();
class_desc->pop(); // cell

if (!prop.setter.is_empty()) {
class_desc->push_cell();
class_desc->pop(); // cell
class_desc->push_cell();
_push_code_font();

class_desc->push_cell();
_push_code_font();
class_desc->push_color(theme_cache.text_color);
const bool has_setter_link = method_line.has(prop.setter);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const bool has_setter_link = method_line.has(prop.setter);
bool has_setter_link = method_line.has(prop.setter);

if (!has_setter_link) {
method_line[prop.setter] = property_line[prop.name];
}

if (method_map[prop.setter].arguments.size() > 1) {
// Setters with additional arguments are exposed in the method list, so we link them here for quick access.
class_desc->push_meta("@method " + prop.setter);
class_desc->add_text(prop.setter + TTR("(value)"));
class_desc->pop(); // meta
if (method_map.has(prop.setter)) {
_add_method(*method_map[prop.setter], METHOD_PLACE_PROPERTY_SETGET, has_setter_link, false);
} else {
class_desc->push_color(theme_cache.text_color);
if (prop.setter.begins_with("@")) {
// GDScript inline setter.
class_desc->add_text("<inline setter>");
} else {
class_desc->add_text(prop.setter + TTR("(value)"));
class_desc->add_text(prop.setter);
}
class_desc->pop(); // color

class_desc->push_color(theme_cache.symbol_color);
class_desc->add_text("(");
class_desc->pop(); // color
class_desc->push_color(theme_cache.comment_color);
class_desc->add_text(" setter");

class_desc->push_color(theme_cache.text_color);
class_desc->add_text("value");
class_desc->pop(); // color
_pop_code_font();
class_desc->pop(); // cell

method_line[prop.setter] = property_line[prop.name];
class_desc->push_color(theme_cache.symbol_color);
class_desc->add_text(")");
class_desc->pop(); // color
}

if (!prop.getter.is_empty()) {
class_desc->push_cell();
class_desc->pop(); // cell
class_desc->push_color(theme_cache.comment_color);
class_desc->add_text(" setter");
class_desc->pop(); // color

class_desc->push_cell();
_push_code_font();
class_desc->push_color(theme_cache.text_color);
_pop_code_font();
class_desc->pop(); // cell
}

if (!prop.getter.is_empty()) {
class_desc->push_cell();
class_desc->pop(); // cell

class_desc->push_cell();
_push_code_font();

const bool has_getter_link = method_line.has(prop.getter);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const bool has_getter_link = method_line.has(prop.getter);
bool has_getter_link = method_line.has(prop.getter);

if (!has_getter_link) {
method_line[prop.getter] = property_line[prop.name];
}

if (!method_map[prop.getter].arguments.is_empty()) {
// Getters with additional arguments are exposed in the method list, so we link them here for quick access.
class_desc->push_meta("@method " + prop.getter);
class_desc->add_text(prop.getter + "()");
class_desc->pop(); // meta
if (method_map.has(prop.getter)) {
_add_method(*method_map[prop.getter], METHOD_PLACE_PROPERTY_SETGET, has_getter_link, false);
} else {
class_desc->push_color(theme_cache.text_color);
if (prop.getter.begins_with("@")) {
// GDScript inline getter.
class_desc->add_text("<inline getter>");
} else {
class_desc->add_text(prop.getter + "()");
class_desc->add_text(prop.getter);
}

class_desc->pop(); // color
class_desc->push_color(theme_cache.comment_color);
class_desc->add_text(" getter");
class_desc->pop(); // color
_pop_code_font();
class_desc->pop(); // cell

method_line[prop.getter] = property_line[prop.name];
class_desc->push_color(theme_cache.symbol_color);
class_desc->add_text("()");
class_desc->pop(); // color
}

class_desc->push_color(theme_cache.comment_color);
class_desc->add_text(" getter");
class_desc->pop(); // color

_pop_code_font();
class_desc->pop(); // cell
}

class_desc->pop(); // table
Expand Down
Loading
Loading