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

Jump to bottom

Improve CodeEdit's toggle comments behavior #44557

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Improve CodeEdit's toggle comments behavior #44557

merged 1 commit into from
Jul 12, 2023

Conversation

iwek7
Copy link
Contributor

@iwek7 iwek7 commented Dec 20, 2020 •
edited by YuriSizov
Loading

Draft implementation for godotengine/godot-proposals#2005. Not sure if using is_line_comment method of text_edit is proper (it breaks if line begins with /) but I like idea of encapsulating idea of checking if line is comment in text_edit - so I can either use this method or write new one.

Maintainer edit: Closes godotengine/godot-proposals#110

@iwek7 iwek7 mentioned this pull request Dec 20, 2020
Closed
@Calinou Calinou added this to the 4.0 milestone Dec 20, 2020
@EricEzaM
Copy link
Contributor

From memory is_line_comment includes slash because it is used for the shader editor too which uses c-style comments. I think @Paulb23 is still working on a rewrite and reorganisation of TextEdit which should move some of this functionality out of TextEdit

@iwek7
Copy link
Contributor Author

iwek7 commented Dec 28, 2020

I think ok solution would be to create function in text_edit that provided with list of strings checks if line starts with any of them (ignoring whitespaces). I'd remove is_line_comment function altogether because I think that text_edit does not really need to know what is comment and what is not as it leaks gdscript and shader language details there.

I can also create separate function just for my usage but It duplicates very similar logic.

Either solution is simple, not sure which one to use since some rewrite is underway, maybe if @Paulb23 or other maintainer will decide to review this feature they will advise.

@Paulb23
Copy link
Member

Paulb23 commented Aug 30, 2021 •
edited
Loading

@iwek7 Any chance of rebasing, it should possible to make significant improvements now with the new comment tracking API (#45393)?

@iwek7
Copy link
Contributor Author

iwek7 commented Sep 27, 2021 •
edited
Loading

Sorry did not see your comment. I will try to make rebase and work on this issue this week

@iwek7 iwek7 force-pushed the improved_comment_toggle branch from 3918735 to 36dc42f Compare October 29, 2021 18:04
@iwek7 iwek7 requested a review from a team as a code owner October 29, 2021 18:04
@iwek7 iwek7 force-pushed the improved_comment_toggle branch 2 times, most recently from 7fcfef3 to 084da69 Compare October 29, 2021 18:06
@iwek7
Copy link
Contributor Author

iwek7 commented Oct 29, 2021 •
edited
Loading

I've done it - toggling comments now respects indentation which solves godotengine/godot-proposals#110 - but I forgot about new comment api :D, will take a look at it and update this PR in near future.

@YuriSizov
Copy link
Contributor

@iwek7 Do you still plan to work on this?

@YuriSizov YuriSizov changed the title updating toggle comment behaviour Improve CodeEdit's toggle comments behavior May 28, 2022
@YuriSizov YuriSizov mentioned this pull request May 28, 2022
Closed
@iwek7
Copy link
Contributor Author

iwek7 commented Jun 1, 2022

Hi, I kind of forgot about this issue. I will check code today and see if it is easy to incorporate Pauls proposal.

@iwek7 iwek7 force-pushed the improved_comment_toggle branch from 084da69 to c8e8e70 Compare June 1, 2022 19:30
@iwek7
Copy link
Contributor Author

iwek7 commented Jun 1, 2022

I've pushed some changes. I tried to use text_edit.is_in_comment() function instead of checking delimiters. Please take a look @Paulb23

@iwek7 iwek7 force-pushed the improved_comment_toggle branch from c8e8e70 to 8e41527 Compare June 2, 2022 13:18
@iwek7
Copy link
Contributor Author

iwek7 commented Jun 2, 2022

I've introduced minor selection regression, which is fixed by last addition to this pr. Now all looks good.

Paulb23
Paulb23 requested changes Jun 25, 2022
View reviewed changes
editor/code_editor.cpp
for (int i = begin; i <= end; i++) {
if (!text_editor->get_line(i).begins_with(delimiter)) {
// `+ delimiter.length()` here because comment delimiter is not actually `in comment` so we check first character after it
if (text_editor->is_in_comment(i, text_editor->get_first_non_whitespace_column(i) + delimiter.length()) == -1) {
is_commented = false;
Copy link
Member

Choose a reason for hiding this comment

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

Probably need to also check the delimiter matches with get_delimiter_start_key.

Copy link
Contributor Author

@iwek7 iwek7 Jul 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I've added this check here and in other branch of this if as well, so that for instance shader code like

/* code... */

will have // added at the beggining when toggled

@iwek7 iwek7 force-pushed the improved_comment_toggle branch from 8e41527 to e3ed6db Compare July 1, 2022 21:51
@iwek7
Copy link
Contributor Author

iwek7 commented Jul 31, 2022

I've applied requested changes, code is ready for review again

@akien-mga akien-mga requested a review from Paulb23 July 31, 2022 21:52
@YuriSizov
Copy link
Contributor

@Paulb23 If you could take a look!

@iwek7 iwek7 force-pushed the improved_comment_toggle branch 2 times, most recently from 0e7c6f3 to 97d7a1c Compare February 7, 2023 19:53
@iwek7
Copy link
Contributor Author

iwek7 commented Feb 7, 2023

I've rebased it after multicaret changes, it is ready for review again.

Paulb23
Paulb23 approved these changes Feb 11, 2023
View reviewed changes
Copy link
Member

@Paulb23 Paulb23 left a comment

Choose a reason for hiding this comment

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

thanks for rebasing, lgtm!

@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 11, 2023
@YuriSizov
Copy link
Contributor

Hey, sorry for the wait. Could you please rebase your PR? And while you do that, would be good to adjust the commit message to state that this is about CodeEdit.

@iwek7 iwek7 force-pushed the improved_comment_toggle branch from 97d7a1c to b0df2e1 Compare June 1, 2023 17:12
@iwek7
Copy link
Contributor Author

iwek7 commented Jun 1, 2023

Code is rebased and ready for review again

@akien-mga akien-mga changed the title Improve CodeEdit's toggle comments behavior Improve CodeEdit's toggle comments behavior Jun 2, 2023
@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 2, 2023
@YuriSizov YuriSizov merged commit 1e1d2a8 into godotengine:master Jul 12, 2023
@YuriSizov
Copy link
Contributor

Thanks!

@iwek7 iwek7 deleted the improved_comment_toggle branch July 20, 2023 15:24
@L4Vo5 L4Vo5 mentioned this pull request Aug 7, 2023
Closed
@dalexeev dalexeev mentioned this pull request Aug 7, 2023
Merged
@dalexeev dalexeev mentioned this pull request Oct 18, 2023
Open
@AThousandShips AThousandShips mentioned this pull request Dec 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YuriSizov YuriSizov YuriSizov approved these changes

@Paulb23 Paulb23 Paulb23 approved these changes

Assignees
No one assigned
Labels
enhancement topic:editor usability
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.

Make Toggle Comment respect indentation level in the Script Editor
6 participants
@iwek7 @EricEzaM @Paulb23 @YuriSizov @Calinou @akien-mga