Open WaffleLapkin opened 1 year ago
Yeah, that's tricky, some thoughts:
In general, I am not quite happy with push_dir API,
{
let _dir = sh.push_dir("./bla")
...
}
looks quite noisy at the call-site
:thinking:
I wonder if the solution here is to be dumber, rather than to be smarter?
impl Shell {
fn push_dir(&self, dir) -> Shell {
let mut new_sh = self.clone();
new_sh.dir = dir;
new_sh
}
}
At the time of writing this, xshell (
v0.2.3
) uses interiour mutability inside theShell
: https://github.com/matklad/xshell/blob/af75dd49c3bb78887aa29626b1db0b9d0a202946/src/lib.rs#L381-L384To me, this seems unnecessary & error prone.
The path value is mutated in
Shell::change_dir
(it could just as well accept&mut self
I don't immediately see the need for IM here)Shell::set_var
(again, could accept&mut self
)PushDir::new
/PushDir::drop
andPushEnv::new
/PushEnv::drop
Push*
guards are a bit more tricky to re-design, but they are also IMO the most error prone part — guards being disconnected from the object they are guarding seems fragile (although I can't come up with a really bad example, so maybe I'm imagining...).Instead I think it would be better and more "rusty" to either
&mut Shell
inside ofPush*
and implementDerefMut<Target = Shell>
— this still feels a little bit weird, because you can still mutate the guarded object, but at least it uses no interiour mutabilityPush*
, such that they don't even need to mutateShell
(possibly use a trait such that you can nestPush*
es) — IMO the nicer approach, but requires more design workIn both cases the way you use
push_dir
/push_env
changes to be more akin to mutexes for example:Either way this is just something that came to my mind while I was using
xshell
. I want to design/implement those API changes, my question thus is: would you like for these changes to be upstreamed, or should I create a fork? :)