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

Harden random renderer-arg in powerline-setup.fish #1938

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

simonfxr
Copy link

Select the random renderer-arg in _powerline_update from a bigger domain. The previous version was overly optimistic, by ignoring the birthday paradox, so the actual collision probability could have been much higher. This is a very small fix, so it should not break anything :-)

@ZyX-I
Copy link
Contributor

ZyX-I commented Aug 20, 2018

It should actually be %self there (preferably without any subshells if possible: 'foo'%self does not work), like in bash bindings which have $$ there: that should have exactly zero probability of having two shells on one machine with same client ID in most common setups (I do know that there are PID namespaces in linux, but normally you would not have separate PID namespace and still share access to the location of socket: feature is used primary for containers). Not sure why I did not use that earlier, perhaps this was introduced in later versions? (Or, at least, was error message pointing to %self when trying to use $$.)

@simonfxr
Copy link
Author

Okay, this is indeed even simpler and more elegant! The original comment in the code just got me thinking.

@ZyX-I
Copy link
Contributor

ZyX-I commented Aug 20, 2018

%self may be simpler, but I am really not sure whether or not it needs to come with a version check.

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.

None yet

2 participants