Implement Add Repositories UI in the File Explorer Dev Home Page - #3488

ssparach · 2024-07-23T23:03:11Z

Summary of the pull request

This PR removes the temporary UI and implements the design based UI for adding and removing repositories (for source control integration) in the File Explorer Settings page.

References and relevant issues

Detailed description of the pull request / Additional comments

Validation steps performed

Built msix release pckg
Validated UI functionality

PR checklist

Closes #xxx
Tests added/passed
Documentation updated

DefaultRyan · 2024-07-30T20:48:48Z

Please file an issue/bug to track this. We only have the one provider at the moment, so this shouldn't block some testing/selfhosting. But let's not forget to come back to this and fix it.

Adding this as a potential fix. Using an MSBuild task to iterate through all the extensions and populate XAML before it is built. It would be difficult though. Just an idea.

This reverts commit f30055f.

**Summary of the pull request** This PR contains implementation changes for the File Explorer Source Control Integration experimental feature. This feature will allow File Explorer to obtain property information from source control technologies for display (image attached below): **Detailed description of the pull request / Additional comments** This PR contains the following changes: - Reference official Dev Home SDK version (that defines APIs for File Explorer Source Control Integration) - Declare FileExplorerSourceControlIntegration as an experimental feature - The FileExplorerSourceControlIntegration project which creates a COM Server used to communicate information with File Explorer - The FileExplorerGitIntegration project which allows Dev Home to come with an inbox extension that understands git - Basic Unit Tests **Validation steps performed** SDK and MSIX local builds Tested Git Integration File Explorer behavior inside VM Related work items: #48431506

* declare appextension for git, find all source control extensions, UI changes, SDK changes, get extension information to use for mapping * changes to map extension to registered root paths, add validation to git implementation * changes after testing * use serilog in validation code * reorder using * use published SDK version * address PR feedback * address PR feedback * address PR feedback * Minor cleanup of RepositoryTracking.cs --------- Co-authored-by: Ryan Shepherd <ryansh@microsoft.com>

dhoehna · 2024-08-01T20:44:00Z

PropName can be used here instead of re-typing the string.

dhoehna · 2024-08-01T20:50:46Z

I believe the null checks are used because of iterating through the properties and this needs the latest commit to be not null.

I don't like the duplication of the check.

If LatestCommit returns null for a property, would a second call to LatestCommit return a non-null value? If this is the case the call to LatestCommit can be moved outside the foreach loop.

The null check for LatestCommit still needs to be in each case "If not null, add the property"

dhoehna · 2024-08-01T20:52:10Z

Another way to do this is,

Put LatestCommit outside of the for loop.

For every property use result.Add(propName, latestCommit?.[PropertyName] ?? string.empty)

Before returning result remove all entries that contain string.empty.

dhoehna · 2024-08-01T20:54:17Z

Why is this in its own scope?

dhoehna · 2024-08-01T20:57:38Z

I believe this can be simplified to commits.FirstOrDefault()?.Tree?[relativePath] == null

dhoehna · 2024-08-01T20:58:59Z

NIT: commit.Select(x => x.Tree[relativePath]).Where(x +> x is not null)

dhoehna · 2024-08-01T20:59:54Z

NIT: currentCommit.Parents.Count() == 0

dhoehna · 2024-08-01T21:02:33Z

NIT: repo?.Dispose()

dhoehna · 2024-08-01T21:06:50Z

NIT: You can use Microsoft.Windows.System.EnvironmentManager and the EV will be removed when the app is uninstalled.

dhoehna · 2024-08-01T21:11:10Z

Why use PInvoke to get the ILocalRepositoryProvider? I believe DevHome has interfaces to get the providers already.

dhoehna · 2024-08-01T21:17:36Z

old name? Can the name be changed to something more descriptive?

Considering on line 72 the case is ignored.

dhoehna · 2024-08-01T21:20:06Z

I understand. Tracked repositories is a map from a root path to the extension that is tracking the repository. I understand now. Okay.

NIT: change the name to TrackedRepositoriesToExtension, or something similar.

That is why so many other files use pUnk to determing the extension type.

dhoehna · 2024-08-01T21:23:24Z

return value.

dhoehna · 2024-08-01T21:26:34Z

NIT for the file: Make CLSID the second argument. That is what it is. All other methods need either

nothing

RootPath

RootPath and CLSID.

This keeps the arguments in a consistent order.

ssparach · 2024-08-01T21:27:34Z

