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.

Sign up for GitHub

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

Jump to bottom

Show "None" message in diagnose instead of invalid url #329

Merged
merged 1 commit into from
Oct 4, 2018
Merged

Show "None" message in diagnose instead of invalid url #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions GVFS/GVFS.Common/GVFSEnlistment.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public partial class GVFSEnlistment : Enlistment
public const string BlobSizesCacheName = "blobSizes";

private const string GitObjectCacheName = "gitObjects";
private const string InvalidRepoUrl = "invalid://repoUrl";

private string gitVersion;
private string gvfsVersion;
Expand Down Expand Up @@ -90,7 +89,7 @@ public static GVFSEnlistment CreateWithoutRepoUrlFromDirectory(string directory,
return null;
}

return new GVFSEnlistment(enlistmentRoot, InvalidRepoUrl, gitBinRoot, gvfsHooksRoot);
return new GVFSEnlistment(enlistmentRoot, string.Empty, gitBinRoot, gvfsHooksRoot);
Copy link
Member Author

Choose a reason for hiding this comment

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

@sanoursa we discussed sending null in here, however in the base if you send in null it will attempt to look up details for the repository here. We don't want that in this case. Previously there was a bug fix to diagnose explicitly saying don't fail if you can't get origin info.

We could look at other options like sending in a flag rather than the string.Empty. Let me know what you think.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine. Any place where we try to use this as a URL, we will still fail. And your fix in DiagnoseVerb now ensures that we show a good display string.

}

return null;
Expand Down
2 changes: 1 addition & 1 deletion GVFS/GVFS/CommandLine/DiagnoseVerb.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ protected override void Execute(GVFSEnlistment enlistment)
this.WriteMessage(GVFSPlatform.Instance.GitInstallation.GetInstalledGitBinPath());
this.WriteMessage(string.Empty);
this.WriteMessage("Enlistment root: " + enlistment.EnlistmentRoot);
this.WriteMessage("Cache Server: " + CacheServerResolver.GetUrlFromConfig(enlistment));
this.WriteMessage("Cache Server: " + CacheServerResolver.GetCacheServerFromConfig(enlistment));

string localCacheRoot;
string gitObjectsRoot;
Expand Down