Feature/initial Git VCS provider

[akinsho]

Having had a bit of spare time I've refactored this PR to move the git specific functionality into the oni-plugin-git and use instead an abstracted VersionControlManager to render ui elements as per #2110, just wanted to point out that this is still an initial implementation so doesn't have all the bells and whistles but hopefully can be view a further step towards achieving #2110

Sidebar plugin

Status bar changes

But what I have added so far:

• Git VCS is now derived from oni-plugin-git
• VCS service in oni now handles rendering statusbar, sidebar, menu
• functionality changes deletions and additions show in status bar
• Sidebar pane which shows modified files, selecting one stages the file.
• Menu triggered via oni.{vcs}.branches show local branches, selection checkout the branch

Additional Changes (development-related):

• I've switched some oni package.json commands to npm-run-all (which uses yarn or npm depending on the command) has a much tidier syntax which could potentially make the package.json readable again.

example:  {
	build:js: "yarn do stuff",
	build:html: "yarn do stuff",
	build:css: "yarn do stuff",
	// with npm-run-all: accepts globs * and ** to run matching commands
	// run-p runs in parallel and run-s in series
	build: "run-p build:*"
}

Todo

• Fix buffer layer test - this fails due to issues building this branch which relates to Bugfix/upgrade rxjs and change imports to match docs oni-types#1 i.e. issues importing subject from rxjs
• Add enzyme tests for components
  - [ ] Render lists with react-virtualized Required changes in Feature/ switch explorer rendering to use react-virtualized #2182 - so will hold off adding this till that is merged in
• Make sections collapsible
• Fix filename overflow issue
• Gate sidebar under experimental flag

wil fix #1308

[akinsho]

Potentially need to switch lib to nodegit which explicitly supports electron but has a slightly more involved API as a packaged version of this PR doesn't show the changes (granted that this is due to the package and not an undiagnosed bug).

[bryphe]

Is there a case where we could not the summary, but we could get the branch? If not, it might make sense to simplify and consolidate these try/catches

[akinsho]

That's essentially there for development purposes I'll remove that once the PR is finalised

[bryphe]

What I've implemented here is using simple-git which thankfully is typed and promise compatible. It provides very handy wrappers around git commands and added the number of additions and deletions to the git statusbar. Given that I rushed ahead I'm more than open to feed back re choice of library, changes to the Git service etc.

Excellent. I think simple-git is fine - it seems like a pretty thin wrapper around the CLI git interface.

Performance is important to consider - but it seems like we're just hooking this to the existing FocusGained hook, as well as on buffer saved, which I think this is fine.

I tried this out locally though and didn't see the status indicator - is that related to needing to switch to nodegit?

[akinsho]

@bryphe are the changes not working in development for you or when you run the packaged app? I thought there might be a need to switch the library because it wasnt working once packaged and nodegit advertise their compatibility with electron so I thought electron might have a unique issue with this but having looked at the code base for simple-git its essentially a wrapper around child process git command execution which we do to get the branch so it's likely there's a bug in my implementation somwhere

[akinsho]

@bryphe
I think I've gotten to the bottom of the error here which is that the packaged app has no access to git, I'm not sure how the Child process exec method used for the branch name works but I think that may have to do with using shellEnv which means that it has access to the system git

To get this PR working once packaged I could just replicate a similar method to manually determine the changes on the branch but something that occurred to me here is that using nodegit or simple-git could make for a very straight forward jumping of point for way more git functionality in oni, like merging, cloning, branching etc.

Node git seems to come with git bundled but there's an issue on the repo about current issues installing it which I'm facing on top of which its api is extensively but arguably poorly documented. I'm looking into seeing if the packaged app can be bundled with git and am wondering if you know about this as I can't find any information regarding this? (that way I can stick with simple git and a use a bundled git executable)

EDIT: I'm not entirely certain of my conclusion re. how git operates in electron as when I run require(simple-git) in the console and run git function they work without any issue so I'm not sure what is different about how the code is executed in the console vs. how it executes in Git.ts

[akinsho]

I've settled on creating a diffSummary function manually having had a look at how simple-git does the same. I don't think though that the Git service if based on hand written commands like this will scale well if adding more functionality like this.
An example of which is that by doing it manually if git gets updated if the output of the command I'm using changes the parsing I'm doing could entirely fall apart so its somewhat brittle.

I'm going to log an issue with simple-git, maybe they can provide an explanation or point me in the right direction of whatever the cause maybe and if no joy there I can always check back in with nodegit.

screenshot with local changes enabled (configurable):

EDIT: I'm considering removing the perFile changes as I'm not sure they aren't just confusing and currently I'm not entirely certain they actually work correctly

[akinsho]

@bryphe I've added tests for all the new vcs stuff using jest but the codecov and diff don't seem to reflect this no matter what I add, is this likely due codecov not picking up jest test?

@CrossR think I've addressed the issues in your review except for the icons which I'm going to leave for now if you wanted to have another look over this if you get a chance (side note I put the new sidebar item under an experimental flag till the functionality is fleshed out)

[akinsho]

Actually seems like jest is not collecting coverage at all re. setting up the upload all examples I've seen indicate that this is done with the current settings we are using aka it is set to generate a coverage report and out put it into the the coverage directory not sure if anything else is required remotely

[CrossR]

Changes look good to me, and the bits that weren't working for me before (new files + reload on save) work great now! Very excited for this!

[akinsho]

@CrossR thanks for the review 👍 bringing this in now them 🎉

[bryphe]

Woohoo! Congrats on getting this in @Akin909 - really exciting! Lays the foundation for lots cool possibilities with VCS

Thanks for all your hard work on this @Akin909 , and thanks @CrossR for reviewing it! 💯