Add GitHub helper on Windows using WPF on netfx #40
Conversation
mjcheetham
added
enhancement
|
The GitHub.UI project was originally a 'shared project' in GCM Windows which is no longer recommended or supported. Shared projects used to be basically a big sort of " Should we fold these types into the main |
|
/azp run GCM-PR |
|
No commit pushedDate could be found for PR 40 in repo microsoft/Git-Credential-Manager-Core |
| @@ -19,7 +19,7 @@ | |||
| <!-- Implicit SDK targets import (so we can override the default targets below) --> | |||
| <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" /> | |||
|
|
|||
| <Target Name="CoreCompile" DependsOnTargets="GetBuildVersion"> | |||
| <Target Name="CoreCompile" DependsOnTargets="GetBuildVersion" Condition="'$(OSPlatform)'=='osx'"> | |||
There was a problem hiding this comment.
Add an extra condition here to prevent this project from building on non-macOS systems.
This shouldn't be needed as the solution does not select this project for building except in Mac* configurations.. however VS Windows feels the need to try and build it anyway 😕 Ugh.
|
|
||
| public TtyGitHubPromptAuthentication(ICommandContext context) | ||
| public async Task<ICredential> GetCredentialsAsync(Uri targetUri) |
There was a problem hiding this comment.
The logic here is now:
if helper exists
exec helper
else
exec terminal-prompt
endif
There was a problem hiding this comment.
If we can't find a helper would it be because one doesn't match or because the exe is missing / in the wrong place?
There was a problem hiding this comment.
Or the platform doesn't have a helper (like macOS in this case)
There was a problem hiding this comment.
As and when a Mac GitHub helper is available, they just need to drop the binary with that name, compliant with same the wire/cmdarg protocol as the Windows one and it'll start working.
There was a problem hiding this comment.
For Windows I'll add a warning message if we cannot find it.
|
|
||
| namespace Microsoft.Git.CredentialManager.Authentication | ||
| { | ||
| public abstract class AuthenticationBase |
There was a problem hiding this comment.
Pulling out the shared bits between the Microsoft and GitHub auth components: specifically helper process invocation and EnsureTerminalPromptsEnabled.
| && envarPrompts == "0") | ||
| string helperName = GitHubConstants.AuthHelperName; | ||
|
|
||
| if (PlatformUtils.IsWindows()) |
There was a problem hiding this comment.
I'm wondering if it would be easier to have constant GitHubConstants.AuthHelperName.Windows / GitHubConstants.AuthHelperName.NonWindows .
Food for thought.
There was a problem hiding this comment.
I did consider this, but then we'd still need this "if Windows then" check. Might as well just enforce the difference is only the ".exe" part rather than allowing a different name entirely.
|
@mjcheetham, can you add some pictures of GUI for reference. I believe it should be very similar to the old. |
| @@ -0,0 +1,283 @@ | |||
| <ResourceDictionary | |||
There was a problem hiding this comment.
@mjcheetham , can you confirm the wpf is mostly unchanged?
It looks that way from my quick glance comparison
There was a problem hiding this comment.
The WPF/XAML code is unchanged except for namespace and white space changes.
Add a Windows authentication helper for GitHub using WPF on .NET Framework 4.6.1.
We can revisit this in the future if we need to. I don't think we should block on his now. |
Here you go @jeschu1! Username & password:
Auth code (app):
Auth code (SMS):
|
Since #40 is complete, we now have UI for GitHub on Windows.. updating the markdown current status table. Also minor formatting change to the table, we now centre the tick columns.
Port the GitHub WPF-based UI from GCM Windows to GCM Core.
Fixes #36