GNOME terminal not always present when using "Open in Shell" for first time

jmejia32 commented 4 years ago

Describe the bug

Can't open repository folder in terminal on Deepin DE (also fails on Deepin OS distro)

Version & OS

Github Desktop Version: 2.5.4-linux1 OS: Arch Linux x64 - kernel version: 5.8.8-arch1-1

Steps to reproduce the behavior

Open a local repository
Go to Repository menu
Click on 'Open in GNOME(?) Terminal' menu item (no GNOME installed)
Bang! Error message shown

Expected behavior

System's default Virtual Terminal Emulator (Deepin Terminal, in my case) started on repository folder

Actual behavior

This message:

Screenshots

Compared to referenced issue #167 , Deepin Terminal finally appears in shell menu

Logs

2020-09-16T17:11:11.064Z - info: [ui] Background fetch for 10 repositories took 1.321sec
2020-09-16T17:17:31.709Z - info: [ui] [AppStore] loading 10 repositories from store
2020-09-16T17:17:31.709Z - info: [ui] [AppStore] found account: jmejia32 (Javier Mejía Estrada)
2020-09-16T17:17:32.253Z - info: [ui] launching: 2.5.4-linux1 (Linux 5.8.8-arch1-1)
2020-09-16T17:17:32.254Z - info: [ui] execPath: '/opt/github-desktop/github-desktop'
2020-09-16T17:19:21.527Z - info: [ui] [AppStore] loading 9 repositories from store
2020-09-16T17:19:21.527Z - info: [ui] [AppStore] found account: jmejia32 (Javier Mejía Estrada)
2020-09-16T17:19:22.070Z - info: [ui] launching: 2.5.4-linux1 (Linux 5.8.8-arch1-1)
2020-09-16T17:19:22.071Z - info: [ui] execPath: '/opt/github-desktop/github-desktop'

shiftkey commented 3 years ago

@jmejia32 apologies for the delay in replying.

Is this issue that "GNOME Terminal" is shown despite not having it installed? Or is the issue that Deepin Terminal doesn't work?

jmejia32 commented 3 years ago

@shiftkey "GNOME Terminal" is shown regardless it is installed or not, and Deepin Terminal works so far. Other applications call it as default terminal emulator.

jmejia32 commented 3 years ago

PD: Currently, I use Debian distro (11 - bullseye - testing branch) + Cinnamon DE because IT security requirements in office, but I have a dual boot of both systems and the issue still exists under that environment.

It is not related to Deepin only.

shiftkey commented 3 years ago

"GNOME Terminal" is shown regardless it is installed or not, and Deepin Terminal works so far.

There's some history in here that I'm hazy on, but on initial launch we don't have a selected terminal and the placeholder we use is a best guess. This works fine on Windows and macOS because we know what shells are available by default, but clearly that won't work for Linux due to the different permutations of configurations available.

It'd be nice to be able to probe on this first launch and see what shells are installed, and choose the first valid shell, but I'm not sure if we have a suitable hook to do that.

Daniel-McCarthy commented 2 years ago

Hi all, I am currently experiencing this issue as well on KDE Manjaro with GitHub Desktop installed from the AUR. I have been troubleshooting this issue toIday as I noticed "GNOME Terminal" appeared instead of "Konsole" and gave this error when clicking it.

Interestingly, when I built GitHub Desktop from source and ran it, I found that the selectedShell variable actually did contain "Konsole" correctly, and that in the Integrations settings, Konsole was already selected, but showed "GNOME Terminal" in the labels anyways and attempted to launch GNOME Terminal instead.

I was able to see that when debugging app.tsx was passing down "GNOME Terminal" as a prop, and continued to do so even after saving the shell selection again. I tried placing a break point at the call to updatePreferredAppMenuItemLabels in app-store.tsx and it continued to show the wrong terminal. I saved the shell selection again with the breakpoint placed, and it showed "Konsole" finally, and the labels were successfully updated & I could launch it. It even persists after closing and launching the program.

In tried in my source-compiled Desktop instance but placed the breakpoint at _setShell in app-store.tsx and found once again it only successfully saved and the state successfully changed to Konsole when stepping through. Perhaps this is an indicator of a race condition causing the change to not save, resulting in the incorrect value to be passed down?

The TL;DR of this:

My install showed GNOME Terminal instead of Konsole for all labels and failed to launch the terminal.
The Integrations settings successfully showed Konsole as my current selected shell, despite the menu options showing the incorrect terminal. (Saving it didn't effect anything).
Only when stepping through the _setShell and _setExternalEditor methods in app-store.tsx and saving the selected shell/terminal did it successfully save, which resolved my issue even on relaunch.

I would be happy to provide more info on this to help debug this if needed. If there is a way to clear the setting to reproduce it, I would love to dig further.

Interpause commented 2 years ago

Hi all, I am currently experiencing this issue as well on KDE Manjaro with GitHub Desktop installed from the AUR. I have been troubleshooting this issue toIday as I noticed "GNOME Terminal" appeared instead of "Konsole" and gave this error when clicking it.

Interestingly, when I built GitHub Desktop from source and ran it, I found that the selectedShell variable actually did contain "Konsole" correctly, and that in the Integrations settings, Konsole was already selected, but showed "GNOME Terminal" in the labels anyways and attempted to launch GNOME Terminal instead.

I was able to see that when debugging app.tsx was passing down "GNOME Terminal" as a prop, and continued to do so even after saving the shell selection again. I tried placing a break point at the call to updatePreferredAppMenuItemLabels in app-store.tsx and it continued to show the wrong terminal. I saved the shell selection again with the breakpoint placed, and it showed "Konsole" finally, and the labels were successfully updated & I could launch it. It even persists after closing and launching the program.

In tried in my source-compiled Desktop instance but placed the breakpoint at _setShell in app-store.tsx and found once again it only successfully saved and the state successfully changed to Konsole when stepping through. Perhaps this is an indicator of a race condition causing the change to not save, resulting in the incorrect value to be passed down?

The TL;DR of this:

My install showed GNOME Terminal instead of Konsole for all labels and failed to launch the terminal.

The Integrations settings successfully showed Konsole as my current selected shell, despite the menu options showing the incorrect terminal. (Saving it didn't effect anything).

Only when stepping through the _setShell and _setExternalEditor methods in app-store.tsx and saving the selected shell/terminal did it successfully save, which resolved my issue even on relaunch.

I would be happy to provide more info on this to help debug this if needed. If there is a way to clear the setting to reproduce it, I would love to dig further.

If you don't want to patch it yourself, using the dev tools and setting breakpoints following the above post while changing the terminal emulator works without having to modify anything.

Etaash-mathamsetty commented 2 years ago

these issues are caused by gnome terminal being the default in the code, a simple way to fix this is to install a second different terminal, switch to it, then switch to the terminal you want to use, and then uninstall the second terminal. This worked well for me as shown in pr #700

aminya commented 1 year ago

I have the same problem. On Kubuntu, the default terminal is the Konsole, but the "Open in " UI still shows the GNOME terminal, which doesn't work; giving a path error. KDE doesn't come with a GNOME terminal.

The workaround seems to be going into the setting and manually setting the integrations to Konsole.

I think the default terminal detection logic should be improved. Checking more famous terminal apps by default and skipping those that are not installed.

khawkins98 commented 1 year ago

Thanks to @Daniel-McCarthy for the tip on working around this.

A few screenshots for posterity of my fix on KDE Manjaro.

Stepping through _setShell

Changing the string to "Konsole" (in my case)

Then resume script execution and presto

shiftkey / desktop