Change Details

This came again to my mind, because of the discussion in https://invent.kde.org/plasma/plasma-desktop/-/merge_requests/38#note_131833. Currently we store the exec line when selecting a default terminal, this has the following problems: - We don't know the service it belongs to - We don't update the value of the desktop file of the terminal program changes(lets say through an update) - We don't use the desktop exec parser Also we have a few other issues: - The value is often directly read from the desktop file -- Not all terminals get supported with the correct command line parameters, especially if we want to support more external terminals we don't want to have the logic in multiple places. - We don't call `KToolInvokcation::invokeTerminal` which is supposed to be the central place for invoking terminals and stuff. This method does the command creation and also supports windows terminals. Because `konsole` is hardcoded as the default value for the config read accesses other than in `KToolInvokcation::invokeTerminal` this won't work on windows(also the KCM does not exist there, obviously ;) ) My suggestion is: - Adding a simple static method to KToolInvocation which will just read the TerminalApplication. Then all the hardcoded config read accesses get replaced. - Also save the service path of the terminal in the KCM -- Use the service path to get the exec line in the newly created method mentioned above. The old value can be used as a fallback. This way we also ensure compatibility. - Create a new method which will create the string+args that we can put into our QProcess, this will take the command that should be executed in the terminal as a parameter. In case we don't want to launch a detached process(like `KToolInvokcation::invokeTerminal` does) we can use this and avoid hacks/duplicate code. After that we are far more flexible and robust with our default terminal application config and can also fix the cases where users are unhappy that their config gets ignored :)

This came again to my mind, because of the discussion in https://invent.kde.org/plasma/plasma-desktop/-/merge_requests/38#note_131833. Currently we store the exec line when selecting a default terminal, this has the following problems: - We don't know the service it belongs to - We don't update the value of the desktop file of the terminal program changes(lets say through an update) - We don't use the desktop exec parser Also we have a few other issues: - The value is often directly read from the desktop file -- Not all terminals get supported with the correct command line parameters, especially if we want to support more external terminals we don't want to have the logic in multiple places. - We don't call `KToolInvokcation::invokeTerminal` which is supposed to be the central place for invoking terminals and stuff. This method does the command creation and also supports windows terminals. Because `konsole` is hardcoded as the default value for the config read accesses this won't work on windows(also the KCM does not exist there, obviously ;) ) My suggestion is: - Adding a simple static method to KToolInvocation which will just read the TerminalApplication. Then all the hardcoded config read accesses get replaced. - Also save the service path of the terminal in the KCM -- Use the service path to get the exec line in the newly created method mentioned above. The old value can be used as a fallback. This way we also ensure compatibility. - Create a new method which will create the string+args that we can put into our QProcess, this will take the command that should be executed in the terminal as a parameter. In case we don't want to launch a detached process(like `KToolInvokcation::invokeTerminal` does) we can use this and avoid hacks/duplicate code. Then we can also refactor and reuse the KToolInvocation logic for windows. After that we are far more flexible and robust with our default terminal application config and can also fix the cases where users are unhappy that their config gets ignored :)

This came again to my mind, because of the discussion in https://invent.kde.org/plasma/plasma-desktop/-/merge_requests/38#note_131833. Currently we store the exec line when selecting a default terminal, this has the following problems: - We don't know the service it belongs to - We don't update the value of the desktop file of the terminal program changes(lets say through an update) - We don't use the desktop exec parser Also we have a few other issues: - The value is often directly read from the desktop file -- Not all terminals get supported with the correct command line parameters, especially if we want to support more external terminals we don't want to have the logic in multiple places. - We don't call `KToolInvokcation::invokeTerminal` which is supposed to be the central place for invoking terminals and stuff. This method does the command creation and also supports windows terminals. Because `konsole` is hardcoded as the default value for the config read accesses other than in `KToolInvokcation::invokeTerminal` this won't work on windows(also the KCM does not exist there, obviously ;) ) My suggestion is: - Adding a simple static method to KToolInvocation which will just read the TerminalApplication. Then all the hardcoded config read accesses get replaced. - Also save the service path of the terminal in the KCM -- Use the service path to get the exec line in the newly created method mentioned above. The old value can be used as a fallback. This way we also ensure compatibility. - Create a new method which will create the string+args that we can put into our QProcess, this will take the command that should be executed in the terminal as a parameter. In case we don't want to launch a detached process(like `KToolInvokcation::invokeTerminal` does) we can use this and avoid hacks/duplicate code. Then we can also refactor and reuse the KToolInvocation logic for windows. After that we are far more flexible and robust with our default terminal application config and can also fix the cases where users are unhappy that their config gets ignored :)