#1004 Propose only available terminals

[Michel Le Bihan]

Hello,

With this patch in environment settings under terminal... only available (present in PATH) terminals will be proposed as an option.

I'm attatching a diff not a patch because I know that fileInPath rather shouldn't be in environmentsettingsdlg.cpp, but I have no clue where I should put it.

[Teodor Petrov]

Why is this useful?
I don't like this change. What happens if you have a terminal in non-default location and not in PATH? Or your favourite terminal is not installed yet, because you've reinstalled, but have forgotten to install it yet? Existence of a file doesn't mean that it can be executed successfully.

Something which would be actually useful would be to select the default terminal somehow.
Debian/ubuntu have the alternatives feature, but I've not researched yet how to do it.

[Michel Le Bihan]

What happens if you have a terminal in non-default location and not in PATH?

Will C::B be able to launch it?

Or your favourite terminal is not installed yet, because you've reinstalled, but have forgotten to install it yet?

That will only help them notice that they forgot to install that terminal.

Existence of a file doesn't mean that it can be executed successfully.

True. The only way to test that that I know would require flashing the terminal window. Even if it can't be executed, I think that it should be show as an option because this might mean that the user installed a terminal and plans to use it in C::B, but for whatever reason it got broken.

Something which would be actually useful would be to select the default terminal somehow.
Debian/ubuntu have the alternatives feature, but I've not researched yet how to do it.

I also think that it would be very useful. x-terminal-emulator could be used. gnome-terminal and xfce4-terminal seem to have a wrapper that makes them accept the same parameters as xterm.

[Teodor Petrov]

You haven't answered why do you think this change is useful. What actual problem are you trying to solve?

The idea of the presets is to give the users a template. So if I have gnome-terminal in non-default location I can just select the preset modify this a bit and be done with it. If you remove it from the list then I have to make up the whole command line. This use case is not very probable, but who knows.

[Michel Le Bihan]

You haven't answered why do you think this change is useful. What actual problem are you trying to solve?

Users chosing a terminal that is in fact not available and getting confused with their terminal not working. Maybe this doesn't make much sense. I will implement usage of x-terminal-emulator instead.

[Teodor Petrov]

It makes sense, but I don't think it is the correct solution to the problem.
One solution might be to make the terminal selection box to be red if the terminal check fails. Similarly to the box for selecting the debugger executable in Settings -> Debugger.

[Baldhari Rajak]

+bool fileInPath(const wxString &filename)
+{
+ wxString pathValues;
+ wxGetEnv(_T("PATH"), &pathValues);
+ if (pathValues.empty())
+ return false;
+
+ const wxString &sep = platform::windows ? _T(";") : _T(":");
+ wxChar pathSep = wxFileName::GetPathSeparator();
+ const wxArrayString &pathArray = GetArrayFromString(pathValues, sep);
+ for (size_t i = 0; i < pathArray.GetCount(); ++i)
+ {
+ if (wxFileExists(pathArray[i] + pathSep + filename))
+ }
+ return true;
+ }
+ {
+ return false;
+}
+
EnvironmentSettingsDlg::EnvironmentSettingsDlg(wxWindow parent, wxAuiDockArt art)
: m_pArt(art),
m_pImageList(nullptr)
{
if (!platform::macosx && !platform::darwin)
{
- combo->Append(wxT("gnome-terminal -t $TITLE -x "));
- combo->Append(wxT("konsole -e "));
- combo->Append(wxT("xfce4-terminal -T $TITLE -x "));
- combo->Append(wxT("terminology -M -T $TITLE -e "));
+ if (fileInPath("gnome-terminal"))
+ combo->Append(wxT("gnome-terminal -t $TITLE -x "));
+ if (fileInPath("konsole"))
+ combo->Append(wxT("konsole -e "));
+ if (fileInPath("xfce4-terminal"))
+ combo->Append(wxT("xfce4-terminal -T $TITLE -x "));
+ if (fileInPath("terminology"))
+ combo->Append(wxT("terminology -M -T $TITLE -e "));
}
wxString terminal = cfg->Read(wxT("/console_terminal"), DEFAULT_CONSOLE_TERM);
if (!combo->SetStringSelection(terminal))