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

Fix indentation in script templates #78675

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Fix indentation in script templates #78675

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
51 changes: 35 additions & 16 deletions editor/script_create_dialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ ScriptLanguage::ScriptTemplate ScriptCreateDialog::_parse_template(const ScriptL
ScriptLanguage::ScriptTemplate script_template = ScriptLanguage::ScriptTemplate();
script_template.origin = p_origin;
script_template.inherit = p_inherits;
String space_indent = " ";
int space_indent_size = 4;
// Get meta delimiter
String meta_delimiter;
List<String> comment_delimiters;
Expand All @@ -884,30 +884,49 @@ ScriptLanguage::ScriptTemplate ScriptCreateDialog::_parse_template(const ScriptL
String line = file->get_line();
if (line.begins_with(meta_prefix)) {
// Store meta information
line = line.substr(meta_prefix.length(), -1);
if (line.begins_with("name")) {
script_template.name = line.substr(5, -1).strip_edges();
}
if (line.begins_with("description")) {
script_template.description = line.substr(12, -1).strip_edges();
}
if (line.begins_with("space-indent")) {
String indent_value = line.substr(17, -1).strip_edges();
line = line.substr(meta_prefix.length());
if (line.begins_with("name:")) {
script_template.name = line.substr(5).strip_edges();
} else if (line.begins_with("description:")) {
script_template.description = line.substr(12).strip_edges();
} else if (line.begins_with("space-indent:")) {
String indent_value = line.substr(13).strip_edges();
if (indent_value.is_valid_int()) {
int indent_size = indent_value.to_int();
if (indent_size >= 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (indent_size >= 0) {
if (indent_size > 0) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there any reason to change the original support for not converting spaces to indentation?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this, just suggested as it looks like a mistake.

Copy link
Member Author

Choose a reason for hiding this comment

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

If it is a mistake it should have an error, but I think it's intentional to simply prevent spaces to be used as indentation

Copy link
Member

Choose a reason for hiding this comment

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

However, the option is not available for tabs. I don't think this is an intentional feature, but I'm not asking you to change it.

Copy link
Member Author

@AThousandShips AThousandShips Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That's true, will see, thank you for your feedback!

There was a check for the value non-zero in the original code so it was at least not a simple error of taking >= over > in the original code.

Will add an error though as it doesn't error out if you have a negative number

space_indent = String(" ").repeat(indent_size);
space_indent_size = indent_size;
} else {
WARN_PRINT(vformat("Template meta-space-indent need to be a non-negative integer value. Found %s.", indent_value));
}
} else {
WARN_PRINT(vformat("Template meta-use_space_indent need to be a valid integer value. Found %s.", indent_value));
WARN_PRINT(vformat("Template meta-space-indent need to be a valid integer value. Found %s.", indent_value));
}
}
} else {
// Store script
if (space_indent != "") {
line = line.replace(space_indent, "_TS_");
// Replace indentation.
int i = 0;
int space_count = 0;
for (; i < line.length(); i++) {
if (line[i] == '\t') {
if (space_count) {
script_template.content += String(" ").repeat(space_count);
space_count = 0;
}
script_template.content += "_TS_";
} else if (line[i] == ' ') {
space_count++;
if (space_count == space_indent_size) {
script_template.content += "_TS_";
space_count = 0;
}
} else {
break;
}
}
if (space_count) {
script_template.content += String(" ").repeat(space_count);
}
script_template.content += line.replace("\t", "_TS_") + "\n";
script_template.content += line.substr(i) + "\n";
Copy link
Member Author

@AThousandShips AThousandShips Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

To deal with empty lines or lines that are just whitespace, this matches the pre-first non-whitespace character behavior of the old code, and also matches generally the behavior of converting indentation on save

Could simplify it and merge all spaces and combine them at the end but I think this is better

}
}
}
Expand Down