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

make terminal suggest addon more generic #233013

Open
wants to merge 130 commits into
base: main
Choose a base branch
from

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Nov 4, 2024

fixes #233364

@meganrogge
Copy link
Contributor Author

the cachedPwshCommands can also be non static now

@Tyriar
Copy link
Member

Tyriar commented Nov 8, 2024

the cachedPwshCommands can also be non static now

That one's static to share across all terminals, since not having cached commands can lock up the terminal for potentially seconds

@meganrogge meganrogge force-pushed the merogge/start-terminal-suggest branch from f3a649d to 19a1981 Compare November 8, 2024 18:44
Tyriar
Tyriar previously requested changes Nov 8, 2024
return;
}
this._lastAcceptedCompletionTimestamp = Date.now();
SuggestAddon.lastAcceptedCompletionTimestamp = Date.now();
Copy link
Member

Choose a reason for hiding this comment

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

We don't want this to be static as it's intentionally tracked per suggest addon. It's unlikely to cause a problem if it were static, but it makes the code a bit harder to understand being static imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is referenced in the pwshAddon

if (this._lastUserDataTimestamp > SuggestAddon.lastAcceptedCompletionTimestamp) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[+character does not trigger terminal completions
4 participants