Fix indentation in script templates #78675
Conversation
|
If possible, you could add some unit tests. |
|
I'm not sure that's possible this is specific to the editor interface AFIK it's just not possible to do unit testing on the editor UI and stuff, and it wouldn't be easy to even do so as it is an internal function like this, I don't think there's any unit tests for any Editor components, but I might be wrong |
|
Purely theoretical: Copy test-template to Project/script_templates/Node/. |
|
You can't do that from CLI, this is specific to the editor UI, the |
akien-mga
added
the
cherrypick:4.1
AThousandShips
force-pushed
the
template_fix
branch
from
6d6bcce to
e5732b8
Compare
| } | ||
| script_template.content += line.replace("\t", "_TS_") + "\n"; | ||
| script_template.content += line.substr(i) + "\n"; |
There was a problem hiding this comment.
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
| if (line.begins_with("space-indent")) { | ||
| String indent_value = line.substr(17, -1).strip_edges(); | ||
| 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) { |
There was a problem hiding this comment.
| if (indent_size >= 0) { | |
| if (indent_size > 0) { |
There was a problem hiding this comment.
Is there any reason to change the original support for not converting spaces to indentation?
There was a problem hiding this comment.
I'm not sure about this, just suggested as it looks like a mistake.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
AThousandShips
force-pushed
the
template_fix
branch
from
e5732b8 to
3935346
Compare
|
Thanks! |
|
Thank you! |
|
Cherry-picked for 4.1.3. |
YuriSizov
removed
the
cherrypick:4.1
Uses a variation on the code from
CodeEdit, works well in my testing, also fixes the parsing ofmeta-space-indentwhich was not updated and used the wrong spacing and error messageWould be useful to be able to unify the code but the code for
CodeEditturns one type of indentation into another instead of turning two types of indentation into one special sequence