Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

File History #961

Closed
wants to merge 12 commits into from
Closed

Conversation

sambonfire
Copy link
Contributor

Description of the Change

I'm submitting this PR right now mostly for feedback; there are a handful of things that I'd still like to fix/cleanup; I just want early feedback!

I wanted to be able to display a window showing the history of a single file. On this file history view, users should be able to "check out" an old version of the file. This is a common theme in game development -- just wanting to roll back one file to test vs. all files in a change.

In order to accomplish this, 2 new commands were added to the git client:

  • log history of a file
  • checkout a set of files at a specific change

I also made some interfaces public; this was mostly because I initially started developing the user interface outside of the github plugin because I wasn't sure you'd want it. I've left these changes in this PR because I think that they'll be worthwhile in the future.

To view file history, users can select files in the project file explorer, right click on them, then select Git -> History. This pops up a new window showing the history of the selected file(s).

Alternate Designs

A new view could have been built into the main github window.

Benefits

Users can quickly view history of a single file and revert to any point in history with just a few clicks.

Possible Drawbacks

Currently, there is no checks to see if the file has modifications. As is, the checkout is destructive and this could lead to data loss. I plan on fixing this by adding a confirmation dialog.

Applicable Issues

#948, #947

@StanleyGoldman
Copy link
Contributor

I haven't forgotten you. 💛

@StanleyGoldman
Copy link
Contributor

I'm working on getting everything that was in master out the door into a release. Then we can start working on this together for the next release. Sorry to keep you hanging.

@sambonfire
Copy link
Contributor Author

sambonfire commented Dec 12, 2018 via email

@StanleyGoldman
Copy link
Contributor

Happy New Year. This is on my radar this week.

Copy link
Contributor

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

I wanted to review the API separately from the UI changes.
It looks good. Also, ApiClient can be made public.

@StanleyGoldman
Copy link
Contributor

I sent you a pull request to fix the build

https://github.com/bonfirestudios/Unity/pull/1

@StanleyGoldman
Copy link
Contributor

After that pull request is merged in...

Adding some screenshots for further discussion

image

image

@StanleyGoldman
Copy link
Contributor

@donokuda put this mockup together for us. I think I like it.

image

So we are going to make a "Pinnable" File Log History view.

@donokuda
Copy link
Contributor

👋Adding a little more clarity to what I was thinking with my mock-up. For the most part, everything that is already built is kept as is (including having the file history appear as a separate window.)

The main thing was making the header feel more like the Unity inspector where we show the file's large icon in the header as well as the filename. Clicking the "Show in Project" button would highlight the file in the project pane (which is the current functionality of clicking the "GIT File history for [filepath]" link)

❓Something I'm not 100% sure about is whether it makes sense to show the full path or just the filename. The benefit for the full path is that it ensures that we're looking at the right file and where it's located. The downside is that we can run out of space quickly. Right now, I'm leaning towards showing just the file name since the "Show in Project" button could also be a compromise that's already built.

@sambonfire
Copy link
Contributor Author

<3 the mockup and ideas (dockable history view). I should have some time in the next couple weeks to work on this!

sam

History window will prompt to overwrite a file if the local contents
have changed.

Also fixed a couple bugs where the window would cause exceptions after a
domain reload.
@sambonfire
Copy link
Contributor Author

sambonfire commented Jan 19, 2019

Added some code here that will see if the file has any local changes, and if so it prompts the user to overwrite it (when doing a Checkout on the revision). I also fixed some bugs where the UI was causing exceptions after a domain reload -- it should behave now.

Are there any other outstanding changes you'd like me to make @StanleyGoldman ?

@StanleyGoldman
Copy link
Contributor

Hey @sambonfire I'm going to close this pull request, but I'm going to continue to graft whatever remaining functionality you added to the new pull request #999

I don't think you will be able to push to that branch, but you can open additional pull requests adding functionality to that branch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants