lirios / terminal

:rocket: Terminal
GNU General Public License v3.0
29 stars 3 forks source link

Enforce a default shell when 'shell-program' setting is empty #32

Closed itay-grudev closed 6 years ago

itay-grudev commented 6 years ago

If you don't want to bother checking the default shell and running it, how about simply changing the default shell command setting to:

$SHELL
itay-grudev commented 6 years ago

You're already doing it correctly, but the default hard-coded value in the settings overrides this: https://github.com/lirios/terminal/blob/bd46c117b647faeeb23009571077376bf23b4b92/imports/terminal/ksession.cpp#L72-L74

So it might be better to leave it empty in the default settings and make sure the defaults from the Konsole session are used:

https://github.com/lirios/terminal/blob/18d121a2c79350a1337d4a5a6d7078745af8042b/data/io.liri.Terminal.gschema.xml#L14-L20

plfiorini commented 6 years ago

The code in ksession.cpp that you pasted sets the default shell to whatever $SHELL is or, if it's empty, to /bin/bash. Then later, after the initial session creation, the shell program is changed to whatever is configured in the settings.

The configured shell program is then used for new tabs and windows. Terminal can't change the shell program currently in use in the existing tabs: it would have to kill the current shell process and start a new one loosing your work.

itay-grudev commented 6 years ago

Right now if you leave the shell-program param empty, the app becomes unusable. Yet if you simply put $SHELL it works ok. So I propose two changes:

  1. Remove the override if the shell-program param is not set (empty).
  2. Change the default to either $SHELL or leave it empty.

That should be relatively easy to do.

plfiorini commented 6 years ago

Got it, I didn't test with shell-program empty. The logic in ksession.cpp should be moved to the shell program setter so that $SHELL is used instead of an empty string.