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

Fix jumping to function definition using Ctrl+LMB or the "Lookup Symbol" button #73196

Merged
merged 1 commit into from
Jul 24, 2023
Merged

Fix jumping to function definition using Ctrl+LMB or the "Lookup Symbol" button #73196

merged 1 commit into from
Jul 24, 2023

Conversation

Vilcrow
Copy link
Contributor

@Vilcrow Vilcrow commented Feb 13, 2023 •
edited
Loading

Fixes #72353.
Fixes #73167.
Fixes #73480.

I found the cause of bug #73167. The problem is that when we click on the Lookup Symbol button in the context menu, the method String CodeEdit::get_text_for_symbol_lookup() returns position of cursor which is located on the Lookup Symbol and wrong for code parsing.

bug

Such a bug found in the 3.5.1.stable too.

This was referenced Feb 13, 2023
Closed
Closed
@akien-mga akien-mga added this to the 4.0 milestone Feb 13, 2023
@akien-mga
Copy link
Member

akien-mga commented Feb 13, 2023 •
edited
Loading

Since we're at RC stage, and #72789 fixed the relatively minor #72353, I'm thinking the safest path might be to revert #72789 for now (and reopen #72353), and then keep this new PR to redo the relevant changes and make sure they're regression free for 4.1 (and possibly a 4.0.x cherry-pick). WDYT?

@Vilcrow
Copy link
Contributor Author

Vilcrow commented Feb 13, 2023 •
edited
Loading

@akien-mga ok. So I have to wait for that and rebase my branch?

This was referenced Feb 13, 2023
Merged
Closed
@akien-mga
Copy link
Member

Done, you can rebase and redo the relevant changes to fix #72353 + the regressions.

@akien-mga akien-mga modified the milestones: 4.0, 4.1 Feb 13, 2023
@Vilcrow
Copy link
Contributor Author

Vilcrow commented Feb 13, 2023 •
edited
Loading

It seems to me that this is not a solution for #73167. I will try to fix it too.

@Vilcrow Vilcrow force-pushed the fix-lookup-symbol branch 2 times, most recently from 71d2f05 to 6abc667 Compare February 14, 2023 03:05
@Vilcrow Vilcrow changed the title Fixed the jumping to function definition using 'Ctrl+LMB'. Fixed the jumping to function definition using 'Ctrl+LMB' and the 'Lookup Symbol' button. Feb 14, 2023
@Vilcrow Vilcrow marked this pull request as ready for review February 14, 2023 03:11
@Vilcrow Vilcrow requested review from a team as code owners February 14, 2023 03:11
@Vilcrow Vilcrow closed this Feb 14, 2023
@Vilcrow Vilcrow reopened this Feb 14, 2023
@Vilcrow Vilcrow marked this pull request as draft February 14, 2023 04:20
@Vilcrow Vilcrow force-pushed the fix-lookup-symbol branch 2 times, most recently from a330bdd to 1cb9651 Compare February 14, 2023 06:30
@Vilcrow Vilcrow marked this pull request as ready for review February 14, 2023 06:32
@Vilcrow Vilcrow requested a review from a team as a code owner February 14, 2023 06:32
@akien-mga
Copy link
Member

Some other related PRs: #68311 #69473

@Vilcrow Vilcrow mentioned this pull request Feb 17, 2023
Closed
@Vilcrow Vilcrow force-pushed the fix-lookup-symbol branch 3 times, most recently from a44e103 to 266fff0 Compare February 17, 2023 21:11
@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 19, 2023
KoBeWi
KoBeWi reviewed Jun 29, 2023
View reviewed changes
scene/gui/code_edit.cpp Outdated Show resolved Hide resolved
KoBeWi
KoBeWi approved these changes Jun 29, 2023
View reviewed changes
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

I can confirm this fixes the issues, but can't tell if the implementation is correct.

@KoBeWi KoBeWi mentioned this pull request Jul 5, 2023
Closed
@YuriSizov YuriSizov requested a review from Paulb23 July 7, 2023 18:50
@YuriSizov
Copy link
Contributor

cc @Paulb23

Paulb23
Paulb23 requested changes Jul 10, 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.

ScriptEditor changes look good, agree with KoBeWi comments.
We could probably convert get_text_for_code_completion to use the new method as well.

Given it touches CodeEdit unit tests would also be nice to add.

doc/classes/CodeEdit.xml Outdated Show resolved Hide resolved
@Vilcrow Vilcrow force-pushed the fix-lookup-symbol branch from 266fff0 to 56e2fad Compare July 12, 2023 18:40
Paulb23
Paulb23 approved these changes Jul 23, 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.

ScriptEditor side looks good.

@YuriSizov YuriSizov changed the title Fixed the jumping to function definition using 'Ctrl+LMB' and the 'Lookup Symbol' button. Fix jumping to function definition using Ctrl+LMB or the "Lookup Symbol" button Jul 24, 2023
@YuriSizov YuriSizov merged commit 2bd904e into godotengine:master Jul 24, 2023
@YuriSizov
Copy link
Contributor

Thanks!

@akien-mga akien-mga mentioned this pull request Aug 2, 2023
Merged
@Vilcrow Vilcrow deleted the fix-lookup-symbol branch September 10, 2023 12:47
@nagidev
Copy link
Contributor

nagidev commented Sep 24, 2023

I'm facing an issue regarding lookup symbol and autocomplete suggestions not working on referenced children nodes.
Could this be caused by this PR?
#82258

@Vilcrow
Copy link
Contributor Author

Vilcrow commented Oct 4, 2023

@nagidev No. I checked this for the previous commit, the result is the same. The reason is not in this PR.

@HolonProduction HolonProduction mentioned this pull request Dec 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AThousandShips AThousandShips AThousandShips left review comments

@KoBeWi KoBeWi KoBeWi approved these changes

@Paulb23 Paulb23 Paulb23 approved these changes

Assignees
No one assigned
Labels
bug regression topic:editor topic:gdscript
Projects
None yet
Milestone
4.2
7 participants
@Vilcrow @akien-mga @YuriSizov @nagidev @KoBeWi @Paulb23 @AThousandShips