Process cleanup #805
Process cleanup #805
Conversation
Can fake once, for_scope, or with a given list.
| Scope(T), | ||
| With(usize, Rc<Vec<T>>), | ||
| None, | ||
| Invalid, |
There was a problem hiding this comment.
Would Exhausted or Used or Consumed be a better name?
(The code here is all quite sophisticated by my Rust standards so I'm really just trying to understand the PR and may not have great suggestions.)
| } | ||
| pub fn get() -> Option<Vec<String>> { | ||
| FAKE_ARGS.with(|a| { | ||
| let old_value = a.replace_with(|old_value| match old_value { |
There was a problem hiding this comment.
would let new_value = a.replace_with... be a better name?
| calling_process_cmdline(ProcInfo::new(), blame::guess_git_blame_filename_extension); | ||
| } | ||
| calling_process_cmdline(ProcInfo::new(), guess_git_blame_filename_extension); | ||
| } |
There was a problem hiding this comment.
I think that this module is getting to the point where it would be easier to understand if we separated the abstract machinery (e.g. ProcessInterface, calling_process_cmdline , FakeParentArgs) from the concrete clients (determine_calling_process, git_blame_filename_extension). E.g. I feel that this test is starting to mix up testing git blame and FakeParentArgs logic. Happy for that to be done in subsequent PRs though.
| where_ | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
These interior mutability pointer tricks are new to me, very nice! I'd heard of them while reading Blandy & Orendorff but hadn't tried to understand code using them before. Your comment above was sufficient though to explain what was going on, if taken slowly.
There was a problem hiding this comment.
Great, and very educational. I made couple of comments -- let me know if you want to change anything or if I should merge first.
#807 is branched off this branch, although not in any meaningful way yet.
Git.exe)once,for_scope, orwith