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

VCS: Port Godot 3.5's VCS features to GDExtension #62157

Merged
merged 5 commits into from
Aug 31, 2022
Merged

VCS: Port Godot 3.5's VCS features to GDExtension #62157

merged 5 commits into from
Aug 31, 2022

Conversation

twaritwaikar
Copy link
Contributor

@twaritwaikar twaritwaikar commented Jun 17, 2022 •
edited
Loading

Pull the newer VCS features and an improved diff viewer from 3.5 to 4

Watch this demo for this intended usage - https://youtu.be/blVtgs4g_GU

I was able to port ALL of the features as they exist in 3.5. The next task is to pull out the UI into the plugin as per godotengine/godot-proposals#4369, but in case that turns out to be too difficult, getting this PR merged will ensure the VCS integration works as expected.

Issues

I tried some fixes to clean the implementation up but there are still some issues that don't seem to be related to the changes in this PR but they can be put into newer GH issues:

  • If a script has been opened in the code editor, and you try to write a commit message, currently, the code editor will steal focus from the commit message textbox on every keystroke, making it impossible to write a proper commit message. I have to switch to the 3D tab or the 2D tab to write a commit message.

image
image

  • RichTextLabel::append_text() is not making the text [center]'d properly. The text surrounded by @@ should have been centered.
  • The push_indent within RichTextLabel's table cells is not working as expected in the split view. The line numbers are overlapping with the content. This doesn't appear to happen in the unified view.

@twaritwaikar twaritwaikar mentioned this pull request Jun 17, 2022
Merged
@twaritwaikar twaritwaikar force-pushed the front-port-vcs-plugin branch from 6806f65 to 6776aff Compare June 17, 2022 19:20
@twaritwaikar twaritwaikar changed the title VCS: Port Godot 3.5 features to GDExtension VCS: Port Godot 3.5's VCS features to GDExtension Jun 17, 2022
@Chaosus Chaosus added this to the 4.0 milestone Jun 22, 2022
@twaritwaikar twaritwaikar mentioned this pull request Jul 9, 2022
Merged
@twaritwaikar twaritwaikar force-pushed the front-port-vcs-plugin branch from 3897cc1 to 337422c Compare July 31, 2022 16:54
@twaritwaikar twaritwaikar marked this pull request as ready for review July 31, 2022 16:54
@twaritwaikar twaritwaikar requested review from a team as code owners July 31, 2022 16:54
@twaritwaikar
Copy link
Contributor Author

I have added a demo of the changes in my PR description

@twaritwaikar twaritwaikar requested a review from a team as a code owner August 9, 2022 21:59
@twaritwaikar twaritwaikar force-pushed the front-port-vcs-plugin branch 4 times, most recently from 62e9963 to a0d67d1 Compare August 13, 2022 17:54
@twaritwaikar twaritwaikar mentioned this pull request Aug 24, 2022
Closed
@twaritwaikar twaritwaikar force-pushed the front-port-vcs-plugin branch from a0d67d1 to 9035135 Compare August 26, 2022 15:47
@akien-mga akien-mga requested a review from groud August 27, 2022 07:18
akien-mga
akien-mga approved these changes Aug 29, 2022
View reviewed changes
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

This is a pretty big PR so not trivial to review, but overall it seems well made, and pretty self-contained so there should be no major problem to merge it.

@twaritwaikar twaritwaikar force-pushed the front-port-vcs-plugin branch from 9035135 to ce7326e Compare August 29, 2022 16:10
@twaritwaikar
Copy link
Contributor Author

I have made all the requested changes ^

@twaritwaikar
Copy link
Contributor Author

Also, @groud had reviewed some of the code that touches things outside of the VCS on chat. There seems to be another bug where the CodeEditor is not updating the script files if they get changed from outside the editor (at least on Windows). I will open a separate bug to track this

@twaritwaikar
Copy link
Contributor Author

Once that is solved, we may not need to call ScriptEditor::reload_scripts() through this code. There needs to be additional testing on the behavior in that case.

@twaritwaikar twaritwaikar force-pushed the front-port-vcs-plugin branch from ce7326e to e342a27 Compare August 29, 2022 16:53
@akien-mga
Copy link
Member

Might need a rebase after #64377 I think.

@twaritwaikar
VCS: Fix empty metadata items being moved across stages
c698e4f
@twaritwaikar
VCS: Replace usage of varray() by using callable_mp::bind()
1bb5e65
@twaritwaikar
VCS: Improve VCS UI/UX by QoL changes
a62b0ec 
The editor will now use the project path i.e. the place where the root of
the repo is supposed to be according to the user. This project path is
also sent into the plugin and so out-of-directory asset folders can also be
maintained this way.
@twaritwaikar twaritwaikar force-pushed the front-port-vcs-plugin branch from e342a27 to a62b0ec Compare August 30, 2022 18:57
@twaritwaikar
Copy link
Contributor Author

@akien-mga Rebased, and replaced update() with queue_redraw()

@akien-mga akien-mga merged commit 36a5160 into godotengine:master Aug 31, 2022
@akien-mga
Copy link
Member

Thanks!

@twaritwaikar twaritwaikar mentioned this pull request Dec 20, 2022
Merged
@AThousandShips AThousandShips mentioned this pull request Oct 13, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akien-mga akien-mga akien-mga approved these changes

@groud groud Awaiting requested review from groud

Assignees
No one assigned
Labels
enhancement topic:editor topic:gdextension
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@twaritwaikar @akien-mga @Chaosus