make terminal title and description configurable and distinguishable #132691
Conversation
…d and cwdFolder is used
src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts
Outdated
Show resolved
Hide resolved
| @@ -305,6 +315,86 @@ export class TerminalService implements ITerminalService { | |||
|
|
|||
| // Create async as the class depends on `this` | |||
| timeout(0).then(() => this._instantiationService.createInstance(TerminalEditorStyle, document.head)); | |||
|
|
|||
| //TODO:@meganrogge is there a better way to do this? | |||
| this.onDidReceiveProcessId(async () => await this._refreshDescriptionCwd()); | |||
There was a problem hiding this comment.
How about we have a new generic pty host message something like this to avoid all this piping through in the future for similar props?
onDidChangeProperty(name: TerminalProperty)
const enum TerminalProperty {
cwd,
initialCwd, // This wouldn't change but we could fetch it using a shared mechanism
// We could add more like below after we validate this is a good idea
// title,
// icon,
}These could always get synced over to the renderer when it changes so accesses are free?
src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts
Outdated
Show resolved
Hide resolved
| if (i === path.length - index) { | ||
| s += `${path[i]}`; | ||
| } else { | ||
| s += `${isWindows ? '\\' : '/'}${path[i]}`; |
There was a problem hiding this comment.
This needs to be based on the remote OS, not local one, otherwise we'd see \home\ on Linux
There was a problem hiding this comment.
We should collapse the path $HOME into ~ when applicable. $HOMEDRIVE + $HOMEPATH is a good alternative to use for Windows https://superuser.com/a/332872
There was a problem hiding this comment.
(on processManager)
| separator: { label: this._configHelper.config.separator } | ||
| }); | ||
|
|
||
| this.getCwd().then(cwd => { |
There was a problem hiding this comment.
The template may not contain the cwd so we want to fetch it optionally. With the property change event this will become sync, but if we ended up keeping this we would want to make setTitle return a Promise.
|
Discussed with @Tyriar and deferring the cwdFolder shortening for now see https://github.com/microsoft/vscode/tree/merogge/cwd2 |
| @@ -1486,7 +1492,10 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { | |||
| // reset cwd if it has changed, so file based url paths can be resolved | |||
| const cwd = await this.getCwd(); | |||
| if (cwd && this._linkManager) { | |||
| this._linkManager.processCwd = cwd; | |||
| if (this._linkManager.processCwd !== cwd) { | |||
There was a problem hiding this comment.
@Tryiar suggeseted link manager could listen to the change property event event instead of on _updateProcessCwd (which would actually be better called _refresh)
This PR fixes #29816