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.

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

Groups of spaces in scripts automatically changing into tabs/indentation #78657

Closed
PIZZA-ALERT opened this issue Jun 25, 2023 · 23 comments · Fixed by #78675
Closed

Groups of spaces in scripts automatically changing into tabs/indentation #78657

PIZZA-ALERT opened this issue Jun 25, 2023 · 23 comments · Fixed by #78675

Comments

@PIZZA-ALERT
Copy link

PIZZA-ALERT commented Jun 25, 2023

UPDATE: THIS IS RESOLVED WITH GODOT 4.2.

Godot version

4.0.3.stable

System information

Windows 11, ask for more info if needed, I'm not sure what else would be needed here.

Issue description

Issue also asked about on Godot Q&A (currently unresolved): https://ask.godotengine.org/155740/godot-automatically-creating-tabs-script-template-how-that

I have a script template that I use for all my scripts. In this template, I have an instance of a long line of spaces leading to a comment (this is my commenting method of choice, putting all comments on the right). Looks like (without underscores, would be spaces):
my code _ _ _ _ _ _ _ _ _ _ _ # my comment

It worked perfectly in prior versions, though after switching to Godot 4 it has started converting my groups of spaces into tabs. It now looks like this (with the indent carrots), despite me only including spaces there in my .gd file:
my code > > > > > > > > > # my comment

I cannot find a way to disable this "feature" and believe it to truly be a bug. Mr. J. Godfrey (a Godot contributor/Q&A admin I think?) believes so as well. I have taken a look at the indentation section of the editor settings but this is a different feature. I like tabs for indents, and am not trying to change that. I just don't want Godot to auto-change my spaces into indents when it sees a group of them. I understand others may, so I'm proposing there is an option created for that (or please inform me of which one it is if it exists). When I create/edit this template in Notepad, I'm able to verify that it is indeed just normal spaces.