@dhoehna Just a heads up. This PR contains alot of changes which have already been merged to the feature branch. Since the feature branch was rebased to latest in main, the changes are appearing here. I am currently (locally) fixing the state of the branch. As such, the PR will only contain the UI changes. However, we do plan to create a PR for review to commit to main soon. Thanks!

dhoehna · 2024-08-01T21:27:42Z

you already retrieved TrackedRepositories[rootPath] in TryGetValue. existingExtensionCLSID can be used in this scope.

NVM. This is used to check the existence of the key. In the case. can you use .ContainsKey instead?

dhoehna · 2024-08-01T21:31:39Z

NIT: use UTcNow. An edge case can occur if using .Now. For example, Daylight savings time occurred. Or the user moved between times zones.

dhoehna · 2024-08-01T21:32:13Z

Is this TODO still relevant?

dhoehna · 2024-08-01T21:35:56Z

NIT: repoCollection.Select(x => new RepositoryInformation(x.Key, x.Value))

dhoehna · 2024-08-01T21:37:25Z

NIT: GetLocalProvider(rootFolderPath).GetRepository(rootFolderPath)

dhoehna · 2024-08-01T21:38:41Z

Is this TODO still relevant?

ssparach requested review from DefaultRyan, ethanfangg, jamespik and krschau July 23, 2024 23:13

ssparach marked this pull request as ready for review July 23, 2024 23:16

ssparach removed the request for review from ethanfangg July 23, 2024 23:16

dhoehna reviewed Jul 24, 2024

View reviewed changes

Comment thread tools/Customization/DevHome.FileExplorerSourceControlIntegration/Services/RepositoryTracking.cs Outdated

dhoehna reviewed Jul 24, 2024

View reviewed changes

Comment thread tools/Customization/DevHome.FileExplorerSourceControlIntegration/Services/RepositoryTracking.cs Outdated

dhoehna reviewed Jul 24, 2024

View reviewed changes

Comment thread tools/Customization/DevHome.Customization/ViewModels/FileExplorerViewModel.cs Outdated

krschau reviewed Jul 24, 2024

View reviewed changes

Comment thread tools/Customization/DevHome.Customization/Views/AddRepositoriesView.xaml Outdated

krschau reviewed Jul 26, 2024

View reviewed changes

Comment thread tools/Customization/DevHome.Customization/Views/AddRepositoriesView.xaml Outdated

DefaultRyan reviewed Jul 30, 2024

View reviewed changes

DefaultRyan approved these changes Jul 30, 2024

View reviewed changes

ssparach force-pushed the feature/fileexplorer-sourcecontrol-integration branch 3 times, most recently from 16b9823 to e7bc4e1 Compare July 31, 2024 23:27

ssparach and others added 13 commits August 1, 2024 11:39

reset branch to 548d868

0838dba

Revert "reset branch to 548d868"

0753b47

This reverts commit f30055f.

save state

f731106

save state

bda31f0

save state

e432815

save state

a0bfb4f

save state

170c7cf

implement button actions

c2f5a8d

changes

504df3a

address PR feedback

f2f95de

address PR feedback

41c9650

dhoehna reviewed Aug 1, 2024

View reviewed changes

Comment thread extensions/GitExtension/FileExplorerGitIntegration/GitLocalRepositoryProviderFactory`1.cs Outdated

dhoehna reviewed Aug 1, 2024

View reviewed changes

rebased user branch, made fixes, validated functionality on VM

3ae6b69

ssparach force-pushed the user/ssparach/AddRepositoriesUIForFEVersionControlSettings branch from 2ea72d7 to 3ae6b69 Compare August 1, 2024 22:56

ssparach added 2 commits August 2, 2024 09:53

cleanup UI changes, tested on VM

c340366

fix proj references to match dev home feed and unblock PR pipeline

64b7609

ssparach merged commit 136ea6e into feature/fileexplorer-sourcecontrol-integration Aug 2, 2024

Uh oh!

Conversation

ssparach commented Jul 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the pull request

References and relevant issues

Detailed description of the pull request / Additional comments

Validation steps performed

PR checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhoehna Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhoehna Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssparach commented Aug 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhoehna Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssparach commented Jul 23, 2024 •

edited

Loading

dhoehna Aug 1, 2024 •

edited

Loading

dhoehna Aug 1, 2024 •

edited

Loading

dhoehna Aug 1, 2024 •

edited

Loading