Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

add a prePrompt_functions variable #15104

Closed
LuanVSO opened this issue Mar 18, 2021 · 22 comments
Closed

add a prePrompt_functions variable #15104

LuanVSO opened this issue Mar 18, 2021 · 22 comments
Labels
WG-Interactive-Console the console experience

Comments

@LuanVSO
Copy link

LuanVSO commented Mar 18, 2021

Description of the new feature/enhancement

it would be the same as zsh's precmd_functions, which is a list of functions that are called before every prompt,
this would allow module makers to add functions to the prompt without overwriting it.

Proof of concept

this allows any script to register a function to run before every command by simply doing
$prePrompt_functions += "<function_name>"

$prePrompt_functions = @()
$prevprompt = $Function:prompt
function prompt {
	$prePrompt_functions | Invoke-Expression
	return $prevprompt.invoke()
}
@iSazonov
Copy link
Collaborator

How order the functions?

@LuanVSO
Copy link
Author

LuanVSO commented Mar 19, 2021

Since it is an array, one could check if there are other entries and reorther them if they really really need to

@iSazonov
Copy link
Collaborator

iSazonov commented Mar 19, 2021

If we described common scenarios we could make the functionality more smart and convenient.

For example, if a module exports a function with a predefined name, then Import-Module could automatically hook it.

/cc @mklement0

@mklement0
Copy link
Contributor

Pragmatically speaking, it's quite easy - though somewhat hackish - to inject additional commands into an existing prompt function:

$function:prompt = "write-host new; $function:prompt"

Note that removing such a command later would be trickier, however (best done via an identifying comment accompanying the injected command)


If we want an officially supported mechanism, my recommendation would be to define a new execution-context event, $ExecutionContext.InvokeCommand.PreExecutionAction,
as previously pondered at #14484 (comment), modeled on the existing $ExecutionContext.InvokeCommand.PreCommandLookupAction hook - though note that the latter doesn't support attaching multiple event handlers and isn't PowerShell-code-friendly.

@iSazonov
Copy link
Collaborator

It is seem better to continue the discussion in #14484

@mklement0
Copy link
Contributor

mklement0 commented Mar 19, 2021

@iSazonov, #14484 is a bit of a dead end, however, because it asks for very specific functionality that I don't think will ever be implemented: the ability to reprint the already-printed prompt string, based on an event hook to be invoked after submission of a command line, but before its execution.

Only the latter part - the event hook - is what we're discussing here, and only that is worth considering as an enhancement.
Either way, this issue belongs in the PowerShell repo, not here (the prompt function feature is implemented there).

@daxian-dbw daxian-dbw transferred this issue from PowerShell/PSReadLine Mar 25, 2021
@vexx32
Copy link
Collaborator

vexx32 commented Mar 26, 2021

IIRC @Jaykul's PowerLine module has some functionality like this?

@LuanVSO
Copy link
Author

LuanVSO commented Mar 26, 2021

yeah his implementation looks cleaner, but having a way to do that by default on PowerShell nice for the "ecosystem"

@daxian-dbw daxian-dbw added Needs-Triage The issue is new and needs to be triaged by a work group. WG-Interactive-Console the console experience labels Jun 25, 2021
@daxian-dbw
Copy link
Member

daxian-dbw commented Jun 26, 2021

@LuanVSO The ask is to allow custom script to run before prompt runs, right? How will this technique be used? Can you share some real world examples?

@daxian-dbw daxian-dbw added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jun 26, 2021
@LuanVSO
Copy link
Author

LuanVSO commented Jun 26, 2021

