Corrected 'add_callback' for Signal Connection to Prevent Incorrect Number of New Lines

[Eoin-ONeill-Yokai]

As originally reported by KoBeWi #33287 -- With code end-line behavior added in #29262, it's sometimes possible for the wrong number of new lines to be created depending on the number of new lines at the end of the document.

Now, it will try to find the last line with valid content and add the appropriate number of spaces according to the GDScript standard (2 new lines between each function.) This should bring more consistency to the user experience. For files with ridiculous amount of whitespace at the end of the document, there's a cutoff of 50 lines.

[Calinou]

I'm not sure about this, as the GDScript style guide recommends separating functions with two blank lines (like in Python). However, we can still make a check to avoid separating functions with more than two blank lines.

[Eoin-ONeill-Yokai]

Ah yeah that's a good point. Well I could update it to reflect that (so that 2 lines don't become 3, for instance.)

Basically, the operation should always try to maintain ideal spacing between functions regardless of the amount of blank space at the end of the document.

[Eoin-ONeill-Yokai]

In fact, I will go ahead and update this so that all new functions added conform to the GDScript guidelines with a single new line at the end of the document.

[Eoin-ONeill-Yokai]

@Calinou Thanks for that feedback -- that's a good call and I should have double-checked the gdscript coding standards before pushing this upstream.

I've made changes that still address the concern of unwanted extra space after the last function. Now we can fetch the last line with actual content in it (non-empty line) and insert the appropriate number of new lines (2 lines).

Somewhat related to this, I actually feel that the number of preemptive new lines should be fetched from the scripts' language (via script->get_language()->lines_between_functions() akin to how function text is generated) so that the languages actually specify the standard of how many new lines we should be making.

[KoBeWi]

I'm not sure about this, as the GDScript style guide recommends separating functions with two blank lines (like in Python).

How about making this optional? I use one line and expect the dialog to do the same.

[Eoin-ONeill-Yokai]

I'm not sure about this, as the GDScript style guide recommends separating functions with two blank lines (like in Python).

How about making this optional? I use one line and expect the dialog to do the same.

Anybody have an additional thoughts on this?

Also, should I add the above proposed function to the Language api?

[aaronfranke]

@KoBeWi IMO, Godot should always follow the language standards when modifying script files. If you use one line between functions in your project, it is very easy to simply erase the extra line that Godot creates :)

@Eoin-ONeill-Yokai When you get a chance, this needs to be rebased on the latest master branch. While there are no conflicts, rebasing is important so that reviewers can easily test the PR.

[Eoin-ONeill-Yokai]

@Eoin-ONeill-Yokai When you get a chance, this needs to be rebased on the latest master branch. While there are no conflicts, rebasing is important so that reviewers can easily test the PR.

Yeah no worries, I'll rebase it this weekend.

[Eoin-ONeill-Yokai]

@aaronfranke I've gone ahead and rebased this patch. Did some light testing and nothing seems to be broken since implementation.