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

C#: Fix line in OpenInExternalEditor #79404

Merged

Conversation

raulsntos
Copy link
Member

@raulsntos raulsntos commented Jul 13, 2023

@raulsntos raulsntos added bug topic:editor needs testing topic:dotnet cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:4.0 cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Jul 13, 2023
@raulsntos raulsntos added this to the 4.x milestone Jul 13, 2023
@raulsntos raulsntos requested a review from a team as a code owner July 13, 2023 01:26
@raulsntos raulsntos force-pushed the dotnet/lines-open-in-external-editor branch from 849363e to fdea8c8 Compare July 13, 2023 02:31
Copy link
Member

@RedworkDE RedworkDE left a comment

Choose a reason for hiding this comment

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

This also works in VS, but the built-in editor now points to the line above.

@raulsntos raulsntos force-pushed the dotnet/lines-open-in-external-editor branch from fdea8c8 to 132a1da Compare July 18, 2023 11:55
@raulsntos raulsntos requested a review from a team as a code owner July 18, 2023 11:55
@raulsntos raulsntos removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jul 18, 2023
@raulsntos
Copy link
Member Author

I looked into this and reached the conclusion that EditorInterface::edit takes a 1-based line number, and every other API (ScriptEditor, CodeTextEditor and TextEditor) takes a 0-based line number.

The ScriptEditor::edit method was subtracting 1 from the line number which seems to be wrong, since every other API in ScriptEditor seems to be 0-based. This was causing the double subtract that made this PR break with the built-in editor.

The double subtract should also be happening with GDScript errors, but in there there is a call to goto_line_centered after the call to edit which essentially resets the line to right one thus hiding the bug.

As a consequence of these changes this is likely no longer cherry-pickable for 3.x and I have removed the label. If this change is desired for 3.x, a backport PR can be made.

Copy link
Member

@RedworkDE RedworkDE left a comment

Choose a reason for hiding this comment

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

Didn't notice any more issues

@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Jul 25, 2023
@YuriSizov YuriSizov requested a review from Paulb23 July 25, 2023 19:49
@akien-mga akien-mga merged commit 2132638 into godotengine:master Aug 2, 2023
@raulsntos raulsntos deleted the dotnet/lines-open-in-external-editor branch August 2, 2023 11:01
@akien-mga
Copy link
Member

Thanks!

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.3.

@YuriSizov YuriSizov removed needs testing cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Oct 19, 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 this pull request may close these issues.

C# Error message point to one line above in Visual Studio Code
4 participants