Fix keys used to resolve terminal settings#132851
Conversation
|
Verified couple of times - with this change I can revert back changes in async createTerminal(options?: ICreateTerminalOptions): Promise<ITerminalInstance> {
- if (!this._availableProfiles) {
- await this._refreshAvailableProfilesNow();
- }
- const config = options?.config || this._availableProfiles?.find(p => p.profileName === this._defaultProfileName);
+ const config = options?.config;Will create follow-up PR with this change and related cleanup so it can be verified in Insiders before release, while I think c813199 should be included into Recovery release. |
|
@Tyriar could you please take a look on this PR. It's good candidate for Recovery release as it solves set of issues with Terminal profiles. |
|
Thanks! |
|
@IllusionMH FYI I don't think we can remove that |
|
@Tyriar agree about check and awaiting for . Borrowed some code from my POC that helps to repro race conditions to repro this one again 😄 For some reason it was harder then before. However I doubt that P.S. Also Thank you and @meganrogge for considering it for Recovery and your patience! ❤️ |
|
Yep that might not be needed due to resolving happening later on: vscode/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts Lines 413 to 416 in 3a90216 Little hesitant to change unless there's an issue though. |
|
Will this resolve #132766 as well? |
|
I believe that it should. You can try latest Insiders build which includes this change and another tuning form Tyriar. |
This PR fixes #132718 ("open in integrated terminal") and most likely #132824 (npm scripts, etc.)
Also this one looks like resolving infamous #132150 as well.
TerminalSettingPrefix.*enum members already have.in the end (e.g'terminal.integrated.shell.') and initially used in simple string concatenationperfix + platformbut in some template literal values has additional.added which results inundefinedinstead of user configured settings.@meganrogge @Tyriar it would be nice to consider this for recovery as well.