From c5c959cde7cc2ef4cfe0ecf2ea02dc3d59ffda5c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 15 Oct 2019 15:28:43 +0100 Subject: [PATCH 01/18] Remember folder layout preference between edits --- .../Dialog/Clone/RepositoryCloneViewModel.cs | 55 +++++-------------- .../Clone/RepositoryCloneViewModelTests.cs | 10 ++-- 2 files changed, 18 insertions(+), 47 deletions(-) diff --git a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs index 1971914a4f..127f42b445 100644 --- a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs @@ -210,55 +210,26 @@ void UpdatePath(RepositoryModel repository) { if (repository != null) { - var basePath = GetUpdatedBasePath(Path); - previousRepository = repository; - Path = System.IO.Path.Combine(basePath, repository.Owner, repository.Name); - } - } - - string GetUpdatedBasePath(string path) - { - if (string.IsNullOrEmpty(path)) - { - return service.DefaultClonePath; - } - - if (previousRepository == null) - { - return path; - } - - if (FindDirWithout(path, previousRepository?.Owner, 2) is string dirWithoutOwner) - { - return dirWithoutOwner; - } - - if (FindDirWithout(path, previousRepository?.Name, 1) is string dirWithoutRepo) - { - return dirWithoutRepo; - } - - return path; - - string FindDirWithout(string dir, string match, int levels) - { - string dirWithout = null; - for (var i = 0; i < 2; i++) + if (previousRepository != null && !string.IsNullOrEmpty(Path)) { - if (string.IsNullOrEmpty(dir)) + var basePath = System.IO.Path.GetDirectoryName(Path); + var ownerName = System.IO.Path.GetFileName(basePath); + if (previousRepository.Owner == ownerName) { - break; + basePath = System.IO.Path.GetDirectoryName(basePath); + Path = System.IO.Path.Combine(basePath, repository.Owner, repository.Name); } - - var name = System.IO.Path.GetFileName(dir); - dir = System.IO.Path.GetDirectoryName(dir); - if (name == match) + else { - dirWithout = dir; + Path = System.IO.Path.Combine(basePath, repository.Name); } } + else + { + Path = System.IO.Path.Combine(service.DefaultClonePath, repository.Owner, repository.Name); + } - return dirWithout; + previousRepository = repository; } } diff --git a/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs b/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs index 2bf41bc536..4ac59b675e 100644 --- a/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs +++ b/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs @@ -273,21 +273,21 @@ public void Repository_Name_Replaces_Last_Part_Of_Non_Base_Path() Description = "Path unchanged")] [TestCase("c:\\base", "owner1/repo1", "c:\\base\\owner1\\changed", "owner2/repo2", "c:\\base\\owner2\\repo2", Description = "Repo name changed")] - [TestCase("c:\\base", "owner1/repo1", "c:\\base\\owner1", "owner2/repo2", "c:\\base\\owner2\\repo2", + [TestCase("c:\\base", "owner1/repo1", "c:\\base\\owner1", "owner2/repo2", "c:\\base\\repo2", Description = "Repo name deleted")] - [TestCase("c:\\base", "owner1/repo1", "c:\\base", "owner2/repo2", "c:\\base\\owner2\\repo2", + [TestCase("c:\\base", "owner1/repo1", "c:\\base", "owner2/repo2", "c:\\repo2", Description = "Base path reverted")] [TestCase("c:\\base", "owner1/repo1", "c:\\new\\base\\owner1\\changed", "owner2/repo2", "c:\\new\\base\\owner2\\repo2", Description = "Base path and repo name changed")] - [TestCase("c:\\base", "owner1/repo1", "c:\\new\\base\\owner1", "owner2/repo2", "c:\\new\\base\\owner2\\repo2", + [TestCase("c:\\base", "owner1/repo1", "c:\\new\\base\\owner1", "owner2/repo2", "c:\\new\\base\\repo2", Description = "Base path changed and repo name deleted")] - [TestCase("c:\\base", "owner1/repo1", "c:\\new\\base", "owner2/repo2", "c:\\new\\base\\owner2\\repo2", + [TestCase("c:\\base", "owner1/repo1", "c:\\new\\base", "owner2/repo2", "c:\\new\\repo2", Description = "Base path changed and repo owner/name deleted")] [TestCase("c:\\base", "owner1/repo1", "", "owner2/repo2", "c:\\base\\owner2\\repo2", Description = "Base path cleared")] - [TestCase("c:\\base", "owner1/repo1", "c:\\base\\repo1", "owner2/repo2", "c:\\base\\owner2\\repo2", + [TestCase("c:\\base", "owner1/repo1", "c:\\base\\repo1", "owner2/repo2", "c:\\base\\repo2", Description = "Owner deleted")] [TestCase("c:\\base", "same/same", "c:\\base\\same\\same", "owner2/repo2", "c:\\base\\owner2\\repo2", Description = "Owner and repo have same name")] From 58fb3155291a7c245042d6ee7f64a3c178fa120f Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 15 Oct 2019 15:46:46 +0100 Subject: [PATCH 02/18] Reset bad paths --- .../Dialog/Clone/RepositoryCloneViewModel.cs | 24 ++++++++++++------- .../Clone/RepositoryCloneViewModelTests.cs | 3 +++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs index 127f42b445..f5e9b2e2b1 100644 --- a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs @@ -210,22 +210,30 @@ void UpdatePath(RepositoryModel repository) { if (repository != null) { - if (previousRepository != null && !string.IsNullOrEmpty(Path)) + try { - var basePath = System.IO.Path.GetDirectoryName(Path); - var ownerName = System.IO.Path.GetFileName(basePath); - if (previousRepository.Owner == ownerName) + if (previousRepository != null && !string.IsNullOrEmpty(Path)) { - basePath = System.IO.Path.GetDirectoryName(basePath); - Path = System.IO.Path.Combine(basePath, repository.Owner, repository.Name); + var basePath = System.IO.Path.GetDirectoryName(Path); + var ownerName = System.IO.Path.GetFileName(basePath); + if (previousRepository.Owner == ownerName) + { + basePath = System.IO.Path.GetDirectoryName(basePath); + Path = System.IO.Path.Combine(basePath, repository.Owner, repository.Name); + } + else + { + Path = System.IO.Path.Combine(basePath, repository.Name); + } } else { - Path = System.IO.Path.Combine(basePath, repository.Name); + Path = System.IO.Path.Combine(service.DefaultClonePath, repository.Owner, repository.Name); } } - else + catch (Exception) { + // Reset illegal paths Path = System.IO.Path.Combine(service.DefaultClonePath, repository.Owner, repository.Name); } diff --git a/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs b/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs index 4ac59b675e..a81f7380bf 100644 --- a/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs +++ b/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs @@ -291,6 +291,9 @@ public void Repository_Name_Replaces_Last_Part_Of_Non_Base_Path() Description = "Owner deleted")] [TestCase("c:\\base", "same/same", "c:\\base\\same\\same", "owner2/repo2", "c:\\base\\owner2\\repo2", Description = "Owner and repo have same name")] + + [TestCase("c:\\base", "owner1/repo1", ":", "owner2/repo2", "c:\\base\\owner2\\repo2", + Description = "The path is not of a legal form")] public void User_Edits_Path(string defaultClonePath, string repo1, string userPath, string repo2, string expectPath) { var target = CreateTarget(defaultClonePath: defaultClonePath); From e07a025ff22690cf5c58af1a8187c1a54552653d Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 15 Oct 2019 16:15:54 +0100 Subject: [PATCH 03/18] Fix package reference in PackageSettingsGen.tt --- .../Settings/generated/PackageSettingsGen.tt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GitHub.VisualStudio/Settings/generated/PackageSettingsGen.tt b/src/GitHub.VisualStudio/Settings/generated/PackageSettingsGen.tt index a575552ca8..7e7f5b8cb0 100644 --- a/src/GitHub.VisualStudio/Settings/generated/PackageSettingsGen.tt +++ b/src/GitHub.VisualStudio/Settings/generated/PackageSettingsGen.tt @@ -3,7 +3,7 @@ <#@ import namespace="System.Linq" #> <#@ import namespace="System.Collections.Generic" #> <#@ import namespace="System.IO" #> -<#@ assembly name="$(PackageDir)\Newtonsoft.Json.6.0.8\lib\net45\Newtonsoft.Json.dll" #> +<#@ assembly name="$(USERPROFILE)\.nuget\packages\Newtonsoft.Json\6.0.8\lib\net45\Newtonsoft.Json.dll" #> <#@ import namespace="Newtonsoft.Json.Linq" #> <#@ output extension=".cs" #> <# From b1d89e19eba0673fd2459b8d888ea593aaaadf84 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 16 Oct 2019 09:47:57 +0100 Subject: [PATCH 04/18] Add default repository location/layout settings Adds DefaultRepositoryLocation and DefaultRepositoryLayout --- .../Settings/generated/IPackageSettings.cs | 2 + .../Settings/generated/PackageSettingsGen.cs | 37 +++++++++++++++---- src/common/settings.json | 12 +++++- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/GitHub.Exports/Settings/generated/IPackageSettings.cs b/src/GitHub.Exports/Settings/generated/IPackageSettings.cs index 258896c409..e3d8811c68 100644 --- a/src/GitHub.Exports/Settings/generated/IPackageSettings.cs +++ b/src/GitHub.Exports/Settings/generated/IPackageSettings.cs @@ -44,5 +44,7 @@ public interface IPackageSettings : INotifyPropertyChanged UIState UIState { get; set; } bool HideTeamExplorerWelcomeMessage { get; set; } bool EnableTraceLogging { get; set; } + string DefaultRepositoryLocation { get; set; } + string DefaultRepositoryLayout { get; set; } } } \ No newline at end of file diff --git a/src/GitHub.VisualStudio/Settings/generated/PackageSettingsGen.cs b/src/GitHub.VisualStudio/Settings/generated/PackageSettingsGen.cs index 6969198bb2..7c57cc7e2c 100644 --- a/src/GitHub.VisualStudio/Settings/generated/PackageSettingsGen.cs +++ b/src/GitHub.VisualStudio/Settings/generated/PackageSettingsGen.cs @@ -5,7 +5,7 @@ { "name": "CollectMetrics", "type": "bool", - "default": 'true' + "default": "true" }, { "name": "EditorComments", @@ -27,6 +27,16 @@ "name": "EnableTraceLogging", "type": "bool", "default": "false" + }, + { + "name": "DefaultRepositoryLocation", + "type": "string", + "default": "null" + }, + { + "name": "DefaultRepositoryLayout", + "type": "string", + "default": "null" } ] } @@ -55,13 +65,6 @@ public bool EditorComments set { editorComments = value; this.RaisePropertyChange(); } } - bool forkButton; - public bool ForkButton - { - get { return forkButton; } - set { forkButton = value; this.RaisePropertyChange(); } - } - UIState uIState; public UIState UIState { @@ -83,6 +86,20 @@ public bool EnableTraceLogging set { enableTraceLogging = value; this.RaisePropertyChange(); } } + string defaultRepositoryLocation; + public string DefaultRepositoryLocation + { + get { return defaultRepositoryLocation; } + set { defaultRepositoryLocation = value; this.RaisePropertyChange(); } + } + + string defaultRepositoryLayout; + public string DefaultRepositoryLayout + { + get { return defaultRepositoryLayout; } + set { defaultRepositoryLayout = value; this.RaisePropertyChange(); } + } + void LoadSettings() { @@ -91,6 +108,8 @@ void LoadSettings() UIState = SimpleJson.DeserializeObject((string)settingsStore.Read("UIState", "{}")); HideTeamExplorerWelcomeMessage = (bool)settingsStore.Read("HideTeamExplorerWelcomeMessage", false); EnableTraceLogging = (bool)settingsStore.Read("EnableTraceLogging", false); + DefaultRepositoryLocation = (string)settingsStore.Read("DefaultRepositoryLocation", null); + DefaultRepositoryLayout = (string)settingsStore.Read("DefaultRepositoryLayout", null); } void SaveSettings() @@ -100,6 +119,8 @@ void SaveSettings() settingsStore.Write("UIState", SimpleJson.SerializeObject(UIState)); settingsStore.Write("HideTeamExplorerWelcomeMessage", HideTeamExplorerWelcomeMessage); settingsStore.Write("EnableTraceLogging", EnableTraceLogging); + settingsStore.Write("DefaultRepositoryLocation", DefaultRepositoryLocation); + settingsStore.Write("DefaultRepositoryLayout", DefaultRepositoryLayout); } } diff --git a/src/common/settings.json b/src/common/settings.json index d0ca9a282a..39b06348dd 100644 --- a/src/common/settings.json +++ b/src/common/settings.json @@ -3,7 +3,7 @@ { "name": "CollectMetrics", "type": "bool", - "default": 'true' + "default": "true" }, { "name": "EditorComments", @@ -25,6 +25,16 @@ "name": "EnableTraceLogging", "type": "bool", "default": "false" + }, + { + "name": "DefaultRepositoryLocation", + "type": "string", + "default": "null" + }, + { + "name": "DefaultRepositoryLayout", + "type": "string", + "default": "null" } ] } \ No newline at end of file From 7c576375183f77012869c9292d16d1c4f3af81a2 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 16 Oct 2019 12:20:13 +0100 Subject: [PATCH 05/18] Consolidate default clone path responsibilities Make RepositoryCloneService responsible for storing the default clone path and deciding on the default clone path for a given repository URL. --- .../Services/RepositoryCloneService.cs | 27 ++++++++++++++++--- .../Services/RepositoryCreationService.cs | 2 +- .../Dialog/Clone/RepositoryCloneViewModel.cs | 6 ++--- .../Services/IRepositoryCloneService.cs | 10 ++++--- .../Services/RepositoryCloneServiceTests.cs | 7 +++-- test/GitHub.App.UnitTests/Substitutes.cs | 4 ++- .../Clone/RepositoryCloneViewModelTests.cs | 7 ++++- .../Substitutes.cs | 4 ++- 8 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs index 86177cda7d..ce130d9cad 100644 --- a/src/GitHub.App/Services/RepositoryCloneService.cs +++ b/src/GitHub.App/Services/RepositoryCloneService.cs @@ -11,6 +11,7 @@ using GitHub.Logging; using GitHub.Models; using GitHub.Primitives; +using GitHub.Settings; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Threading; using Octokit.GraphQL; @@ -39,6 +40,7 @@ public class RepositoryCloneService : IRepositoryCloneService readonly IGraphQLClientFactory graphqlFactory; readonly IGitHubContextService gitHubContextService; readonly IUsageTracker usageTracker; + readonly Lazy packageSettings; readonly Lazy dte; ICompiledQuery readViewerRepositories; @@ -51,6 +53,7 @@ public RepositoryCloneService( IGitHubContextService gitHubContextService, IUsageTracker usageTracker, IGitHubServiceProvider sp, + Lazy packageSettings, [Import(AllowDefault = true)] JoinableTaskContext joinableTaskContext) { this.operatingSystem = operatingSystem; @@ -59,10 +62,9 @@ public RepositoryCloneService( this.graphqlFactory = graphqlFactory; this.gitHubContextService = gitHubContextService; this.usageTracker = usageTracker; + this.packageSettings = packageSettings; dte = new Lazy(() => sp.GetService()); JoinableTaskContext = joinableTaskContext ?? ThreadHelper.JoinableTaskContext; - - defaultClonePath = GetLocalClonePathFromGitProvider(operatingSystem.Environment.GetUserRepositoriesPath()); } /// @@ -220,7 +222,8 @@ public async Task CloneRepository( await vsGitServices.Clone(cloneUrl, repositoryPath, true, progress, cancellationToken); await usageTracker.IncrementCounter(x => x.NumberOfClones); - if (repositoryPath.StartsWith(DefaultClonePath, StringComparison.OrdinalIgnoreCase)) + var defaultClonePath = GetDefaultClonePath(); + if (repositoryPath.StartsWith(defaultClonePath, StringComparison.OrdinalIgnoreCase)) { // Count the number of times users clone into the Default Repository Location await usageTracker.IncrementCounter(x => x.NumberOfClonesToDefaultClonePath); @@ -251,7 +254,23 @@ string GetLocalClonePathFromGitProvider(string fallbackPath) : fallbackPath; } - public string DefaultClonePath { get { return defaultClonePath; } } + public void SetDefaultClonePath(string repositoryPath, UriString cloneUrl) => throw new NotImplementedException(); + + public string GetDefaultClonePath(UriString cloneUrl = null) + { + var defaultPath = packageSettings.Value.DefaultRepositoryLocation; + if (string.IsNullOrEmpty(defaultPath)) + { + defaultPath = GetLocalClonePathFromGitProvider(operatingSystem.Environment.GetUserRepositoriesPath()); + } + + if (cloneUrl == null) + { + return defaultPath; + } + + return Path.Combine(defaultPath, cloneUrl.Host, cloneUrl.RepositoryName); + } JoinableTaskContext JoinableTaskContext { get; } diff --git a/src/GitHub.App/Services/RepositoryCreationService.cs b/src/GitHub.App/Services/RepositoryCreationService.cs index 7c41ba4d81..baef408f5f 100644 --- a/src/GitHub.App/Services/RepositoryCreationService.cs +++ b/src/GitHub.App/Services/RepositoryCreationService.cs @@ -25,7 +25,7 @@ public RepositoryCreationService(IRepositoryCloneService cloneService) public string DefaultClonePath { - get { return cloneService.DefaultClonePath; } + get { return cloneService.GetDefaultClonePath(); } } public IObservable CreateRepository( diff --git a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs index f5e9b2e2b1..e3f3dd0354 100644 --- a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs @@ -59,7 +59,7 @@ public RepositoryCloneViewModel( var repository = this.WhenAnyValue(x => x.SelectedTabIndex) .SelectMany(x => tabs[x].WhenAnyValue(tab => tab.Repository)); - Path = service.DefaultClonePath; + Path = service.GetDefaultClonePath(); repository.Subscribe(x => UpdatePath(x)); pathWarning = Observable.CombineLatest( @@ -228,13 +228,13 @@ void UpdatePath(RepositoryModel repository) } else { - Path = System.IO.Path.Combine(service.DefaultClonePath, repository.Owner, repository.Name); + Path = service.GetDefaultClonePath(repository.CloneUrl); } } catch (Exception) { // Reset illegal paths - Path = System.IO.Path.Combine(service.DefaultClonePath, repository.Owner, repository.Name); + Path = service.GetDefaultClonePath(repository.CloneUrl); } previousRepository = repository; diff --git a/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs b/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs index 791aa25734..c463e2b1e1 100644 --- a/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs +++ b/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs @@ -11,10 +11,14 @@ namespace GitHub.Services public interface IRepositoryCloneService { /// - /// Default path to clone things to, used as fallback if we can't find the correct path - /// from VS. + /// Get the default clone path for a given clone URL. /// - string DefaultClonePath { get; } + string GetDefaultClonePath(UriString cloneUrl = null); + + /// + /// Infer the default clone path and layout from an example repository path and clone URL. + /// + void SetDefaultClonePath(string repositoryPath, UriString cloneUrl); /// /// Clones the specificed repository into the specified directory. diff --git a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs index 02e1898222..4c2f274e60 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs @@ -5,6 +5,7 @@ using GitHub.Api; using GitHub.Models; using GitHub.Services; +using GitHub.Settings; using Microsoft.VisualStudio.Threading; using NSubstitute; using NUnit.Framework; @@ -157,19 +158,21 @@ public async Task CloneIntoEmptyDirectory() static RepositoryCloneService CreateRepositoryCloneService(IOperatingSystem operatingSystem = null, IVSGitServices vsGitServices = null, IUsageTracker usageTracker = null, - ITeamExplorerServices teamExplorerServices = null, IGitHubServiceProvider serviceProvider = null) + ITeamExplorerServices teamExplorerServices = null, IGitHubServiceProvider serviceProvider = null, + IPackageSettings packageSettings = null) { operatingSystem = operatingSystem ?? Substitute.For(); vsGitServices = vsGitServices ?? Substitute.For(); usageTracker = usageTracker ?? Substitute.For(); teamExplorerServices = teamExplorerServices ?? Substitute.For(); serviceProvider = serviceProvider ?? Substitute.For(); + packageSettings = packageSettings ?? Substitute.For(); operatingSystem.Environment.ExpandEnvironmentVariables(Args.String).Returns(x => x[0]); return new RepositoryCloneService(operatingSystem, vsGitServices, teamExplorerServices, Substitute.For(), Substitute.For(), - usageTracker, serviceProvider, new JoinableTaskContext()); + usageTracker, serviceProvider, new Lazy(() => packageSettings), new JoinableTaskContext()); } } } diff --git a/test/GitHub.App.UnitTests/Substitutes.cs b/test/GitHub.App.UnitTests/Substitutes.cs index c47e998657..74dd90d537 100644 --- a/test/GitHub.App.UnitTests/Substitutes.cs +++ b/test/GitHub.App.UnitTests/Substitutes.cs @@ -11,6 +11,7 @@ using GitHub.Factories; using GitHub.Api; using Microsoft.VisualStudio.Threading; +using GitHub.Settings; namespace UnitTests { @@ -89,7 +90,8 @@ public static IGitHubServiceProvider GetServiceProvider( var clone = cloneService ?? new RepositoryCloneService(Substitute.For(), Substitute.For(), Substitute.For(), Substitute.For(), Substitute.For(), - Substitute.For(), ret, new JoinableTaskContext()); + Substitute.For(), ret, new Lazy(() => Substitute.For()), + new JoinableTaskContext()); var create = creationService ?? new RepositoryCreationService(clone); avatarProvider = avatarProvider ?? Substitute.For(); ret.GetService(typeof(IGitService)).Returns(gitservice); diff --git a/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs b/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs index a81f7380bf..5c3343113f 100644 --- a/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs +++ b/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs @@ -412,7 +412,12 @@ static IRepositorySelectViewModel CreateSelectViewModel() static IRepositoryCloneService CreateRepositoryCloneService(string defaultClonePath) { var result = Substitute.For(); - result.DefaultClonePath.Returns(defaultClonePath); + result.GetDefaultClonePath(null).ReturnsForAnyArgs(x => + { + var cloneUrl = x.Arg(); + if (cloneUrl == null) return defaultClonePath; + return Path.Combine(defaultClonePath, cloneUrl.Owner, cloneUrl.RepositoryName); + }); result.DestinationDirectoryExists(directoryExists).Returns(true); result.DestinationDirectoryExists(directoryEmpty).Returns(true); result.DestinationDirectoryEmpty(directoryEmpty).Returns(true); diff --git a/test/GitHub.VisualStudio.UnitTests/Substitutes.cs b/test/GitHub.VisualStudio.UnitTests/Substitutes.cs index fe983fde88..b0aa111869 100644 --- a/test/GitHub.VisualStudio.UnitTests/Substitutes.cs +++ b/test/GitHub.VisualStudio.UnitTests/Substitutes.cs @@ -11,6 +11,7 @@ using GitHub.Factories; using GitHub.Api; using Microsoft.VisualStudio.Threading; +using GitHub.Settings; namespace UnitTests { @@ -114,7 +115,8 @@ public static IGitHubServiceProvider GetServiceProvider( var vsgit = IVSGitServices; var clone = cloneService ?? new RepositoryCloneService(os, vsgit, Substitute.For(), Substitute.For(), Substitute.For(), - Substitute.For(), ret, new JoinableTaskContext()); + Substitute.For(), ret, new Lazy(() => Substitute.For()), + new JoinableTaskContext()); var create = creationService ?? new RepositoryCreationService(clone); avatarProvider = avatarProvider ?? Substitute.For(); ret.GetService(typeof(IGitService)).Returns(gitservice); From f8f03d1e9dae322c93d050a27d2b2d791f205bba Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 16 Oct 2019 12:48:56 +0100 Subject: [PATCH 06/18] Set default clone path after cloning --- .../Services/RepositoryCloneService.cs | 16 +++++- .../Services/RepositoryCloneServiceTests.cs | 52 +++++++++++++------ 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs index ce130d9cad..e3ed2543ba 100644 --- a/src/GitHub.App/Services/RepositoryCloneService.cs +++ b/src/GitHub.App/Services/RepositoryCloneService.cs @@ -228,6 +228,8 @@ public async Task CloneRepository( // Count the number of times users clone into the Default Repository Location await usageTracker.IncrementCounter(x => x.NumberOfClonesToDefaultClonePath); } + + SetDefaultClonePath(repositoryPath, cloneUrl); } catch (Exception ex) { @@ -254,7 +256,17 @@ string GetLocalClonePathFromGitProvider(string fallbackPath) : fallbackPath; } - public void SetDefaultClonePath(string repositoryPath, UriString cloneUrl) => throw new NotImplementedException(); + public void SetDefaultClonePath(string repositoryPath, UriString cloneUrl) + { + var possibleOwnerPath = Path.GetDirectoryName(repositoryPath); + var possibleOwner = Path.GetFileName(possibleOwnerPath); + var defaultPath = string.Equals(possibleOwner, cloneUrl.Owner, StringComparison.OrdinalIgnoreCase) ? + Path.GetDirectoryName(possibleOwnerPath) : possibleOwnerPath; + + log.Information("Setting DefaultRepositoryLocation to {Location}", defaultPath); + packageSettings.Value.DefaultRepositoryLocation = defaultPath; + packageSettings.Value.Save(); + } public string GetDefaultClonePath(UriString cloneUrl = null) { @@ -269,7 +281,7 @@ public string GetDefaultClonePath(UriString cloneUrl = null) return defaultPath; } - return Path.Combine(defaultPath, cloneUrl.Host, cloneUrl.RepositoryName); + return Path.Combine(defaultPath, cloneUrl.Owner, cloneUrl.RepositoryName); } JoinableTaskContext JoinableTaskContext { get; } diff --git a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs index 4c2f274e60..af480ea021 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs @@ -155,24 +155,44 @@ public async Task CloneIntoEmptyDirectory() operatingSystem.Directory.DidNotReceive().CreateDirectory(clonePath); await vsGitServices.Received().Clone(cloneUrl, clonePath, true); } + } - static RepositoryCloneService CreateRepositoryCloneService(IOperatingSystem operatingSystem = null, - IVSGitServices vsGitServices = null, IUsageTracker usageTracker = null, - ITeamExplorerServices teamExplorerServices = null, IGitHubServiceProvider serviceProvider = null, - IPackageSettings packageSettings = null) + public class TheSetDefaultClonePathMethod + { + [TestCase(@"c:\source\owner\repositoryName", "https://github.com/owner/repositoryName", @"c:\source")] + [TestCase(@"c:\source\owner\newRepositoryName", "https://github.com/owner/repositoryName", @"c:\source")] + [TestCase(@"c:\source\owner\repositoryName", "https://github.com/OWNER/repositoryName", @"c:\source")] + [TestCase(@"c:\owner\repositoryName", "https://github.com/owner/repositoryName", @"c:\")] + + [TestCase(@"c:\source\repositoryName", "https://github.com/owner/repositoryName", @"c:\source")] + [TestCase(@"c:\repositoryName", "https://github.com/owner/repositoryName", @"c:\")] + public void SetDefaultClonePath(string targetPath, string cloneUrl, string expectedDefaultPath) { - operatingSystem = operatingSystem ?? Substitute.For(); - vsGitServices = vsGitServices ?? Substitute.For(); - usageTracker = usageTracker ?? Substitute.For(); - teamExplorerServices = teamExplorerServices ?? Substitute.For(); - serviceProvider = serviceProvider ?? Substitute.For(); - packageSettings = packageSettings ?? Substitute.For(); - - operatingSystem.Environment.ExpandEnvironmentVariables(Args.String).Returns(x => x[0]); - - return new RepositoryCloneService(operatingSystem, vsGitServices, teamExplorerServices, - Substitute.For(), Substitute.For(), - usageTracker, serviceProvider, new Lazy(() => packageSettings), new JoinableTaskContext()); + var cloneService = CreateRepositoryCloneService(); + + cloneService.SetDefaultClonePath(targetPath, cloneUrl); + var defaultPath = cloneService.GetDefaultClonePath(); + + Assert.That(defaultPath, Is.EqualTo(expectedDefaultPath)); } } + + static RepositoryCloneService CreateRepositoryCloneService(IOperatingSystem operatingSystem = null, + IVSGitServices vsGitServices = null, IUsageTracker usageTracker = null, + ITeamExplorerServices teamExplorerServices = null, IGitHubServiceProvider serviceProvider = null, + IPackageSettings packageSettings = null) + { + operatingSystem = operatingSystem ?? Substitute.For(); + vsGitServices = vsGitServices ?? Substitute.For(); + usageTracker = usageTracker ?? Substitute.For(); + teamExplorerServices = teamExplorerServices ?? Substitute.For(); + serviceProvider = serviceProvider ?? Substitute.For(); + packageSettings = packageSettings ?? Substitute.For(); + + operatingSystem.Environment.ExpandEnvironmentVariables(Args.String).Returns(x => x[0]); + + return new RepositoryCloneService(operatingSystem, vsGitServices, teamExplorerServices, + Substitute.For(), Substitute.For(), + usageTracker, serviceProvider, new Lazy(() => packageSettings), new JoinableTaskContext()); + } } From 48491d07e4adccd7c47b2c6b846d5bfbd6604827 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 16 Oct 2019 15:33:07 +0100 Subject: [PATCH 07/18] Remember user's preferred repository layout --- .../Services/RepositoryCloneService.cs | 54 +++++++++++++++++-- .../Services/RepositoryCloneServiceTests.cs | 27 ++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs index e3ed2543ba..e5a27d8187 100644 --- a/src/GitHub.App/Services/RepositoryCloneService.cs +++ b/src/GitHub.App/Services/RepositoryCloneService.cs @@ -260,11 +260,26 @@ public void SetDefaultClonePath(string repositoryPath, UriString cloneUrl) { var possibleOwnerPath = Path.GetDirectoryName(repositoryPath); var possibleOwner = Path.GetFileName(possibleOwnerPath); - var defaultPath = string.Equals(possibleOwner, cloneUrl.Owner, StringComparison.OrdinalIgnoreCase) ? - Path.GetDirectoryName(possibleOwnerPath) : possibleOwnerPath; + + string defaultPath; + RepositoryLayout repositoryLayout; + if (string.Equals(possibleOwner, cloneUrl.Owner, StringComparison.OrdinalIgnoreCase)) + { + repositoryLayout = RepositoryLayout.OwnerName; + defaultPath = Path.GetDirectoryName(possibleOwnerPath); + } + else + { + repositoryLayout = RepositoryLayout.Name; + defaultPath = possibleOwnerPath; + } log.Information("Setting DefaultRepositoryLocation to {Location}", defaultPath); packageSettings.Value.DefaultRepositoryLocation = defaultPath; + + log.Information("Setting DefaultRepositoryLayout to {Layout}", repositoryLayout); + SetDefaultRepositoryLayout(repositoryLayout); + packageSettings.Value.Save(); } @@ -281,11 +296,44 @@ public string GetDefaultClonePath(UriString cloneUrl = null) return defaultPath; } - return Path.Combine(defaultPath, cloneUrl.Owner, cloneUrl.RepositoryName); + var repositoryLayout = GetDefaultRepositoryLayout(); + switch (repositoryLayout) + { + case RepositoryLayout.Name: + return Path.Combine(defaultPath, cloneUrl.RepositoryName); + case RepositoryLayout.OwnerName: + return Path.Combine(defaultPath, cloneUrl.Owner, cloneUrl.RepositoryName); + default: + throw new ArgumentException($"Unknown repository layout: {repositoryLayout}"); + + } + } + + RepositoryLayout GetDefaultRepositoryLayout() + { + RepositoryLayout repositoryLayout; + if (!Enum.TryParse(packageSettings.Value.DefaultRepositoryLayout, out repositoryLayout)) + { + repositoryLayout = RepositoryLayout.OwnerName; + } + + return repositoryLayout; + } + + void SetDefaultRepositoryLayout(RepositoryLayout repositoryLayout) + { + packageSettings.Value.DefaultRepositoryLayout = repositoryLayout.ToString(); } JoinableTaskContext JoinableTaskContext { get; } + enum RepositoryLayout + { + + Name, // Layout repositories by name + OwnerName // Layout repositories by owner and name + } + class OrganizationAdapter { public IReadOnlyList Repositories { get; set; } diff --git a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs index af480ea021..169778ff43 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs @@ -177,6 +177,33 @@ public void SetDefaultClonePath(string targetPath, string cloneUrl, string expec } } + public class TheGetDefaultClonePathMethod + { + [TestCase(@"c:\source\owner\name", "https://github.com/owner/name", "https://github.com/newOwner/newName", @"c:\source\newOwner\newName")] + [TestCase(@"c:\source\name", "https://github.com/owner/name", "https://github.com/newOwner/newName", @"c:\source\newName")] + public void GetDefaultClonePath(string setTargetPath, string setCloneUrl, string getCloneUrl, string expectedDefaultPath) + { + var cloneService = CreateRepositoryCloneService(); + + cloneService.SetDefaultClonePath(setTargetPath, setCloneUrl); + var defaultPath = cloneService.GetDefaultClonePath(getCloneUrl); + + Assert.That(defaultPath, Is.EqualTo(expectedDefaultPath)); + } + + [TestCase(@"c:\default", "https://github.com/owner/repositoryName", @"c:\default\owner\repositoryName")] + public void Defaults_To_Owner_Name_Layout(string localClonePath, string getCloneUrl, string expectedDefaultPath) + { + var vsGitServices = Substitute.For(); + vsGitServices.GetLocalClonePathFromGitProvider().Returns(localClonePath); + var cloneService = CreateRepositoryCloneService(vsGitServices: vsGitServices); + + var defaultPath = cloneService.GetDefaultClonePath(getCloneUrl); + + Assert.That(defaultPath, Is.EqualTo(expectedDefaultPath)); + } + } + static RepositoryCloneService CreateRepositoryCloneService(IOperatingSystem operatingSystem = null, IVSGitServices vsGitServices = null, IUsageTracker usageTracker = null, ITeamExplorerServices teamExplorerServices = null, IGitHubServiceProvider serviceProvider = null, From 72f631d001c40e89b603f14639651679221d3ed9 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 16 Oct 2019 15:33:25 +0100 Subject: [PATCH 08/18] Remove dead code --- src/GitHub.App/Services/RepositoryCloneService.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs index e5a27d8187..235a91bd7b 100644 --- a/src/GitHub.App/Services/RepositoryCloneService.cs +++ b/src/GitHub.App/Services/RepositoryCloneService.cs @@ -333,10 +333,5 @@ enum RepositoryLayout Name, // Layout repositories by name OwnerName // Layout repositories by owner and name } - - class OrganizationAdapter - { - public IReadOnlyList Repositories { get; set; } - } } } From 62207e8e0708cf9fc272dc69491dd6890f2cccc4 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 18 Oct 2019 11:27:23 +0100 Subject: [PATCH 09/18] Factor out repository layout logic Factor out RepositoryLayout related logic into RepositoryLayoutUtilities. --- .../Services/RepositoryCloneService.cs | 55 ++----------------- src/GitHub.App/Services/RepositoryLayout.cs | 9 +++ .../Services/RepositoryLayoutUtilities.cs | 48 ++++++++++++++++ .../Dialog/Clone/RepositoryCloneViewModel.cs | 13 +---- 4 files changed, 64 insertions(+), 61 deletions(-) create mode 100644 src/GitHub.App/Services/RepositoryLayout.cs create mode 100644 src/GitHub.App/Services/RepositoryLayoutUtilities.cs diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs index 235a91bd7b..d322cf2cf9 100644 --- a/src/GitHub.App/Services/RepositoryCloneService.cs +++ b/src/GitHub.App/Services/RepositoryCloneService.cs @@ -258,27 +258,13 @@ string GetLocalClonePathFromGitProvider(string fallbackPath) public void SetDefaultClonePath(string repositoryPath, UriString cloneUrl) { - var possibleOwnerPath = Path.GetDirectoryName(repositoryPath); - var possibleOwner = Path.GetFileName(possibleOwnerPath); - - string defaultPath; - RepositoryLayout repositoryLayout; - if (string.Equals(possibleOwner, cloneUrl.Owner, StringComparison.OrdinalIgnoreCase)) - { - repositoryLayout = RepositoryLayout.OwnerName; - defaultPath = Path.GetDirectoryName(possibleOwnerPath); - } - else - { - repositoryLayout = RepositoryLayout.Name; - defaultPath = possibleOwnerPath; - } + var (defaultPath, repositoryLayout) = RepositoryLayoutUtilities.GetDefaultPathAndLayout(repositoryPath, cloneUrl); log.Information("Setting DefaultRepositoryLocation to {Location}", defaultPath); packageSettings.Value.DefaultRepositoryLocation = defaultPath; log.Information("Setting DefaultRepositoryLayout to {Layout}", repositoryLayout); - SetDefaultRepositoryLayout(repositoryLayout); + packageSettings.Value.DefaultRepositoryLayout = repositoryLayout.ToString(); packageSettings.Value.Save(); } @@ -296,42 +282,11 @@ public string GetDefaultClonePath(UriString cloneUrl = null) return defaultPath; } - var repositoryLayout = GetDefaultRepositoryLayout(); - switch (repositoryLayout) - { - case RepositoryLayout.Name: - return Path.Combine(defaultPath, cloneUrl.RepositoryName); - case RepositoryLayout.OwnerName: - return Path.Combine(defaultPath, cloneUrl.Owner, cloneUrl.RepositoryName); - default: - throw new ArgumentException($"Unknown repository layout: {repositoryLayout}"); - - } - } - - RepositoryLayout GetDefaultRepositoryLayout() - { - RepositoryLayout repositoryLayout; - if (!Enum.TryParse(packageSettings.Value.DefaultRepositoryLayout, out repositoryLayout)) - { - repositoryLayout = RepositoryLayout.OwnerName; - } - - return repositoryLayout; - } - - void SetDefaultRepositoryLayout(RepositoryLayout repositoryLayout) - { - packageSettings.Value.DefaultRepositoryLayout = repositoryLayout.ToString(); + var repositoryLayoutSetting = packageSettings.Value.DefaultRepositoryLayout; + var repositoryLayout = RepositoryLayoutUtilities.GetRepositoryLayout(repositoryLayoutSetting); + return RepositoryLayoutUtilities.GetDefaultRepositoryPath(cloneUrl, defaultPath, repositoryLayout); } JoinableTaskContext JoinableTaskContext { get; } - - enum RepositoryLayout - { - - Name, // Layout repositories by name - OwnerName // Layout repositories by owner and name - } } } diff --git a/src/GitHub.App/Services/RepositoryLayout.cs b/src/GitHub.App/Services/RepositoryLayout.cs new file mode 100644 index 0000000000..12ec162677 --- /dev/null +++ b/src/GitHub.App/Services/RepositoryLayout.cs @@ -0,0 +1,9 @@ +namespace GitHub.Services +{ + public enum RepositoryLayout + { + + Name, // Layout repositories by name + OwnerName // Layout repositories by owner and name + } +} diff --git a/src/GitHub.App/Services/RepositoryLayoutUtilities.cs b/src/GitHub.App/Services/RepositoryLayoutUtilities.cs new file mode 100644 index 0000000000..aa4a5e4ea1 --- /dev/null +++ b/src/GitHub.App/Services/RepositoryLayoutUtilities.cs @@ -0,0 +1,48 @@ +using System; +using System.IO; +using GitHub.Primitives; + +namespace GitHub.Services +{ + public static class RepositoryLayoutUtilities + { + public static string GetDefaultRepositoryPath(UriString cloneUrl, string defaultPath, RepositoryLayout repositoryLayout) + { + switch (repositoryLayout) + { + case RepositoryLayout.Name: + return Path.Combine(defaultPath, cloneUrl.RepositoryName); + case RepositoryLayout.OwnerName: + return Path.Combine(defaultPath, cloneUrl.Owner, cloneUrl.RepositoryName); + default: + throw new ArgumentException($"Unknown repository layout: {repositoryLayout}"); + + } + } + + public static RepositoryLayout GetRepositoryLayout(string repositoryLayoutSetting) + { + RepositoryLayout repositoryLayout; + if (!Enum.TryParse(repositoryLayoutSetting, out repositoryLayout)) + { + repositoryLayout = RepositoryLayout.OwnerName; + } + + return repositoryLayout; + } + + public static (string, RepositoryLayout) GetDefaultPathAndLayout(string repositoryPath, UriString cloneUrl) + { + var possibleOwnerPath = Path.GetDirectoryName(repositoryPath); + var possibleOwner = Path.GetFileName(possibleOwnerPath); + if (string.Equals(possibleOwner, cloneUrl.Owner, StringComparison.OrdinalIgnoreCase)) + { + return (Path.GetDirectoryName(possibleOwnerPath), RepositoryLayout.OwnerName); + } + else + { + return (possibleOwnerPath, RepositoryLayout.Name); + } + } + } +} diff --git a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs index e3f3dd0354..4c5b1ce604 100644 --- a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs @@ -214,17 +214,8 @@ void UpdatePath(RepositoryModel repository) { if (previousRepository != null && !string.IsNullOrEmpty(Path)) { - var basePath = System.IO.Path.GetDirectoryName(Path); - var ownerName = System.IO.Path.GetFileName(basePath); - if (previousRepository.Owner == ownerName) - { - basePath = System.IO.Path.GetDirectoryName(basePath); - Path = System.IO.Path.Combine(basePath, repository.Owner, repository.Name); - } - else - { - Path = System.IO.Path.Combine(basePath, repository.Name); - } + var (path, layout) = RepositoryLayoutUtilities.GetDefaultPathAndLayout(Path, previousRepository.CloneUrl); + Path = RepositoryLayoutUtilities.GetDefaultRepositoryPath(repository.CloneUrl, path, layout); } else { From 129154faf1b548c670ed5954120e3f7c2b9a510c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 18 Oct 2019 13:05:54 +0100 Subject: [PATCH 10/18] Remove layout logic from RepositoryCloneService --- .../Services/RepositoryCloneService.cs | 28 ++++++++--------- .../Services/RepositoryCreationService.cs | 2 +- .../Dialog/Clone/RepositoryCloneViewModel.cs | 8 +++-- .../Services/IRepositoryCloneService.cs | 9 ++++-- .../Services/RepositoryLayout.cs | 0 .../Services/RepositoryCloneServiceTests.cs | 30 ++++++++----------- .../Clone/RepositoryCloneViewModelTests.cs | 7 +---- 7 files changed, 41 insertions(+), 43 deletions(-) rename src/{GitHub.App => GitHub.Exports.Reactive}/Services/RepositoryLayout.cs (100%) diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs index d322cf2cf9..72e46226d0 100644 --- a/src/GitHub.App/Services/RepositoryCloneService.cs +++ b/src/GitHub.App/Services/RepositoryCloneService.cs @@ -222,8 +222,7 @@ public async Task CloneRepository( await vsGitServices.Clone(cloneUrl, repositoryPath, true, progress, cancellationToken); await usageTracker.IncrementCounter(x => x.NumberOfClones); - var defaultClonePath = GetDefaultClonePath(); - if (repositoryPath.StartsWith(defaultClonePath, StringComparison.OrdinalIgnoreCase)) + if (repositoryPath.StartsWith(DefaultClonePath, StringComparison.OrdinalIgnoreCase)) { // Count the number of times users clone into the Default Repository Location await usageTracker.IncrementCounter(x => x.NumberOfClonesToDefaultClonePath); @@ -269,22 +268,23 @@ public void SetDefaultClonePath(string repositoryPath, UriString cloneUrl) packageSettings.Value.Save(); } - public string GetDefaultClonePath(UriString cloneUrl = null) + public RepositoryLayout DefaultRepositoryLayout { - var defaultPath = packageSettings.Value.DefaultRepositoryLocation; - if (string.IsNullOrEmpty(defaultPath)) - { - defaultPath = GetLocalClonePathFromGitProvider(operatingSystem.Environment.GetUserRepositoriesPath()); - } + get => RepositoryLayoutUtilities.GetRepositoryLayout(packageSettings.Value.DefaultRepositoryLayout); + } - if (cloneUrl == null) + public string DefaultClonePath + { + get { - return defaultPath; - } + var defaultPath = packageSettings.Value.DefaultRepositoryLocation; + if (!string.IsNullOrEmpty(defaultPath)) + { + return defaultPath; + } - var repositoryLayoutSetting = packageSettings.Value.DefaultRepositoryLayout; - var repositoryLayout = RepositoryLayoutUtilities.GetRepositoryLayout(repositoryLayoutSetting); - return RepositoryLayoutUtilities.GetDefaultRepositoryPath(cloneUrl, defaultPath, repositoryLayout); + return GetLocalClonePathFromGitProvider(operatingSystem.Environment.GetUserRepositoriesPath()); + } } JoinableTaskContext JoinableTaskContext { get; } diff --git a/src/GitHub.App/Services/RepositoryCreationService.cs b/src/GitHub.App/Services/RepositoryCreationService.cs index baef408f5f..7c41ba4d81 100644 --- a/src/GitHub.App/Services/RepositoryCreationService.cs +++ b/src/GitHub.App/Services/RepositoryCreationService.cs @@ -25,7 +25,7 @@ public RepositoryCreationService(IRepositoryCloneService cloneService) public string DefaultClonePath { - get { return cloneService.GetDefaultClonePath(); } + get { return cloneService.DefaultClonePath; } } public IObservable CreateRepository( diff --git a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs index 4c5b1ce604..ed54a2b1c0 100644 --- a/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/Dialog/Clone/RepositoryCloneViewModel.cs @@ -59,7 +59,7 @@ public RepositoryCloneViewModel( var repository = this.WhenAnyValue(x => x.SelectedTabIndex) .SelectMany(x => tabs[x].WhenAnyValue(tab => tab.Repository)); - Path = service.GetDefaultClonePath(); + Path = service.DefaultClonePath; repository.Subscribe(x => UpdatePath(x)); pathWarning = Observable.CombineLatest( @@ -219,13 +219,15 @@ void UpdatePath(RepositoryModel repository) } else { - Path = service.GetDefaultClonePath(repository.CloneUrl); + Path = RepositoryLayoutUtilities.GetDefaultRepositoryPath(repository.CloneUrl, + service.DefaultClonePath, service.DefaultRepositoryLayout); } } catch (Exception) { // Reset illegal paths - Path = service.GetDefaultClonePath(repository.CloneUrl); + Path = RepositoryLayoutUtilities.GetDefaultRepositoryPath(repository.CloneUrl, + service.DefaultClonePath, service.DefaultRepositoryLayout); } previousRepository = repository; diff --git a/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs b/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs index c463e2b1e1..d8df1d8b9d 100644 --- a/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs +++ b/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs @@ -11,9 +11,14 @@ namespace GitHub.Services public interface IRepositoryCloneService { /// - /// Get the default clone path for a given clone URL. + /// Default path to clone things to /// - string GetDefaultClonePath(UriString cloneUrl = null); + string DefaultClonePath { get; } + + /// + /// Default layout of repository directories. + /// + RepositoryLayout DefaultRepositoryLayout { get; } /// /// Infer the default clone path and layout from an example repository path and clone URL. diff --git a/src/GitHub.App/Services/RepositoryLayout.cs b/src/GitHub.Exports.Reactive/Services/RepositoryLayout.cs similarity index 100% rename from src/GitHub.App/Services/RepositoryLayout.cs rename to src/GitHub.Exports.Reactive/Services/RepositoryLayout.cs diff --git a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs index 169778ff43..698bf16cf0 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs @@ -171,36 +171,32 @@ public void SetDefaultClonePath(string targetPath, string cloneUrl, string expec var cloneService = CreateRepositoryCloneService(); cloneService.SetDefaultClonePath(targetPath, cloneUrl); - var defaultPath = cloneService.GetDefaultClonePath(); - Assert.That(defaultPath, Is.EqualTo(expectedDefaultPath)); + Assert.That(cloneService.DefaultClonePath, Is.EqualTo(expectedDefaultPath)); } } - public class TheGetDefaultClonePathMethod + public class TheDefaultClonePathProperty { - [TestCase(@"c:\source\owner\name", "https://github.com/owner/name", "https://github.com/newOwner/newName", @"c:\source\newOwner\newName")] - [TestCase(@"c:\source\name", "https://github.com/owner/name", "https://github.com/newOwner/newName", @"c:\source\newName")] - public void GetDefaultClonePath(string setTargetPath, string setCloneUrl, string getCloneUrl, string expectedDefaultPath) + [TestCase(@"c:\source\owner\name", "https://github.com/owner/name", @"c:\source")] + [TestCase(@"c:\source\name", "https://github.com/owner/name", @"c:\source")] + public void GetDefaultClonePath(string setTargetPath, string setCloneUrl, string expectedDefaultPath) { var cloneService = CreateRepositoryCloneService(); cloneService.SetDefaultClonePath(setTargetPath, setCloneUrl); - var defaultPath = cloneService.GetDefaultClonePath(getCloneUrl); - Assert.That(defaultPath, Is.EqualTo(expectedDefaultPath)); + Assert.That(cloneService.DefaultClonePath, Is.EqualTo(expectedDefaultPath)); } - - [TestCase(@"c:\default", "https://github.com/owner/repositoryName", @"c:\default\owner\repositoryName")] - public void Defaults_To_Owner_Name_Layout(string localClonePath, string getCloneUrl, string expectedDefaultPath) + } + public class TheDefaultRepositoryLayoutProperty + { + [Test] + public void Defaults_To_Owner_Name_Layout() { - var vsGitServices = Substitute.For(); - vsGitServices.GetLocalClonePathFromGitProvider().Returns(localClonePath); - var cloneService = CreateRepositoryCloneService(vsGitServices: vsGitServices); - - var defaultPath = cloneService.GetDefaultClonePath(getCloneUrl); + var cloneService = CreateRepositoryCloneService(); - Assert.That(defaultPath, Is.EqualTo(expectedDefaultPath)); + Assert.That(cloneService.DefaultRepositoryLayout, Is.EqualTo(RepositoryLayout.OwnerName)); } } diff --git a/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs b/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs index 5c3343113f..a81f7380bf 100644 --- a/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs +++ b/test/GitHub.App.UnitTests/ViewModels/Dialog/Clone/RepositoryCloneViewModelTests.cs @@ -412,12 +412,7 @@ static IRepositorySelectViewModel CreateSelectViewModel() static IRepositoryCloneService CreateRepositoryCloneService(string defaultClonePath) { var result = Substitute.For(); - result.GetDefaultClonePath(null).ReturnsForAnyArgs(x => - { - var cloneUrl = x.Arg(); - if (cloneUrl == null) return defaultClonePath; - return Path.Combine(defaultClonePath, cloneUrl.Owner, cloneUrl.RepositoryName); - }); + result.DefaultClonePath.Returns(defaultClonePath); result.DestinationDirectoryExists(directoryExists).Returns(true); result.DestinationDirectoryExists(directoryEmpty).Returns(true); result.DestinationDirectoryEmpty(directoryEmpty).Returns(true); From 09a84d9b930cd04cfba97db5caa2a72c81c2e821 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 18 Oct 2019 17:11:10 +0100 Subject: [PATCH 11/18] Use RepositoryLayout.Default when unitialized --- src/GitHub.App/Services/RepositoryLayoutUtilities.cs | 10 +++------- .../Services/RepositoryLayout.cs | 2 +- .../Services/RepositoryCloneServiceTests.cs | 4 ++-- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/GitHub.App/Services/RepositoryLayoutUtilities.cs b/src/GitHub.App/Services/RepositoryLayoutUtilities.cs index aa4a5e4ea1..26a6b9643b 100644 --- a/src/GitHub.App/Services/RepositoryLayoutUtilities.cs +++ b/src/GitHub.App/Services/RepositoryLayoutUtilities.cs @@ -12,6 +12,7 @@ public static string GetDefaultRepositoryPath(UriString cloneUrl, string default { case RepositoryLayout.Name: return Path.Combine(defaultPath, cloneUrl.RepositoryName); + case RepositoryLayout.Default: case RepositoryLayout.OwnerName: return Path.Combine(defaultPath, cloneUrl.Owner, cloneUrl.RepositoryName); default: @@ -22,13 +23,8 @@ public static string GetDefaultRepositoryPath(UriString cloneUrl, string default public static RepositoryLayout GetRepositoryLayout(string repositoryLayoutSetting) { - RepositoryLayout repositoryLayout; - if (!Enum.TryParse(repositoryLayoutSetting, out repositoryLayout)) - { - repositoryLayout = RepositoryLayout.OwnerName; - } - - return repositoryLayout; + return Enum.TryParse(repositoryLayoutSetting, out RepositoryLayout repositoryLayout) ? + repositoryLayout : RepositoryLayout.Default; } public static (string, RepositoryLayout) GetDefaultPathAndLayout(string repositoryPath, UriString cloneUrl) diff --git a/src/GitHub.Exports.Reactive/Services/RepositoryLayout.cs b/src/GitHub.Exports.Reactive/Services/RepositoryLayout.cs index 12ec162677..aa32bad77a 100644 --- a/src/GitHub.Exports.Reactive/Services/RepositoryLayout.cs +++ b/src/GitHub.Exports.Reactive/Services/RepositoryLayout.cs @@ -2,7 +2,7 @@ { public enum RepositoryLayout { - + Default, // Layout hasn't been specified Name, // Layout repositories by name OwnerName // Layout repositories by owner and name } diff --git a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs index 698bf16cf0..cbe4127bb2 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs @@ -192,11 +192,11 @@ public void GetDefaultClonePath(string setTargetPath, string setCloneUrl, string public class TheDefaultRepositoryLayoutProperty { [Test] - public void Defaults_To_Owner_Name_Layout() + public void Defaults_To_Default_Layout() { var cloneService = CreateRepositoryCloneService(); - Assert.That(cloneService.DefaultRepositoryLayout, Is.EqualTo(RepositoryLayout.OwnerName)); + Assert.That(cloneService.DefaultRepositoryLayout, Is.EqualTo(RepositoryLayout.Default)); } } From c9973dee079e05b47f1e84a4fb1346c5de34758c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 22 Oct 2019 19:03:46 +0100 Subject: [PATCH 12/18] Add tests fro RepositoryLayoutUtilities --- .../RepositoryLayoutUtilitiesTests.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs diff --git a/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs new file mode 100644 index 0000000000..5dff85bbbd --- /dev/null +++ b/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs @@ -0,0 +1,20 @@ +using System; +using GitHub.Services; +using NUnit.Framework; + +public static class RepositoryLayoutUtilitiesTests +{ + public class TheGetDefaultPathAndLayoutMethod + { + [TestCase(@"c:\source\owner\repositoryName", "https://github.com/owner/repositoryName", @"c:\source", RepositoryLayout.OwnerName)] + [TestCase(@"c:\source\owner\differentName", "https://github.com/owner/repositoryName", @"c:\source", RepositoryLayout.OwnerName)] + [TestCase(@"c:\source\repositoryName", "https://github.com/owner/repositoryName", @"c:\source", RepositoryLayout.Name)] + [TestCase(@"c:\source\differentName", "https://github.com/owner/repositoryName", @"c:\source", RepositoryLayout.Name)] + public void GetDefaultPathAndLayout(string repositoryPath, string cloneUrl, string expectPath, RepositoryLayout expectLayout) + { + var (path, layout) = RepositoryLayoutUtilities.GetDefaultPathAndLayout(repositoryPath, cloneUrl); + + Assert.That((path, layout), Is.EqualTo((expectPath, expectLayout))); + } + } +} From dfbf96a1d0fdc93a3f756bff7f8449853051f180 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 25 Oct 2019 11:50:12 +0100 Subject: [PATCH 13/18] Add tests for GetRepositoryLayout Make sure unknown layouts don't throw. --- .../Services/RepositoryLayoutUtilitiesTests.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs index 5dff85bbbd..651a3f64c1 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs @@ -17,4 +17,18 @@ public void GetDefaultPathAndLayout(string repositoryPath, string cloneUrl, stri Assert.That((path, layout), Is.EqualTo((expectPath, expectLayout))); } } + + public class TheGetRepositoryLayoutMethod + { + [TestCase("Name", RepositoryLayout.Name)] + [TestCase("OwnerName", RepositoryLayout.OwnerName)] + [TestCase("Default", RepositoryLayout.Default)] + [TestCase("__UNKNOWN__", RepositoryLayout.Default)] + public void GetDefaultPathAndLayout(string setting, RepositoryLayout expectedLayout) + { + var layout = RepositoryLayoutUtilities.GetRepositoryLayout(setting); + + Assert.That(layout, Is.EqualTo(expectedLayout)); + } + } } From ae3c092b12aa71c0f42bec7db690f746f46769c2 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 25 Oct 2019 12:05:38 +0100 Subject: [PATCH 14/18] Add tests for GetDefaultRepositoryPath --- .../Services/RepositoryLayoutUtilitiesTests.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs index 651a3f64c1..7c0bc20c76 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs @@ -31,4 +31,17 @@ public void GetDefaultPathAndLayout(string setting, RepositoryLayout expectedLay Assert.That(layout, Is.EqualTo(expectedLayout)); } } + + public class TheGetDefaultRepositoryPathMethod + { + [TestCase("https://github.com/github/VisualStudio", @"c:\source", RepositoryLayout.Name, @"c:\source\VisualStudio")] + [TestCase("https://github.com/github/VisualStudio", @"c:\source", RepositoryLayout.OwnerName, @"c:\source\github\VisualStudio")] + [TestCase("https://github.com/github/VisualStudio", @"c:\source", RepositoryLayout.Default, @"c:\source\github\VisualStudio")] + public void GetDefaultRepositoryPath(string cloneUrl, string defaultPath, RepositoryLayout repositoryLayout, string expectedPath) + { + var path = RepositoryLayoutUtilities.GetDefaultRepositoryPath(cloneUrl, defaultPath, repositoryLayout); + + Assert.That(path, Is.EqualTo(expectedPath)); + } + } } From 81cb9be6a43ac68f3f0ed2617cbb533105ee5cef Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 25 Oct 2019 12:34:15 +0100 Subject: [PATCH 15/18] Do case sensitive match of owner When users clone into a directory with the same name as the repository owner, expect the case of the owner to match. This avoids a false positive when cloning a repository from the "github" organization into a direcotry called "GitHub". This is the default directory that GitHub Desktop uses and could be very anoying. --- src/GitHub.App/Services/RepositoryLayoutUtilities.cs | 2 +- .../Services/RepositoryCloneServiceTests.cs | 2 +- .../Services/RepositoryLayoutUtilitiesTests.cs | 11 +++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/GitHub.App/Services/RepositoryLayoutUtilities.cs b/src/GitHub.App/Services/RepositoryLayoutUtilities.cs index 26a6b9643b..8932816a8f 100644 --- a/src/GitHub.App/Services/RepositoryLayoutUtilities.cs +++ b/src/GitHub.App/Services/RepositoryLayoutUtilities.cs @@ -31,7 +31,7 @@ public static (string, RepositoryLayout) GetDefaultPathAndLayout(string reposito { var possibleOwnerPath = Path.GetDirectoryName(repositoryPath); var possibleOwner = Path.GetFileName(possibleOwnerPath); - if (string.Equals(possibleOwner, cloneUrl.Owner, StringComparison.OrdinalIgnoreCase)) + if (string.Equals(possibleOwner, cloneUrl.Owner, StringComparison.Ordinal)) { return (Path.GetDirectoryName(possibleOwnerPath), RepositoryLayout.OwnerName); } diff --git a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs index cbe4127bb2..a13ad516ab 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs @@ -161,7 +161,7 @@ public class TheSetDefaultClonePathMethod { [TestCase(@"c:\source\owner\repositoryName", "https://github.com/owner/repositoryName", @"c:\source")] [TestCase(@"c:\source\owner\newRepositoryName", "https://github.com/owner/repositoryName", @"c:\source")] - [TestCase(@"c:\source\owner\repositoryName", "https://github.com/OWNER/repositoryName", @"c:\source")] + [TestCase(@"c:\source\owner\repositoryName", "https://github.com/OWNER/repositoryName", @"c:\source\owner")] [TestCase(@"c:\owner\repositoryName", "https://github.com/owner/repositoryName", @"c:\")] [TestCase(@"c:\source\repositoryName", "https://github.com/owner/repositoryName", @"c:\source")] diff --git a/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs index 7c0bc20c76..8e38d8e5d1 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryLayoutUtilitiesTests.cs @@ -16,6 +16,17 @@ public void GetDefaultPathAndLayout(string repositoryPath, string cloneUrl, stri Assert.That((path, layout), Is.EqualTo((expectPath, expectLayout))); } + + [TestCase(@"c:\GitHub\VisualStudio", "https://github.com/github/VisualStudio", @"c:\GitHub", RepositoryLayout.Name)] + [TestCase(@"c:\GitHub\github\VisualStudio", "https://github.com/github/VisualStudio", @"c:\GitHub", RepositoryLayout.OwnerName)] + [TestCase(@"c:\github", "https://github.com/github/github", @"c:\", RepositoryLayout.Name)] + [TestCase(@"c:\github\github", "https://github.com/github/github", @"c:\", RepositoryLayout.OwnerName)] + public void Case_Sensitive_Owner(string repositoryPath, string cloneUrl, string expectPath, RepositoryLayout expectLayout) + { + var (path, layout) = RepositoryLayoutUtilities.GetDefaultPathAndLayout(repositoryPath, cloneUrl); + + Assert.That((path, layout), Is.EqualTo((expectPath, expectLayout))); + } } public class TheGetRepositoryLayoutMethod From 6c77d122729baae5e8d2a97340167d64b2340ff7 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 25 Oct 2019 15:40:17 +0100 Subject: [PATCH 16/18] Use same capitalization as underlying file system When remembering the default clone directory, use same capitalization as underlying sile system. --- .../Services/RepositoryCloneService.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs index 72e46226d0..4c8d77121d 100644 --- a/src/GitHub.App/Services/RepositoryCloneService.cs +++ b/src/GitHub.App/Services/RepositoryCloneService.cs @@ -257,6 +257,16 @@ string GetLocalClonePathFromGitProvider(string fallbackPath) public void SetDefaultClonePath(string repositoryPath, UriString cloneUrl) { + if (!Splat.ModeDetector.InUnitTestRunner()) + { + // Use same capitalization as underlying file system + if (DestinationDirectoryExists(repositoryPath)) + { + var dirInfo = operatingSystem.Directory.GetDirectory(repositoryPath); + repositoryPath = GetProperDirectoryCapitalization(operatingSystem, dirInfo); + } + } + var (defaultPath, repositoryLayout) = RepositoryLayoutUtilities.GetDefaultPathAndLayout(repositoryPath, cloneUrl); log.Information("Setting DefaultRepositoryLocation to {Location}", defaultPath); @@ -287,6 +297,19 @@ public string DefaultClonePath } } + static string GetProperDirectoryCapitalization(IOperatingSystem operatingSystem, IDirectoryInfo dirInfo) + { + var parentDirInfo = dirInfo.Parent; + if (parentDirInfo is null) + { + return dirInfo.Name; + } + + var parentDir = GetProperDirectoryCapitalization(operatingSystem, parentDirInfo); + var dirName = parentDirInfo.EnumerateDirectories(dirInfo.Name).First().Name; + return Path.Combine(parentDir, dirName); + } + JoinableTaskContext JoinableTaskContext { get; } } } From 13aa808469fedfe7fa039550a5176ec9fb22c216 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Sat, 2 Nov 2019 17:30:19 +0000 Subject: [PATCH 17/18] Exclude "GitHub" dirs from being a possible owner --- src/GitHub.App/Services/RepositoryLayoutUtilities.cs | 7 ++++++- .../Services/RepositoryCloneServiceTests.cs | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/GitHub.App/Services/RepositoryLayoutUtilities.cs b/src/GitHub.App/Services/RepositoryLayoutUtilities.cs index 8932816a8f..b66b562da1 100644 --- a/src/GitHub.App/Services/RepositoryLayoutUtilities.cs +++ b/src/GitHub.App/Services/RepositoryLayoutUtilities.cs @@ -6,6 +6,10 @@ namespace GitHub.Services { public static class RepositoryLayoutUtilities { + // Exclude directories with a case sensitive name of "GitHub" from being a possible owner. + // GitHub Desktop uses "GitHub" as its default directory and this isn't a user or organization name. + const string GitHubDirectoryName = "GitHub"; + public static string GetDefaultRepositoryPath(UriString cloneUrl, string defaultPath, RepositoryLayout repositoryLayout) { switch (repositoryLayout) @@ -31,7 +35,8 @@ public static (string, RepositoryLayout) GetDefaultPathAndLayout(string reposito { var possibleOwnerPath = Path.GetDirectoryName(repositoryPath); var possibleOwner = Path.GetFileName(possibleOwnerPath); - if (string.Equals(possibleOwner, cloneUrl.Owner, StringComparison.Ordinal)) + if (string.Equals(possibleOwner, cloneUrl.Owner, StringComparison.OrdinalIgnoreCase) && + !string.Equals(possibleOwner, GitHubDirectoryName, StringComparison.Ordinal)) { return (Path.GetDirectoryName(possibleOwnerPath), RepositoryLayout.OwnerName); } diff --git a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs index a13ad516ab..cbe4127bb2 100644 --- a/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/RepositoryCloneServiceTests.cs @@ -161,7 +161,7 @@ public class TheSetDefaultClonePathMethod { [TestCase(@"c:\source\owner\repositoryName", "https://github.com/owner/repositoryName", @"c:\source")] [TestCase(@"c:\source\owner\newRepositoryName", "https://github.com/owner/repositoryName", @"c:\source")] - [TestCase(@"c:\source\owner\repositoryName", "https://github.com/OWNER/repositoryName", @"c:\source\owner")] + [TestCase(@"c:\source\owner\repositoryName", "https://github.com/OWNER/repositoryName", @"c:\source")] [TestCase(@"c:\owner\repositoryName", "https://github.com/owner/repositoryName", @"c:\")] [TestCase(@"c:\source\repositoryName", "https://github.com/owner/repositoryName", @"c:\source")] From 3fa718d4cd0e09cc6e82f5621fe3bfd76abf8656 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Sat, 2 Nov 2019 17:32:01 +0000 Subject: [PATCH 18/18] Revert "Use same capitalization as underlying file system" This reverts commit 6c77d122729baae5e8d2a97340167d64b2340ff7. --- .../Services/RepositoryCloneService.cs | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs index 4c8d77121d..72e46226d0 100644 --- a/src/GitHub.App/Services/RepositoryCloneService.cs +++ b/src/GitHub.App/Services/RepositoryCloneService.cs @@ -257,16 +257,6 @@ string GetLocalClonePathFromGitProvider(string fallbackPath) public void SetDefaultClonePath(string repositoryPath, UriString cloneUrl) { - if (!Splat.ModeDetector.InUnitTestRunner()) - { - // Use same capitalization as underlying file system - if (DestinationDirectoryExists(repositoryPath)) - { - var dirInfo = operatingSystem.Directory.GetDirectory(repositoryPath); - repositoryPath = GetProperDirectoryCapitalization(operatingSystem, dirInfo); - } - } - var (defaultPath, repositoryLayout) = RepositoryLayoutUtilities.GetDefaultPathAndLayout(repositoryPath, cloneUrl); log.Information("Setting DefaultRepositoryLocation to {Location}", defaultPath); @@ -297,19 +287,6 @@ public string DefaultClonePath } } - static string GetProperDirectoryCapitalization(IOperatingSystem operatingSystem, IDirectoryInfo dirInfo) - { - var parentDirInfo = dirInfo.Parent; - if (parentDirInfo is null) - { - return dirInfo.Name; - } - - var parentDir = GetProperDirectoryCapitalization(operatingSystem, parentDirInfo); - var dirName = parentDirInfo.EnumerateDirectories(dirInfo.Name).First().Name; - return Path.Combine(parentDir, dirName); - } - JoinableTaskContext JoinableTaskContext { get; } } }