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

VCS: Add push, pull, fetch and improved diff view to VCS UI #53900

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

twaritwaikar
Copy link
Contributor

@twaritwaikar twaritwaikar commented Oct 16, 2021

This commit was created by merging the commits presented in #39255 for
the GSoC 2020 VCS Improvement project

VCS: Make EditorVCSInterface store less amount of internal state

VCS: Add force push checkbox + more frequent VCS updates

Add force push checkbox in the Commit dock. Also add some missing
opportunities for checking the VCS state again on from UI inputs

VCS: Fix script contents not being updated on merge conflict

VCS: Add branch creation VCS interface calls

VCS: Add VCS remote creation and remote selection menus

VCS: Show more commit information + Fix truncated commit offsets

VCS: Make VCS less noisy + Fix diff view refreshes

Co-authored by @Janglee123 🎉


This PR is containing the first few out of our commits touching the EditorVCSInterface, to update it for the next version.

Once this is merged, I will be sending the next batch of changes (instead of sending them all at once), so that I am able to address any review comments and slowly get the rest of the work reviewed/merged

@twaritwaikar
Copy link
Contributor Author

@pycbouh Based on a discussion with Akien, we agreed to add this in 3.5, not 3.4 which is in a feature freeze ;)

@twaritwaikar
Copy link
Contributor Author

twaritwaikar commented Nov 14, 2021

This force push fixes the documentation so the CI checks should be ok now

@generrosity
Copy link

May I suggest a few things to make the learning curve a little easier? Appologies if you have already worked passed these. I've attempted to read the XML and code files where possible.

Please include a (?) help link to take people to the help webpage for VCS. I think it would be greatly appreciated.

Small projects would benifit defaulting to a 'Stage and Commit All Changes' button option once a branch is selected. In another program I use for Git, 'Commit and Push' is the default action. Obviously, advanced users may wish to choose to slow this process down. I think this would speed up workflow for majority of small project users, and even medium projects may wish to keep their Remote branch up to date rapidly. Some workflows dont even mention staging.
image

I don't understand all the icons. You look to have the 'move item down/up list' icons to move files into staging. Or is it to 'move all files' as it miight always be availible? Are these then used again for Push/Pull/Fetch/Sync at the bottom?

I would like to see when a user hovers over the "Staging" label that a popup opens including the concequence that staging the file stores the file "at that point in time" pending commit.

Is it possible to require commit comments? OnClick the default comment message should be cleared out (I know this is an engine limitation for suggested text). Would it be contencious to replace "add a committ message" with something that has a subject and body, indicates what 'impertive' is, guiding our new (and old) users to better messages?

I am a little worried if tech-specific terms are used instead of human-readable ones. For instance - do you ask for the Remote, or the display name to show for the repository url? What does that default to - ORIGIN? Is it possible to show the repository url along side the remote in a dim colour? Are you committing changes or committing staged changes? Several of the VCS now specifically blend parameters and natural language together.

Thanks for considering.

@twaritwaikar
Copy link
Contributor Author

Leaving this here so I remember to update this PR to use #54056 in a later commit, or in a separate PR

(@generrosity I will respond to the comment above ^^ as soon as I get time this week. Most of these suggestions are welcome! Just need a bit of clarity on a couple others)

@twaritwaikar twaritwaikar requested a review from a team as a code owner November 30, 2021 20:12
@twaritwaikar twaritwaikar force-pushed the update-vcs-1 branch 3 times, most recently from 1ef9585 to 25fcb9d Compare November 30, 2021 21:03
@twaritwaikar twaritwaikar force-pushed the update-vcs-1 branch 2 times, most recently from 694e429 to 550381c Compare December 1, 2021 13:33
@twaritwaikar
Copy link
Contributor Author

@generrosity Thanks for the feedback!

Please include a (?) help link to take people to the help webpage for VCS. I think it would be greatly appreciated

I actually not sure if there is a common way of handling such behaviour in code, will need to check that with other contributors. Likely opening a new proposal?

