You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 and PushEnv::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
Store a &mut Shell inside of Push* and implement DerefMut<Target = Shell> — this still feels a little bit weird, because you can still mutate the guarded object, but at least it uses no interiour mutability
Implement all the methods on Push*, such that they don't even need to mutate Shell (possibly use a trait such that you can nest Push*es) — IMO the nicer approach, but requires more design work
In 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? :)
The text was updated successfully, but these errors were encountered:
At the time of writing this, xshell (
v0.2.3
) uses interiour mutability inside theShell
:xshell/src/lib.rs
Lines 381 to 384 in af75dd4
To 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? :)The text was updated successfully, but these errors were encountered: