Conversation
If the user is logged out and clicks Create or Clone, they'll go through the login dialog and the repository list will get populated as soon as that's done, before the actual create/clone action starts. We don't want to handle those "new" repos on the list as if they've just been cloned/created.
Conflicts: src/GitHub.VisualStudio/TeamExplorer/Connect/GitHubConnectSection.cs
Instead of just showing the remote url of the new repo, show a link to create a new project or solution locally as well, so the user doesn't get stuck.
I'm doing it this way instead of calling GetRepoFromPath directly from the calling site because I don't want GitHub.VisualStudio to have an explicit reference to libgit2sharp, and I'm not sure if I want to add this to the ISimpleRepositoryModel interface.
Now both creating and cloning show messages with the url of the repository, plus a link to create or open a project/solution, depending on the situation. If it's a created repo, link to the Create dialog. If it's a cloned repo that doesn't have commits, link to the Create dialog. If it's a cloned repo that has commits, link to the Open dialog.
Having commits is not enough to decide whether to show the create or the open dialog. Check whether the repo has subdirectories or files that are not a readme and don't start with .. That at least should be a hint that there's something to open in it.
|
This incorporates and adds to #76, so that one should either go in or be closed before this one goes. |
|
|
|
We can also always show links to the create and open dialogs in all situations. Dunno if that's more confusing to people. |
|
@shana I like the use of a link in the success message. I'm wondering if we should add a bit of context there. For example, "Open an existing project or solution in this repository." /cc @github/desktop for any feels. |
There was a problem hiding this comment.
Do we need to unsubscribe this when you log out?
There was a problem hiding this comment.
I need to test to make sure, but I believe this subscription gets killed when this instance dies (when the flow is done) and doesn't keep anything alive that shouldn't be. I'll check.
I can be pretty nitpicky about code and like to clean things up as we go.
Had a quick chat with @shana about this - start here - and to TL;DR it:
|
|
❤️ @shiftkey |
We were totally leaking like a sieve on event subscriptions to the connection manager.
|
Ok, I'm happy with this for now ✨ |
For the ExportMetadata, the assert didn't cover the potential nullref case, so I refactored it so it would.
There was a problem hiding this comment.
This assert doesn't actually assert the part that could be a null ref, namely x.NamedAttributes. I went ahead and added the proper assert in a commit.
|
Looks good, thanks! |
Fixes to the repo list in the connect page
3 participants
Found a few bugs that needed fixing.
I didn't like the open solution thing popping up automatically, so I changed the notifications to show links to the Open/Create project dialogs when the cloning/creating process is done.
Now it checks whether a cloned repo has commits and content (files or directories) to decide whether to link to the Create or the Open project dialog. It's not too smart, but it should work well for most cases.