@daxian-dbw was thinking of this being a way for module authors to add features to the user prompt, with for example support for (osc7 MS/terminal#3158)

this feature idea is from Zsh's $precmd_functions

@Jaykul
Copy link
Contributor

Jaykul commented Aug 28, 2021

PowerLine was just a proof of concept, see Why PowerLine.

The goal was always to convince the PowerShell team to change the default prompt function to

function prompt {
    -join $prompt.Invoke()
}

.. and put the current prompt into that new global as three statements:

[System.Collections.Generic.List[ScriptBlock]]$Prompt = @(
    { "PS " }
    { $executionContext.SessionState.Path.CurrentLocation }
    { '>' * ($nestedPromptLevel + 1) }
)

@ghost
Copy link

ghost commented Jan 8, 2022

This issue has been marked as "Waiting on Author" and has not had any activity for 7 day. It has been closed for housekeeping purposes.

@ghost ghost closed this as completed Jan 8, 2022
@LuanVSO
Copy link
Author

LuanVSO commented Jan 9, 2022

This issue has been marked as "Waiting on Author" and has not had any activity for 7 day. It has been closed for housekeeping purposes.

Well I did reply but it seems like it Didn't remove the tag

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jan 9, 2022
@daxian-dbw daxian-dbw reopened this Jan 10, 2022
@adityapatwardhan
Copy link
Member

Thanks for the suggestion! The Working Group discussed this and we think that this can be done by updating the prompt function. On zsh, prompt is a variable whereas in PowerShell it is a function.

@adityapatwardhan adityapatwardhan added Resolution-Declined The proposed feature is declined. and removed Needs-Triage The issue is new and needs to be triaged by a work group. labels Mar 1, 2023
@daxian-dbw
Copy link
Member

we think that this can be done by updating the prompt function.

VSCode's shell integration feature does exactly this, by first saving the original prompt function to a global variable, and then replace the prompt function with its logic plus invoking the original prompt function:

# the VSCode injected prompt function logic
...

# Run the original prompt
$Result += $Global:__VSCodeOriginalPrompt.Invoke()
# Write command started
$Result += "$([char]0x1b)]633;B`a"
...
return $Result

@vexx32
Copy link
Collaborator

vexx32 commented Mar 1, 2023

Right, and the problem with doing that is that (as I think jaykul and others covered in the thread pretty well already) if anyone else is also overwriting the prompt function, there's a very high probability of collision.

The ask was to have as a builtin API some way to execute multiple functions alongside (before?) the prompt, without having to be concerned about them all being overwritten by something else clobbering the prompt function.

The fact that prompt can be completely overwritten is sort of the problem statement moreso than the conclusion here, I feel.

@daxian-dbw
Copy link
Member

if anyone else is also overwriting the prompt function, there's a very high probability of collision.

PowerShell is mostly single threaded, and importing multiple modules into the same Runspace will be synchronous operation, so I think there shouldn't be a collision in updating the prompt function. In what scenario would the collision happen?

@vexx32
Copy link
Collaborator

vexx32 commented Mar 2, 2023

It's not that uncommon for modules to want to add things to the prompt -- oh-my-posh is perhaps one of the more extensive examples, but I've seen modules that just add for example a git status display to the prompt as well.

Loading both modules currently requires you to figure out which one you want to "win" and you have to hope you can figure out how to configure the thing the other module does within the framework that the "winning" module provides, and hope it's good enough, or else start from scratch again and nullify the point of some of these modules existing in the first place.

@daxian-dbw daxian-dbw added Needs-Triage The issue is new and needs to be triaged by a work group. and removed Resolution-Declined The proposed feature is declined. labels Mar 2, 2023
@daxian-dbw
Copy link
Member

I think adding something to the prompt is not a problem.
Module A can save the current prompt function to $global:__OriginalPromptA, and then overwrite the prompt function with A's new logic, which calls $global:__OriginalPromptA in it. Since all modules are loaded synchronously, Module B will do the similar thing -- saving the current prompt function (content form A) to $global:__OriginalPromptB then overwriting the prompt function with B's new logic, which calls $global:__OriginalPromptB. In this way, both Module A and B may win, assuming their new logics and the very initial prompt can all work together (there may be a higher chance, I guess, that they just don't).

However, I do see a problem when Module A and Module B unregister the changes they made to the prompt function. In the above case, if Module A is unloaded first, then it will restore the prompt function to $global:__OriginalPromptA, which will erase the changes made by Module B, even though B is still loaded. So, the current prompt function design seems insufficient for this unregistering scenario. However, I don't know whether there are any real-world cases that require undoing the changes made to prompt when a module is being unloaded.

Given that, I removed the Resolution-Declined label to facilitate further discussions around this. Later on, the WG can re-review this issue.

@theJasonHelmick
Copy link
Collaborator

I wonder if a registration system is required so that modules could register a string for the prompt function --- I also wonder what the security ramifications are with injecting arbitrary code into the prompt. The working group would encourage further conversation -- We think this could be implemented outside of the engine, and would look forward to a prototype from the community. We would like to see further discussion on this issue.

@theJasonHelmick
Copy link
Collaborator

theJasonHelmick commented Mar 22, 2023

One example as reference is of course @Jaykul's work: https://github.com/Jaykul/PowerLine

GitHub
A more PowerShell prompt. Contribute to Jaykul/PowerLine development by creating an account on GitHub.

@theJasonHelmick
Copy link
Collaborator

The WG re-reviewed this issue -- while we understand the desire, we are still unclear on scenarios that this would address and are suspect of the challenges to implement. Rather than declining this, we would like to convert this to a discussion and continue to work with the community to better understand.

@theJasonHelmick theJasonHelmick removed the Needs-Triage The issue is new and needs to be triaged by a work group. label Aug 9, 2023
@theJasonHelmick theJasonHelmick converted this issue into discussion #20099 Aug 9, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
WG-Interactive-Console the console experience
Projects
None yet
Development

No branches or pull requests

8 participants