-
Notifications
You must be signed in to change notification settings - Fork 130
Draft spec for auto-discovering feedback provider and tab-completer #386
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
f885ea7
c9d13ba
3c8db5d
e5c3d48
95a51b4
2b1427f
fcf87e7
ab02441
db62c29
c915529
b6596bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,15 +72,15 @@ feedbacks | |
│ | ||
├───_startup_ | ||
│ ├───linux-command-not-found | ||
│ │ linux-command-not-found.json | ||
│ │ linux-command-not-found.psd1 | ||
│ │ | ||
│ └───winget-command-not-found | ||
│ winget-command-not-found.json | ||
│ winget-command-not-found.psd1 | ||
├───git | ||
│ git.json | ||
│ git.psd1 | ||
│ | ||
├───kubectl | ||
│ kubectl.json | ||
│ kubectl.psd1 | ||
│ | ||
├───... | ||
``` | ||
|
@@ -89,13 +89,13 @@ Each item within `feedbacks` is a folder. | |
- `_startup_`: feedback providers declared in this folder will be auto-loaded at the startup of an interactive `ConsoleHost` session. | ||
- `git` or `kubectl`: feedback provider for a specific native command should be declared in a folder named after the native command. It will be auto-loaded when the native command gets executed. | ||
|
||
Within each sub-folder, a JSON file named after the folder name should be defined to configure the auto-discovery for the feedback provider. | ||
Within each sub-folder, a `.psd1` file named after the folder name should be defined to configure the auto-discovery for the feedback provider. | ||
|
||
```JSONC | ||
{ | ||
"module": "<module-name-or-path>[@<version>]", // Module to load to register the feedback provider. | ||
"arguments": ["<arg1>", "<arg2>"], // Optional arguments for module loading. | ||
"disable": false, // Control whether auto-discovery should find this feedback provider. | ||
```powershell | ||
@{ | ||
module = '<module-name-or-path>[@<version>]', # Module to load to register the feedback provider. | ||
arguments = @('<arg1>', '<arg2>'), # Optional arguments for module loading. | ||
disable = $false, # Control whether auto-discovery should find this feedback provider. | ||
} | ||
``` | ||
|
||
|
@@ -124,16 +124,12 @@ Also, it's allowed to register multiple feedback providers for a single native c | |
|
||
#### Discussion Points | ||
|
||
1. Should we expand the string value for `module` key, or always treat the value as literal? | ||
It feels like a useful feature, but could come with security implications, especially in System Lockdown Mode (SLM) or Restricted remoting environments. | ||
- Note: PowerShell data file (`.psd1`) doesn't allow environment variables. | ||
|
||
2. Should we add another key to indicate the target OS? | ||
1. Should we add another key to indicate the target OS? | ||
- A feedback provider may only work on a specific OS, such as the `"WinGet CommandNotFound"` feedback provider only works on Windows. | ||
- Such a key could be handy if a user wants to share the feedback/tab-completer configurations among multiple machines via a cloud drive. | ||
|
||
3. Do we really need a folder for each feedback provider? | ||
For example, can we simply have the files `git.json` and `kubectl.json` under the `feedbacks` folder, and the files `linux-command-not-found.json` and `winget-command-not-found.json` under the `_startup_` folder? | ||
2. Do we really need a folder for each feedback provider? | ||
For example, can we simply have the files `git.psd1` and `kubectl.psd1` under the `feedbacks` folder, and the files `linux-command-not-found.psd1` and `winget-command-not-found.psd1` under the `_startup_` folder? | ||
- Since it's possible to have non-module feedback provider that comes with a DLL only, | ||
then the DLL might need to be deployed along with the configuration. | ||
In that case, the tool that deploys the DLL will either copy the DLL directly to `feedbacks`, | ||
|
@@ -160,12 +156,12 @@ completions | |
│ | ||
├───_startup_ | ||
│ └───unix-completer | ||
│ unix-completer.json | ||
│ unix-completer.psd1 | ||
├───git | ||
│ git.json | ||
│ git.psd1 | ||
│ | ||
├───az | ||
│ az.json | ||
│ az.psd1 | ||
│ | ||
├───... | ||
``` | ||
|
@@ -174,14 +170,14 @@ Each item within `completions` is a folder. | |
- `_startup_`: tab-completer declared in this folder will be auto-loaded at the startup of an interactive `ConsoleHost` session. | ||
- `git` or `az`: tab-completer for a specific native command should be declared in a folder named after the native command. It will be auto-loaded when user tab completes on the command. | ||
|
||
Within each sub-folder, a JSON file named after the folder name should be defined to configure the auto-discovery for the tab-completer. | ||
Within each sub-folder, a `psd1` file named after the folder name should be defined to configure the auto-discovery for the tab-completer. | ||
|
||
```JSONC | ||
{ | ||
"module": "<module-name-or-path>[@<version>]", // Module to load to register the completer. | ||
"script": "<script-path>", // Script to run to register the completer. | ||
"arguments": ["<arg1>", "<arg2>"], // Optional arguments for module loading or script invocation. | ||
"disable": false, // Control whether auto-discovery should find this completer. | ||
```powershell | ||
@{ | ||
module = '<module-name-or-path>[@<version>]', # Module to load to register the completer. | ||
script = '<script-path>', # Script to run to register the completer. | ||
arguments = @('<arg1>', '<arg2>'), # Optional arguments for module loading or script invocation. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Discussion point] Do we really need the |
||
disable" = $false, # Control whether auto-discovery should find this completer. | ||
} | ||
``` | ||
|
||
|
@@ -213,7 +209,7 @@ Different discussions: | |
1. Do we really need a folder for each feedback provider? | ||
- [dongbo] Yes, I think we need. | ||
Appx and MSIX packages on Windows have [many constraints](https://github.com/PowerShell/PowerShell/issues/17283#issuecomment-1522133126) that make it difficult to integrate with a broader plugin ecosystem. The way for such an Appx/MSIX tool to expose tab-completer could be just running the tool with a special flag, such as `<tool> --ps-completion`, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Discussion point] Appx/MSIX package cannot integrate with a broader plugin ecosystem easily [reference]:
Additional constraints that make it difficult to integrate Appx/MSIX with a broader plugin ecosystem:
We should support Appx/MSIX packages without requiring user's manual work. But, how to achieve it given the constrains? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's trivial. An extensibility model has 2 requirements
Both have multiple options with the simplest being Discovery=AppExtension and Access=DynamicDependencies. More details below. DiscoveryA Powershell extension (be it completer or other scenarios) needs to advertise it exists such that Powershell can discover it. MSIX' standard recommendation is for the packaged app to declare an appextension in its manifest and Powershell uses the AppExtensionCatalog API to enumerate these extensions and any needed info about them. Example snippet in
and sample enumeration
AccessNow that you know a package has Powershell extension(s) of interest you need to access file(s) in the package. This requires Windows knows Powershell is using a package's content. The simplest solution is to use the Dynamic Dependencies API to tell Windows you're using the package's content. This ensures Windows doesn't try to service the package (e.g. updating or removing the package) while in use. It also adds the package to the Powershell process' package graph, making its resources available for LoadLibrary, ActivateInstance and other APIs. Recommended But Not OnlyThese are the commonly recommended solutions for Discovery and Access but there are other options. Discovery AlternativesFor access the Access AlternativesDynamic Dependencies enables you to use several tech to interact with a packaged Powershell extension, e.g. WinRT inproc and out-of-proc (ActivateInstance), inproc DLLs (LoadLibrary), inproc .NET assemblies (Assembly.Load*()), read a text file (CreateFile(GetPackagePath()+"\foo.ps1") and other like tech. NOTE: Inproc is generally discouraged for extensibility models given the reliability and security implications. Technically feasible of course as there are times it's necessary (especially older legacy software), but if you can do better, do better :-) There are other options if you need code a package -- Packaged COM OOP servers, AppServices, etc. Some don't require Dynamic Dependencies but bring their other caveats to the table so while supported the Dynamic Dependencies options are recommended. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And note, the MSIX discovery+access recommendations are easily secure. Package content is strongly secure and verifiably trustable. When Powershell uses a package's content it can be assured the content is authentic and unmodified from the developer. Package authors can be Store signed or declare
and its content is hardened even a process running as LocalSystem can't alter the package's files. Powershell can use various APIs to confirm the package is signed and is unmodified e.g. Package.SignatureKind, Package.Status.VerifyIsOK(), Package.VerifyContentIntegrityAsync(). This can jive well with Powershell's |
||
to output some PowerShell script text for the user to run. In that case, the user will need to manually save the script text to a file and place the file next to the `tool.json` file. | ||
to output some PowerShell script text for the user to run. In that case, the user will need to manually save the script text to a file and place the file next to the `tool.psd1` file. | ||
Having a folder is useful to group them together in that case. | ||
|
||
2. When running a script, it will run in the PSReadLine's module scope when completion is triggered from within PSReadLine. | ||
|
@@ -241,7 +237,7 @@ This feature is only for interactive session, so we need to decide on when the f | |
5. We report progress when loading `feedback` or `completer` at startup, so how to allow users to disable the progress report? | ||
- We have the `-NoProfileLoadTime` flag today to not show the time taken for running profile. | ||
6. How about on a System Lockdown Mode (SLM) or Restricted remoting environments? | ||
|
||
- We use `.psd1` file for metadata, which can be signed if needed. | ||
|
||
### Unified Location for load-at-startup Configurations | ||
|
||
|
@@ -254,14 +250,14 @@ Given that, maybe it's better to have a unified location for all the load-at-sta | |
- All modules or scripts that need to be processed at session startup should have configurations deployed in the `startup` folder. | ||
|
||
Each item within `startup` is a folder, whose name should be the friendly name of the component, e.g. `"UnixTabCompletion"`. | ||
Within each sub-folder, a JSON file named after the folder name should be defined to configure the auto-discovery of the component. | ||
|
||
```JSONC | ||
{ | ||
"module": "<module-name-or-path>[@<version>]", // Module to load. | ||
"script": "<script-path>", // Script to run. | ||
"arguments": ["<arg1>", "<arg2>"], // Optional arguments for module loading or script invocation. | ||
"disable": false, // Control whether auto-discovery should find this completer. | ||
Within each sub-folder, a `.psd1` file named after the folder name should be defined to configure the auto-discovery of the component. | ||
|
||
```powershell | ||
@{ | ||
module = '<module-name-or-path>[@<version>]', # Module to load. | ||
script = '<script-path>', # Script to run. | ||
arguments = @('<arg1>', '<arg2>'), # Optional arguments for module loading or script invocation. | ||
disable = $false, # Control whether auto-discovery should find this completer. | ||
} | ||
``` | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be good, thinking about the linux cmd-not-found predictor. Also if a user wants to bring this configuration across their different machines they dont need to have multiple different folder structures? I am not entirely sure how they would necessarily do it but I know some community folks use external tools to share their $PROFILE across machines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Folder structures under
feedbacks
andcompletions
are the same. I guess a user can create a symbolic link to make<personal>/powershell/feedbacks
or<personal>/powershell/completions
points to the folders from a cloud drive.