Conversation
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Generate shell rcfile candidates lazily and consume them directly during installation, removal, and pre-install messages. Share first-candidate selection where applicable and preserve the existing Bash, Zsh, and Tcsh selection rules.
95180d2 to
bd2bc8f
Compare
There was a problem hiding this comment.
What is the motivation for this? It doesn't obviously seem like an improvement to me. It is less code which is nice, but not if it makes the code harder to understand. And exchanging a Vec allocation for Box<dyn> allocation hardly seems like a substantial improvement.
| let host_profiles = config_dir | ||
| .into_iter() | ||
| .flat_map(|dir| dir.read_dir().into_iter().flatten()) | ||
| .filter_map(Result::ok) | ||
| .map(|entry| entry.path()) | ||
| .filter(|path| path.is_file() && path.ends_with("_profile.ps1")); |
There was a problem hiding this comment.
This doesn't seem like an improvement.
|
Firstly I think Secondly even if we use Conclusion: I'm inclined to closing this PR for now, but it might be worth another try when generator expressions are live. @djc Would you agree? |
|
@djc Aside: I think generator expressions will also allow us to avoid the use of |
|
I agree that this is not an improvement. I don't like describing it as "imperative" vs "functional", perhaps because "imperative" to me sounds like "old-school" and "functional" has a more positive connotation. In my mind we want to write code that is easy to understand. Using combinators with (ideally short) callbacks can sometimes be a good way to do that, but IMO are often overused by (usually less experienced) Rust engineers. Having code that is concise is a good goal, but sometimes code that is a little more verbose can be easier to follow, and in those cases I prefer the latter approach. Some of this is subjective, but a lot of it is not. |
4 participants
Replace current
rc_candidateslogic of allocate upon every candidate as a iter for consuming lazily.