-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Optimization of tilde expansion #9709
Conversation
Especially when `expand_tilde` is claled on a path that doesn't contain a tilde.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally try to avoid refactors unless the new code is a big improvement. Avoiding the allocation here is nice-to-have but not worth a medium/large change. With some small tweaks we can get the diff to be very small though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
* Use next and avoid a redundant prefix strip * Avoid allocations Especially when `expand_tilde` is claled on a path that doesn't contain a tilde. * Add a test * Use Into<Cow<…>> * Put the expand_tilde test at the end of the file * Remove unused importsw
* Use next and avoid a redundant prefix strip * Avoid allocations Especially when `expand_tilde` is claled on a path that doesn't contain a tilde. * Add a test * Use Into<Cow<…>> * Put the expand_tilde test at the end of the file * Remove unused importsw
* Use next and avoid a redundant prefix strip * Avoid allocations Especially when `expand_tilde` is claled on a path that doesn't contain a tilde. * Add a test * Use Into<Cow<…>> * Put the expand_tilde test at the end of the file * Remove unused importsw
* Use next and avoid a redundant prefix strip * Avoid allocations Especially when `expand_tilde` is claled on a path that doesn't contain a tilde. * Add a test * Use Into<Cow<…>> * Put the expand_tilde test at the end of the file * Remove unused importsw
* Use next and avoid a redundant prefix strip * Avoid allocations Especially when `expand_tilde` is claled on a path that doesn't contain a tilde. * Add a test * Use Into<Cow<…>> * Put the expand_tilde test at the end of the file * Remove unused importsw
* Use next and avoid a redundant prefix strip * Avoid allocations Especially when `expand_tilde` is claled on a path that doesn't contain a tilde. * Add a test * Use Into<Cow<…>> * Put the expand_tilde test at the end of the file * Remove unused importsw
I was reading some code to fix #5706 but then I found that
expand_tilde
allocates even when it is not necessary. Therefore, I added some 🐄Please read the commit messages and ask me if anything is not clear.
This is my first contribution, so please tell me if I missed some contribution guide point 😇