jamesob / desk

A lightweight workspace manager for the shell
MIT License
2.54k stars 111 forks source link

Use "exec" to start the shell #67

Closed magicant closed 8 years ago

magicant commented 8 years ago

By using "exec", the shell instance that is running the desk script is replaced by the new child shell. This will enable the original shell that invoked desk to take care of the child if the user suspends the child by typing suspend inside the desk.

aratno commented 8 years ago

I think this should be the default, and a desk exit command should be supplied instead of expecting the newly spawned shell process to be closed. It seems more intuitive to the user, and would prevent desk nesting, which doesn't seem desirable.

jamesob commented 8 years ago

Thanks for the PR!

When testing locally, this doesn't seem to prevent desk nesting (which I agree can be annoying):

(venv) [Tue 24 22:56] job/code/desk pr-67* 
 $ desk go scruffy 

(venv) [Tue 24 22:56] job/counsyl/scruffy master scruffy
 $ desk go charon 

(venv) [Tue 24 22:57] job/counsyl/charon master* charon
 $ exit

(venv) [Tue 24 22:57] job/counsyl/scruffy master scruffy
 $ exit

I haven't looked into why this might be yet... any ideas?

magicant commented 8 years ago

Thanks for testing my PR!

There are 3 shell instances involved here:

  1. The original interactive shell that invoked the desk command
  2. The shell that is running the desk script
  3. The child interactive shell that is invoked by the script to load desk-specific configuration

This PR is to remove shell 2 because it is just unnecessary once shell 3 has been started. Since shell 1 is still alive, you can still exit from shell 3 and go back to shell 1.

Removing shell 1 as well would be a difficult job, as suggested in #36. I don't have a good idea on it.

jamesob commented 8 years ago

Sounds good to me. This looks like a great change -- thanks for contributing. Let's merge!