Steps to reproduce

  1. Go to this link and download the script template I created (this program won't let me attach a .gd for some reason):
    https://drive.google.com/file/d/1oXPKAn_3DulkF2oIFNYTFV3QKuNKdGz-/view?usp=sharing

  2. Load it into a folder called "Objects" within your script templates folder (you may need to create this folder, found at Godot master folder location...Godot Engine\editor_data\script_templates\

  3. Create a new script in Godot 4.0.3.stable and select this file as the template. You may need to create it on a node, type shouldn't matter.

  4. Open the new script and see that the "extends" line has a bunch of tab indents when it should just be spaces.

Minimal reproduction project

N/A, this is trivial

@capnm

This comment was marked as outdated.

@AThousandShips
Copy link
Member

AThousandShips commented Jun 25, 2023

This is handled by settings indeed, and desired feature, not a bug, and follows the code style

Mr. J. Godfrey (a Godot contributor/Q&A admin I think?) believes so as well.

I am not convinced their response was based on the exact description as the way you phrased it on the QA page is very different from how it was phrased here, the first image on the QA page also does not work

@capnm
Copy link
Contributor

capnm commented Jun 25, 2023

default-behaviour-and-overriding-it could mention the default WS and how to change it.

@jgodfrey
Copy link

jgodfrey commented Jun 25, 2023

I don't have a horse in this race (since I don't use script templates, as mentioned in the linked Q&A thread), but this feels strange to me.

In my view, an indent refers to some amount of space added ahead of a line (or block) of text to move it away from the margin. I don't see how (or why) indent settings should make automatic adjustments to spacing within a line of text - which is what's happening here.

So, to me, it's perfectly valid for indent settings to make adjustments to this, for example:

if $var == 10:

However, to convert between spaces and tabs within the line seems a bit over-zealous...
var my_var = 10 # some comment

So, while automated modifications to leading and trailing white-space seems perfectly valid (and even expected when using most programmer's editors), I don't think I've ever seen an editor mess with inner line spacing.

Is this really intentional?

@AThousandShips
Copy link
Member

AThousandShips commented Jun 25, 2023

My bad, OP didn't add any description to show this behaviour in any screenshots I saw, and was talking about "indentation" which isn't at the end of a line, hence my confusion

That is a bit more complex yes

@capnm
Copy link
Contributor

capnm commented Jun 25, 2023

However, to convert between spaces and tabs within the line seems a bit over-zealous...
var my_var = 10 # some comment

Yes, this is clearly a bug.

@AThousandShips
Copy link
Member

Note that this does not happen when just converting spaces normally, but must be somehow related to the templates, will look into

@AThousandShips
Copy link
Member

This is due to this part:

ScriptLanguage::ScriptTemplate script_template = ScriptLanguage::ScriptTemplate();
script_template.origin = p_origin;
script_template.inherit = p_inherits;
String space_indent = " ";
// Get meta delimiter
String meta_delimiter;
List<String> comment_delimiters;
p_language->get_comment_delimiters(&comment_delimiters);
for (const String &script_delimiter : comment_delimiters) {
if (!script_delimiter.contains(" ")) {
meta_delimiter = script_delimiter;
break;
}
}
String meta_prefix = meta_delimiter + " meta-";
// Parse file for meta-information and script content
Error err;
Ref<FileAccess> file = FileAccess::open(p_path.path_join(p_filename), FileAccess::READ, &err);
if (!err) {
while (!file->eof_reached()) {
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();
if (indent_value.is_valid_int()) {
int indent_size = indent_value.to_int();
if (indent_size >= 0) {
space_indent = String(" ").repeat(indent_size);
}
} else {
WARN_PRINT(vformat("Template meta-use_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_");
}
script_template.content += line.replace("\t", "_TS_") + "\n";

Will investigate a fix

@AThousandShips AThousandShips self-assigned this Jun 25, 2023
@AThousandShips AThousandShips added this to the 4.1 milestone Jun 25, 2023
@capnm
Copy link
Contributor

capnm commented Jun 25, 2023

I think the auto indent should IMO use the default editor conversion. Here it instead of conversion to tab deletes the 4 spaces.
image

@AThousandShips
Copy link
Member

Please open a separate issue if one doesn't exist, this is a separate area of the code

@jgodfrey
Copy link

This is due to this part:

Will investigate a fix

Yeah, those two line.replace() calls starting with the logic at line 907 seem problematic. Without looking at the implementation, I assume those replace every occurrence in the line. Seems like it'll have to be a bit smarter than that...

@AThousandShips
Copy link
Member

Working on a new fix based on the convert indent code, will update if I solve it

@capnm
Copy link
Contributor

capnm commented Jun 25, 2023

Please open a separate issue if one doesn't exist, this is a separate area of the code

Could we somehow have same code for both cases?

@AThousandShips
Copy link
Member

Not really they're in completely different part of the engine, this is handled in CodeEdit which operates on its own content, much more complicated and not really applicable for the conversion for a template, which doesn't even use tabs but _TS_.

@capnm
Copy link
Contributor

capnm commented Jun 25, 2023

WDYT is the automatic Indentation meant for here. Should it just call the conversion function for editor default WS settings?
(I mean, is this a CodeEdit bug or is the current behavior for some reason intentional)

@AThousandShips
Copy link
Member

There isn't a default conversion function to call, CodeEdit operates on its own text

The template code converts tabs and spaces to _TS_ that then gets converted to indentation on applying the template, so the code isn't compatible unfortunately

@AThousandShips
Copy link
Member

Fix for this specific bug open, if there's an issue in "Auto Indent" please open a dedicated issue to track it

@PIZZA-ALERT
Copy link
Author

Hey everyone, I just woke up to all your messages! Thank you so much for hopping on this so quickly.

I apologize for not including a visual in my post of what happens, I figured my little issue description and the steps to reproduce explained that but I can see that I should have had both. I've updated it to be more clear (hopefully)!

I am a bit confused as the conversation has gone up and down a bit, with some saying it's a bug, others saying it's something else, and others saying to open a new issue for this for some reason...can I please just get a single explanation of what the next steps are and what is truly happening with my code here? Why it's just now doing this when Godot 3.X never had the issue?

Thanks again!!

@AThousandShips
Copy link
Member

AThousandShips commented Jun 25, 2023

No new issue for a different bug, never said anything about a separate issue for this bug, and the code has massively changed since 3.x

@PIZZA-ALERT
Copy link
Author

Hmm ok, sorry if you feel like you've explained but I'm still confused - is my "issue" going to be "fixed" somehow? Should I expect to be able to use my template as intended in later releases?

@AThousandShips
Copy link
Member

It is, there's a PR open for it right now that will close it, what I was talking as a different issue is the "Auto Indent" issue that I mentioned above was a different issue, I'm sorry for causing confusion

@PIZZA-ALERT
Copy link
Author

Thanks a lot! You all rock.

I've never submitted an issue here before, Q&A has always met my needs. Should I expect a message here when the fix is released or something? Or should I just manually check when the next few patches roll out?

@AThousandShips
Copy link
Member

AThousandShips commented Jun 25, 2023

This will close when this is solved, will hopefully be in 4.1

Edit: Scheduled for 4.2, hopefully with a future cherry pick for some 4.1.x version

@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants