Autofill comment when inserting a new line in documentation #95955

Cronos87 · 2024-08-22T16:17:04Z

This PR implements the following proposal godotengine/godot-proposals#8244.

Here it is in action:

CleanShot.2024-08-22.at.18.06.52.mp4

Please keep in mind that this is my first contribution and my first step into the C++ world, but I really want to contribute to Godot :)

Any feedback, help, or advice is very welcome!

Bugsquad edit, closes: Autofill documentation comment block when returning to a new line godot-proposals#8244

Mickeon · 2024-08-22T23:14:12Z

scene/gui/code_edit.cpp

+		// Insert a documentation comment if the caret was previously on a comment block.
+		String prev_line = get_line(get_caret_line(i) - 1);
+
+		if (prev_line.strip_edges().begins_with("##")) {
+			insert_text_at_caret("## ", i);
+		}


This is a feature specific to GDScript and the Editor. I am not savvy enough to say where this should go, but definitely not in the CodeEdit class, which is designed to be much more generalized.

Yes, the initial purpose of this PR was to gather information on how to do it best and to learn the codebase. @kitbdev gave me a clue about it; see #95955 (comment). I can enhance it now :)

Thank you for your feedback!

kitbdev · 2024-08-22T23:19:34Z

CodeEdit should be language independent, so we can't use ## in it.
The delimiter system is how we check for regular comments, so it looks like it needs to know about doc comments and the String to insert on a new line.

Then it can probably be set here:

godot/editor/plugins/script_text_editor.cpp

Lines 252 to 262 in 568589c

    
           List<String> doc_comments; 
        
           script->get_language()->get_doc_comment_delimiters(&doc_comments); 
        
           for (const String &doc_comment : doc_comments) { 
        
           	String beg = doc_comment.get_slice(" ", 0); 
        
           	String end = doc_comment.get_slice_count(" ") > 1 ? doc_comment.get_slice(" ", 1) : String(); 
        
           	text_edit->add_comment_delimiter(beg, end, end.is_empty()); 
        
           	if (!end.is_empty() && !text_edit->has_auto_brace_completion_open_key(beg)) { 
        
           		text_edit->add_auto_brace_completion_pair(beg, end); 
        
           	} 
        
           }

Cronos87 · 2024-08-23T06:47:31Z

CodeEdit should be language independent, so we can't use ## in it. The delimiter system is how we check for regular comments, so it looks like it needs to know about doc comments and the String to insert on a new line.

It totally makes sense. I'll give it a try as soon as I can in the file you just pointed out.

Thank you for your feedback!

Edit: I took a look at the file you told me, @kitbdev. I can get the delimiters in the code_edit.cpp by doing get_comment_delimiters(). However, in my case in GDScript, the array will be ['##', '#']. It doesn't inform me which one is the doc comment, and I didn't find a way to identify it. Maybe you have a clue regarding this?

I continue to search for the answer in any case :)

dalexeev · 2024-08-23T07:35:54Z

Perhaps this makes sense not only for doc comments, but for regular ones too. Kate has interesting behavior:

Enter copies the indentation of the previous line. If it ends with {, :, etc. (depending on the language), the indentation level is increased.
Shift+Enter copies the indentation of the previous line with leading symbols/spaces up to the first letter/number.
Ctrl+Enter inserts only a line break, like in simple text editors like Notepad.

Shift+Enter is also useful for Markdown and YAML lists.

RedMser · 2024-08-23T19:12:09Z

I can get the delimiter in the code_edit.cpp by doing get_comment_delimiters().

Why not use get_doc_comment_delimiters() which returns only ## for GDScript? That's also what's used in the code that kitbdev linked to.

Cronos87 · 2024-08-23T19:50:20Z

Why not use get_doc_comment_delimiters() which returns only ## for GDScript? That's also what's used in the code that kitbdev linked to.

I agree, but this method is only available in script_text_editor.cpp as far as I know. I suppose I can add a method to code_edit.cpp to retrieve the data, but I don't know if this is a good practice.

I'm digging into the codebase; I need time to get used to it, I suppose 😊

Cronos87 · 2024-08-24T22:40:05Z

I did my best to enhance it and to be more flexible. Feedback is very welcome 🙏

kitbdev

Some tests in tests\scene\test_code_edit.h for testing the newline functionality would be nice too.

kitbdev · 2024-08-25T17:24:57Z

scene/gui/code_edit.cpp

@@ -1863,6 +1879,35 @@ int CodeEdit::is_in_comment(int p_line, int p_column) const {
 	return _is_in_delimiter(p_line, p_column, TYPE_COMMENT);


Suggested change

return _is_in_delimiter(p_line, p_column, TYPE_COMMENT);

return _is_in_delimiter(p_line, p_column, TYPE_COMMENT) || _is_in_delimiter(p_line, p_column, TYPE_DOC_COMMENT) ;

Maybe this should also return true for doc comments, so its behavior doesn't change. Or the uses of is_in_comment need to be updated to also check for doc comments.

I agree, it makes sense! Thanks for pointing this out!

hum… for some reason I don't know yet, adding this check breaks the tests and some features too, like line folding.

I'll take a closer look at it after work.

OK I understood! _is_in_delimiter returns an integer and not a boolean, so it can be used this way.

I just pushed the fix :)

kitbdev · 2024-08-25T17:27:05Z

scene/gui/code_edit.cpp

@@ -1154,6 +1154,22 @@ void CodeEdit::_new_line(bool p_split_current_line, bool p_above) {
 			set_caret_line(get_caret_line(i) - 1, false, true, 0, i);
 			set_caret_column(get_line(get_caret_line(i)).length(), i == 0, i);
 		}
+
+		// Insert a documentation comment if the caret was previously on a comment block.
+		int prev_line = get_caret_line(i) - 1;


This should adapt if p_above is true (ctrl+shift+enter).

Fix has been pushed :)

Cronos87 · 2024-08-27T14:33:32Z

Perhaps this makes sense not only for doc comments, but for regular ones too. Kate has interesting behavior:

Enter copies the indentation of the previous line. If it ends with {, :, etc. (depending on the language), the indentation level is increased.

Shift+Enter copies the indentation of the previous line with leading symbols/spaces up to the first letter/number.

Ctrl+Enter inserts only a line break, like in simple text editors like Notepad.

Shift+Enter is also useful for Markdown and YAML lists.

Sorry for the delayed answer, @dalexeev! Yes, indeed, that would be nice, but the proposal is limited to the documentation comment. I don't know what people think about it and if we should extend the feature to comments too.

I am open to discussion.

editor/plugins/script_text_editor.cpp

dalexeev · 2024-08-27T16:04:47Z

scene/gui/code_edit.cpp

@@ -1154,6 +1154,22 @@ void CodeEdit::_new_line(bool p_split_current_line, bool p_above) {
 			set_caret_line(get_caret_line(i) - 1, false, true, 0, i);
 			set_caret_column(get_line(get_caret_line(i)).length(), i == 0, i);
 		}
+
+		// Insert a documentation comment if the caret was previously on a documentation comment block.


Note that a doc comment can be multiline (like /** */).

dalexeev · 2024-08-27T16:22:06Z

Yes, indeed, that would be nice, but the proposal is limited to the documentation comment. I don't know what people think about it and if we should extend the feature to comments too.

Regarding the Kate-like behavior, I agree that it's too much and requires a separate proposal. But I think it would be reasonable to discuss extending the feature to regular comments as part of this proposal/PR.

In fact, it would even simplify the implementation. When I added doc comment highlighting, I didn't add a new API to CodeEdit, since it was not necessary. The "doc comment" term was only introduced in the editor, not the scene.

Also, this can break compatibility in some ways. Previously, doc comments were considered comments, so is_in_comment() returned true for them. Now doc comments are separated into a new category, although it would be more logical to consider them a subset of comments.

Cronos87 · 2024-08-27T19:38:40Z

Regarding the Kate-like behavior, I agree that it's too much and requires a separate proposal. But I think it would be reasonable to discuss extending the feature to regular comments as part of this proposal/PR.

I totally agreed with you. I can start working on this.

In fact, it would even simplify the implementation.

Yes, it would be easier to implement it for both.

Also, this can break compatibility in some ways. Previously, doc comments were considered comments, so is_in_comment() returned true for them. Now doc comments are separated into a new category, although it would be more logical to consider them a subset of comments.

Sadly, my knowledge regarding the Godot codebase is limited; that's why I tried to implement this proposal to gain experience in both the Godot codebase and C++.

I would be glad to have some clues about how to implement it—just a clue, not the full implementation—and I will give it a try to respect the best practices in this codebase.

For example, is Code Edit the right place to implement it? And can you develop your idea to consider them as a subset, please?

By the way, what do you think, is it necessary to add an option to enable / disable this feature?

Many thanks for the help!

kitbdev · 2024-08-29T18:22:00Z

scene/gui/code_edit.cpp

+			for (const String doc_comment : get_doc_comment_delimiters()) {
+				const String beg = doc_comment.get_slice(" ", 0);
+
+				if (prev_line_content.begins_with(beg)) {
+					insert_text_at_caret(doc_comment + " ", i);
+					break;
+				}
+			}


There should be a separate kind of key for the delimiters, since other styles have a different characters for start /**, end */, and in between new lines *.

Maybe the presence of this new line key can be what enables this behavior, and then we may not need a separate doc comment type?

kitbdev · 2024-08-29T18:22:49Z

I think this behavior should only be on doc comments as initially proposed, if it is on regular comments it may be annoying to add new lines for regular code around comments. Though the implementation can be generalized for all types of delimiters if it is easier.

Also this shouldn't cause compatibility issues, CodeEdits behavior should stay the same, since the user hasn't specified to add doc comments instead of regular comments, so all the regular comment delimiter functions should work the same.

By the way, what do you think, is it necessary to add an option to enable / disable this feature?

It shouldn't be needed to have a dedicated enable option, a user can just not use doc comments, or not have a newline delimiter key, depending on how it is implemented.

Cronos87 · 2024-08-30T12:37:03Z

It seems like opinions are divided regarding expanding this behavior to regular comments. Originally, my idea was to reproduce the VSCode behavior that I used for years, and I always missed the autofill for the documentation block.

I'm new to Godot Core development, so how is this kind of situation resolved? How do we decide which way to take?

I'm waiting for new opinions before start working on this PR again :)

Cronos87 · 2024-10-07T19:16:55Z

If the current implementation for the doc comment is fine for everyone, I will add some tests, but I prefer to be sure at the moment it is fine before digging into the tests world.

dalexeev · 2024-11-10T18:55:43Z

See also:

Automatically continue comment blocks when pressing Enter within a comment block in the script editor godot-proposals#4955
When hitting enter on a line containing nothing but a comment, make the next line have a comment godot-proposals#11129

AThousandShips

Will try to test the functionality next week

AThousandShips · 2024-11-16T14:53:44Z

scene/gui/code_edit.h

@@ -457,6 +460,9 @@ class CodeEdit : public TextEdit {
 	Point2 get_delimiter_start_position(int p_line, int p_column) const;
 	Point2 get_delimiter_end_position(int p_line, int p_column) const;

+	Vector<String> get_block_key_delimiters() const;
+	void set_block_key_delimiters(const List<String> *p_delimiters);


Suggested change

void set_block_key_delimiters(const List<String> *p_delimiters);

void set_block_key_delimiters(const List<String> &p_delimiters);

No reason to pass it as a pointer unless it's optional, in that case it needs a null check

But this should really use Vector<String> IMO, it transfers to a Vector anyway so there's going to be a conversion anyway, so I'd say:

Suggested change

void set_block_key_delimiters(const List<String> *p_delimiters);

void set_block_key_delimiters(const Vector<String> &p_delimiters);

I'm wondering if it is viable to convert it to a Vector, as the API requires a List from the key delimiters. So I need to convert it to a Vector and then pass it to the method. It seems to complicate it a little bit. What do you think?

See the update here: https://github.com/godotengine/godot/pull/95955/files#diff-fa2cafb4b2d8b59e5baeba0b861f7f2f6bf6c213d6387254fee5a7682478e588R264-R273

scene/gui/code_edit.h

scene/gui/code_edit.cpp

Cronos87 · 2024-11-16T21:30:26Z

Many thanks for the review 🙏 I'm pushing soon the feedbacks!

Edit: Once again, I'm too used to Gitlab 😅

AThousandShips · 2024-12-03T16:41:42Z

Tested and works as expected generally, but I still hold that it's unintuitive and confusing to not have any way to abort this, but to me it's not necessarily a dealbreaker, but I'd suggest having some way to stop the doc comment and return to normal

At minimum though I think this should be controlled by a setting as it is pretty annoying to get stuck in this mode, and either add support for exiting somehow (possibly using Shift+Enter to insert a normal line, or that enter on a second consecutive entered line makes it a normal line), or making it a dedicated key to do this (again Shift+Enter was suggested above)

But adding an option to control this would be sufficient for me, but I do think that's a necessity

kitbdev · 2024-12-03T21:50:25Z

After thinking about it, I think this should only be for multi-line comments and maybe splitting single line comments. This would help match other editors too.
For single line comments, the "Smart newline" behavior on a separate shortcut would be better so it has to be done intentionally. ui_text_newline_continue_comment or something.
This way it works for regular comments as well as doc comments.

Doc comments that are only a single line are pretty common, probably more common than ones that are multiple lines. So having this behavior by default on single line doc comments can be unwanted and surprising.

Edit: Pressing enter again to remove the comment instead could also work, but the script editor hasn't done stuff like that before, so it may be more work.

Cronos87 · 2024-12-04T14:51:44Z

Thank you for the review and the thoughts shared about this proposal 🙏
It seems that the behavior chosen is the one closest to Kate. That's totally fine for me 😊

I will come back with a new implementation as soon as I am satisfied with what I did!

Edit: I have no words, yeah, same shortcut again… 😅

kitbdev · 2024-12-04T15:58:16Z

Just to be clear, the current approach is still good for multi-line comments, so the new implementation can be in a new PR.

Cronos87 · 2024-12-04T16:38:54Z

You mean doc comments and regular comments, but only multiline like /** */, right?

If that is the case, it will only be triggered for C# in the current implementation.

kitbdev · 2024-12-04T17:03:17Z

It will also work on the shader editor.

Cronos87 · 2024-12-19T19:19:57Z

I didn't abandon this MR; I've been quite busy with my work lately. I'll come back to it soon 😊

AThousandShips · 2024-12-20T19:15:20Z

Don't worry take your time!

…tion comment line

Cronos87 requested a review from a team as a code owner August 22, 2024 16:17

AThousandShips added enhancement topic:gdscript topic:editor labels Aug 22, 2024

AThousandShips added this to the 4.x milestone Aug 22, 2024

Mickeon reviewed Aug 22, 2024

View reviewed changes

Cronos87 changed the title ~~Autofill comment when inserting a new line in documentation~~ [DRAFT] Autofill comment when inserting a new line in documentation Aug 24, 2024

Cronos87 force-pushed the feature/auto-fill-documentation-comment-on-new-line branch from bdb21c6 to fb52d24 Compare August 24, 2024 22:36

Cronos87 requested a review from a team as a code owner August 24, 2024 22:37

Cronos87 marked this pull request as draft August 24, 2024 23:13

Cronos87 changed the title ~~[DRAFT] Autofill comment when inserting a new line in documentation~~ Autofill comment when inserting a new line in documentation Aug 25, 2024

Cronos87 marked this pull request as ready for review August 25, 2024 10:06

Cronos87 requested a review from a team as a code owner August 25, 2024 12:32

Cronos87 force-pushed the feature/auto-fill-documentation-comment-on-new-line branch from b644c56 to b85b8a3 Compare August 25, 2024 12:48

kitbdev reviewed Aug 26, 2024

View reviewed changes

dalexeev reviewed Aug 27, 2024

View reviewed changes

editor/plugins/script_text_editor.cpp Outdated Show resolved Hide resolved

dalexeev reviewed Aug 27, 2024

View reviewed changes

Cronos87 marked this pull request as draft August 27, 2024 19:39

kitbdev reviewed Aug 29, 2024

View reviewed changes

Cronos87 force-pushed the feature/auto-fill-documentation-comment-on-new-line branch 2 times, most recently from 36e42e2 to 6b67dd6 Compare October 11, 2024 12:05

Cronos87 force-pushed the feature/auto-fill-documentation-comment-on-new-line branch from 6b67dd6 to f3526b2 Compare October 18, 2024 16:04

Cronos87 force-pushed the feature/auto-fill-documentation-comment-on-new-line branch from f3526b2 to d12aa00 Compare November 1, 2024 13:32

Cronos87 force-pushed the feature/auto-fill-documentation-comment-on-new-line branch from d12aa00 to ab64319 Compare November 16, 2024 11:13

Mickeon requested a review from AThousandShips November 16, 2024 13:37

AThousandShips reviewed Nov 16, 2024

View reviewed changes

Cronos87 closed this Nov 16, 2024

Cronos87 reopened this Nov 16, 2024

Mickeon requested a review from AThousandShips December 3, 2024 13:04

Cronos87 closed this Dec 4, 2024

Cronos87 reopened this Dec 4, 2024

Cronos87 force-pushed the feature/auto-fill-documentation-comment-on-new-line branch from d68a18d to 2216cf7 Compare December 4, 2024 14:58

Cronos87 added 4 commits December 23, 2024 23:26

Autofill comment when inserting a new line while being on a documenta…

fea517b

…tion comment line

Move behavior into CodeEdit

84545f9

Remove autofill on comments

f0d5fe9

Apply feedbacks

846d8d8

Cronos87 force-pushed the feature/auto-fill-documentation-comment-on-new-line branch from 2216cf7 to 846d8d8 Compare December 23, 2024 22:27

Fix rebase error

8456081

		@@ -1863,6 +1879,35 @@ int CodeEdit::is_in_comment(int p_line, int p_column) const {
		return _is_in_delimiter(p_line, p_column, TYPE_COMMENT);

	void set_block_key_delimiters(const List<String> *p_delimiters);
	void set_block_key_delimiters(const List<String> &p_delimiters);

	void set_block_key_delimiters(const List<String> *p_delimiters);
	void set_block_key_delimiters(const Vector<String> &p_delimiters);

Autofill comment when inserting a new line in documentation #95955

Are you sure you want to change the base?

Autofill comment when inserting a new line in documentation #95955

Conversation

Cronos87 commented Aug 22, 2024 • edited by AThousandShips Loading

Mickeon Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kitbdev commented Aug 22, 2024

Cronos87 commented Aug 23, 2024 • edited Loading

dalexeev commented Aug 23, 2024

RedMser commented Aug 23, 2024 • edited Loading

Cronos87 commented Aug 23, 2024

Cronos87 commented Aug 24, 2024 • edited Loading

kitbdev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cronos87 Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Cronos87 commented Aug 27, 2024

Choose a reason for hiding this comment

dalexeev commented Aug 27, 2024

Cronos87 commented Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

kitbdev commented Aug 29, 2024

Cronos87 commented Aug 30, 2024

Cronos87 commented Oct 7, 2024

dalexeev commented Nov 10, 2024

AThousandShips left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cronos87 Nov 16, 2024 • edited Loading

Choose a reason for hiding this comment

Cronos87 commented Nov 16, 2024 • edited Loading

AThousandShips commented Dec 3, 2024

kitbdev commented Dec 3, 2024 • edited Loading

Cronos87 commented Dec 4, 2024 • edited Loading

kitbdev commented Dec 4, 2024

Cronos87 commented Dec 4, 2024

kitbdev commented Dec 4, 2024

Cronos87 commented Dec 19, 2024 • edited Loading

AThousandShips commented Dec 20, 2024

Cronos87 commented Aug 22, 2024 •

edited by AThousandShips

Loading

Mickeon Aug 22, 2024 •

edited

Loading

Cronos87 commented Aug 23, 2024 •

edited

Loading

RedMser commented Aug 23, 2024 •

edited

Loading

Cronos87 commented Aug 24, 2024 •

edited

Loading

Cronos87 Aug 27, 2024 •

edited

Loading

Cronos87 commented Aug 27, 2024 •

edited

Loading

Cronos87 Nov 16, 2024 •

edited

Loading

Cronos87 commented Nov 16, 2024 •

edited

Loading

kitbdev commented Dec 3, 2024 •

edited

Loading

Cronos87 commented Dec 4, 2024 •

edited

Loading

Cronos87 commented Dec 19, 2024 •

edited

Loading