Terminal command doesn't work if x-terminal-emulator is unavailable

[OPNA2608]

With version 0.1.0 on a non-Debian distro, I'm unable to use the Terminal command:

[2024-02-23 09:05:03.714707] < -warning- > miral: Failed to execute client ("x-terminal-emulator") error: No such file or directory
!!! Fatal signal received. Attempting cleanup, but deadlock may occur.
Mir fatal error: Unsupported attempt to continue after a fatal signal: SIGSEGV
!!! Fatal signal received. Attempting cleanup, but deadlock may occur.
Mir fatal error: Unsupported attempt to continue after a fatal signal: SIGABRT
[2024-02-23 09:05:04.737998] < - debug - > mirserver: Handling Child exited from pid=4895

x-terminal-emulator doesn't exist on this distro, and none of the other POSSIBLE_TERMINALS entries get attempted.

[AlanGriffiths]

@mattkae

            for (auto const& terminal : POSSIBLE_TERMINALS)
            {
                if (external_client_launcher.launch({terminal}) > 0)
                    break;
            }

Doesn't do what you think. external_client_launcher.launch() does a fork()/exec() and returns the pid of the forked process. The fork() will always succeed, even if the exec() doesn't.

The approach I took with Miriway was to allow the user to configure the terminal command (and provide a fallback to a script that tries to identify an installed terminal).

[mattkae]

Doesn't do what you think. external_client_launcher.launch() does a fork()/exec() and returns the pid of the forked process. The fork() will always succeed, even if the exec() doesn't.

Oh that's unfortunate

The approach I took with Miriway was to allow the user to configure the terminal command (and provide a fallback to a script that tries to identify an installed terminal).

I think that I will follow this same approach

[mattkae]

Hi agan @OPNA2608 👋 Would you give this PR try when you get a chance: #40

It should work in both a snap and non-snap context. You should be able to install the AMD snap from this page: https://github.com/mattkae/miracle-wm/actions/runs/8038897957?pr=40. I'll add the fix to a release/0.1.1 later this week if all is good.

Thanks for finding the bug!

[mattkae]

Also, I am testing out #39 now. I will cherry-pick it into release/0.1.1 alongside the terminal fix once approved 👍