Small projects would benifit defaulting to a 'Stage and Commit All Changes' button.

We actually have this already and that is through the Ctrl+Enter editor shortcut in the commit message box. While writing a commit message, you can press Ctrl+Enter to automatically stage and commit all modified files if the staging area is empty and If the staging area is non-empty, the files in the staging area are committed.

I don't understand all the icons. You look to have the 'move item down/up list' icons to move files into staging. Or is it to 'move all files' as it miight always be availible? Are these then used again for Push/Pull/Fetch/Sync at the bottom?

The icons are closely resembling the Atom Git integration which you may find here. However yes, these icons are reused for push/pull/fetch for now. All the icons have tooltips to disambiguate their functions but slightly different icons would certainly bring a better UX. However, I haven't really done graphic designing in quite a while :P. This can certainly be an improvement to make later on.

I would like to see when a user hovers over the "Staging" label that a popup opens including the concequence that staging the file stores the file "at that point in time" pending commit.

Currently, clicking the files under the staging area show the diff of the staged file from the latest commit tree. Would something like that be sufficient?

Is it possible to require commit comments?

The commit button becomes disabled if the commit message is empty, so the user is required to provide a commit message. However, it might become a bit too restrictive to force multi-line commits on the user. Also, anyone who wants to make multiline commits would find that the commit message box is actually a TextEdit and not a LineEdit, so pressing enter will add a line break.

I am a little worried if tech-specific terms are used instead of human-readable ones.

From the examples you have provided, I feel these are a lot of UX related things that can be iterated as and when users show signs of struggle in using the UI. These are interesting problems and need attention to fix, however, I have been wanting to first complete the base of the feature set first so that not only me but also other contributors and users can later also help with the UX of these features. This definitely needs a bit of discussion in an open forum like the godot-proposals repository.

@twaritwaikar
Copy link
Contributor Author

twaritwaikar commented Dec 1, 2021

Here's the Linux build and a demo project to test this on, for anyone reviewing: https://drive.google.com/file/d/1YVTmLNo2RYMt_p3-csHuxQFFvoxT4JXL/view?usp=sharing

(Github is having a tough time handling this tiny archive for some reason, so I added it in my drive)

editor/editor_settings.cpp Outdated Show resolved Hide resolved
editor/editor_vcs_interface.cpp Outdated Show resolved Hide resolved
editor/editor_vcs_interface.cpp Outdated Show resolved Hide resolved
editor/editor_vcs_interface.cpp Outdated Show resolved Hide resolved
doc/classes/EditorVCSInterface.xml Outdated Show resolved Hide resolved
doc/classes/EditorVCSInterface.xml Outdated Show resolved Hide resolved
editor/plugins/version_control_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/version_control_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/version_control_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/version_control_editor_plugin.cpp Outdated Show resolved Hide resolved
This commit was created by merging the commits presented in godotengine#39255 for
the GSoC 2020 VCS Improvement project

VCS: Make EditorVCSInterface store less amount of internal state

VCS: Add force push checkbox + more frequent VCS updates

Add force push checkbox in the Commit dock. Also add some missing
opportunities for checking the VCS state again on from UI inputs

VCS: Fix script contents not being updated on merge conflict

VCS: Add branch creation VCS interface calls

VCS: Add VCS remote creation and remote selection menus

VCS: Show more commit information + Fix truncated commit offsets

VCS: Make VCS less noisy + Fix diff view refreshes

VCS: Fix mismatched argument names in VCS helpers

VCS: Add SSH transport support for remote operations

Also, moves the editor's VCS settings registrations to
project_settings.cpp and editor_settings.cpp

VCS: Change TTR() to vformat() for branch and remote removal text

VCS: Add VCS branch icon instead of using Tree node icon

Co-authored-by: @ChronicallySerious
@akien-mga akien-mga merged commit edd9534 into godotengine:3.x Jan 10, 2022
@akien-mga
Copy link
Member

Thanks for all the work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants