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.

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

Collect all known PowerShell Core installations for dynamic profiles #4273

Merged
8 commits merged into from
Jan 31, 2020

Conversation

DHowett-MSFT
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT commented Jan 17, 2020

This pull request teaches the PowerShell Core generator about a bunch of different locations in which it might find a PowerShell.

These instances will be sorted, a leader will be elected, and that leader will be promoted and given the vaunted title of "PowerShell".

Names will be generated for the rest.

The sort order is documented in the comments, but that comment will be replicated here:

// <-- Less Valued .................................... More Valued -->
// |                 All instances of PS 6                 | All PS7  |
// |          Preview          |          Stable           | ~~~      |
// |  Non-Native | Native      |  Non-Native | Native      | ~~~      |
// | Trd  | Pack | Trd  | Pack | Trd  | Pack | Trd  | Pack | ~~~      |
// (where Pack is a stand-in for store, scoop, dotnet, though they have their own orders,
// and Trd is a stand-in for "Traditional" (Program Files))

Closes #2300

@DHowett-MSFT
Copy link
Contributor Author

I'm fixing the guid to the legacy pscore guid so that users who have it set as their default profiles will not be disappointed. I'm fixing the name to PowerShell because otherwise we would hardcode a versioned name in profiles.json and then when that version moved on we wouldn't be able to override the name!

@DHowett-MSFT
Copy link
Contributor Author

scoop, dotnet global, x86, ARM and msix are not localizable.

"Preview" - we need to discuss that.

@DHowett-MSFT
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ejsmith
Copy link

ejsmith commented Jan 29, 2020

"Preview" - we need to discuss that.

Yes, please discover and setup any preview releases I have installed.

@DHowett-MSFT
Copy link
Contributor Author

@ejsmith oh absolutely! This is about whether the word preview is localizable. Sorry, that context was from the previous sentence and fairly sparse. :)

Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

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

Looks good. My comments are not high importance.

@DHowett-MSFT
Copy link
Contributor Author

Here's a question: should 7 preview actually fall below 6 GA? Or should we always prefer the higher version numbered one

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

I think I only have nits, but I might want to see how the discussion plays out before I sign off. Or Carlos could sign off too, I don't really care

@@ -70,6 +70,7 @@ class TerminalApp::Profile final
bool HasConnectionType() const noexcept;
GUID GetConnectionType() const noexcept;

void SetGuid(GUID guid) noexcept { _guid = guid; }
Copy link
Member

Choose a reason for hiding this comment

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

I mean I get it, but this should probably be in the cpp

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

Seems fine to me - I'll leave the merge to you to re-run CI, maybe push a commit, idc.

@miniksa
Copy link
Member

miniksa commented Jan 30, 2020

Here's a question: should 7 preview actually fall below 6 GA? Or should we always prefer the higher version numbered one

No, in my opinion. If you're the kind of person to install previews, you want the leading edge version number even if it's not ready yet. And if you're a person who wants 6 GA as your default... you could just... not install the previews. Or make some of your own profiles.

@DHowett-MSFT DHowett-MSFT changed the title Collect all known PowerShell Core install locations when building dynamic profiles Collect all known PowerShell Core installations for dynamic profiles Jan 31, 2020
@DHowett-MSFT DHowett-MSFT added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 31, 2020
@ghost
Copy link

ghost commented Jan 31, 2020

Hello @DHowett-MSFT!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit bba0527 into master Jan 31, 2020
@ghost ghost deleted the dev/duhowett/pscore branch January 31, 2020 04:17
@ghost
Copy link

ghost commented Feb 13, 2020

🎉Windows Terminal Preview v0.9.433.0 has been released which incorporates this pull request.:tada:

Handy links:

@bys1123
Copy link

bys1123 commented Mar 6, 2020

Maybe should add powershell core 7 stable profile now?

@DHowett-MSFT
Copy link
Contributor Author

@bys1123 that’s the point, it is automatic.

@bys1123
Copy link

bys1123 commented Mar 7, 2020

@DHowett-MSFT Thanks for explains. But, powershell core 7 stable didn't show up on my Terminal list. Is Windows Terminal Preview v0.9.433.0 included this PR?

@DHowett-MSFT
Copy link
Contributor Author

Yes. Can you share a copy of your profiles.json?

@bys1123
Copy link

bys1123 commented Mar 8, 2020

Here you are:

// To view the default settings, hold "alt" while clicking on the "Settings" button.
// For documentation on these settings, see: https://aka.ms/terminal-documentation

{
    "$schema": "https://aka.ms/terminal-profiles-schema",

    "defaultProfile": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}",

    "profiles":
    {
        "defaults":
        {
            // Put settings here that you want to apply to all profiles
        },
        "list":
        [
            {
                // Make changes here to the powershell.exe profile
                "guid": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}",
                "name": "Windows PowerShell",
                "commandline": "powershell.exe",
                "hidden": false
            },
            {
                // Make changes here to the cmd.exe profile
                "guid": "{0caa0dad-35be-5f56-a8ff-afceeeaa6101}",
                "name": "cmd",
                "commandline": "cmd.exe",
                "hidden": false
            },
            {
                "guid": "{c6eaf9f4-32a7-5fdc-b5cf-066e8a4b1e40}",
                "hidden": false,
                "name": "Ubuntu-18.04",
                "source": "Windows.Terminal.Wsl"
            },
            {
                "guid": "{b453ae62-4e3d-5e58-b989-0a998ec441b8}",
                "hidden": false,
                "name": "Azure Cloud Shell",
                "source": "Windows.Terminal.Azure"
            }
        ]
    },

    // Add custom color schemes to this array
    "schemes": [],

    // Add any keybinding overrides to this array.
    // To unbind a default keybinding, set the command to "unbound"
    "keybindings": []
}

@DHowett-MSFT
Copy link
Contributor Author

And where did you install powershell core 7?

@bys1123
Copy link

bys1123 commented Mar 8, 2020

E:\Program Files\PowerShell\7

@DHowett-MSFT
Copy link
Contributor Author

Sorry, Terminal only checks normal places. It uses your system’s official “Program Files” directory to determine where to look.

Anybody installing autodetectable shells to weird locations will need to make their own profiles for them.

@bys1123
Copy link

bys1123 commented Mar 9, 2020

But vscode detect it automatically.

@Krantz-XRF
Copy link

@bys1123 I have a workaround for this: make a symlink from where you installed PowerShell to C:\Program Files\PowerShell and let Windows Terminal regenerate the profile.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Marked for automatic merge by the bot when requirements are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add additional PowerShell Core paths (preview, scoop, etc.) to dynamic profile generator
7 participants