Skip to content

Conversation

rose-m
Copy link
Contributor

@rose-m rose-m commented May 31, 2021

No description provided.

@rose-m rose-m self-assigned this May 31, 2021
const driverUri = generateUri(options);

process.title = `mongosh ${redactCredentials(driverUri)}`;
setTerminalWindowTitle(process.title);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since process.title is a getter/setter that actually queries the current process title (which might differ from what we have just set it to, for example, because it was truncated or because setting process.title isn’t available on the current platform), I think it makes sense to store the title in a variable and then set it both with setTerminalWindowTitle and process.title separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

O.o I didn't know that might actually change otherwise!

Copy link
Collaborator

@addaleax addaleax May 31, 2021

Choose a reason for hiding this comment

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

Yeah … the problem is that on UNIX systems, this actually works by overwriting the original argv array that was passed to main; and so the maximum length of process.title is also the maximum length of the original command line:

$ node
Welcome to Node.js v17.0.0-pre.
Type ".help" for more information.
> process.title = 'a'.repeat(100)
'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
> process.title.length
4

@rose-m rose-m force-pushed the MONGOSH-725-set-terminal-window-title branch from 8ef93cd to a30201e Compare May 31, 2021 10:26
@rose-m rose-m merged commit 75d5a70 into main May 31, 2021
@rose-m rose-m deleted the MONGOSH-725-set-terminal-window-title branch May 31, 2021 12:10
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.

2 participants