Skip to content

Fix Open Terminal - #225

Merged
todi1856 merged 4 commits into
masterfrom
fix-open-terminal
Sep 30, 2026
Merged

todi1856 merged 4 commits into
masterfrom
fix-open-terminal

Conversation

@todi1856

@todi1856 todi1856 commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Yamato CI

This PR fixes two issues:

Windows: no terminal window opened on CoreCLR

How it broke
OpenTerminal started cmd.exe with new ProcessStartInfo("cmd.exe") { WorkingDirectory = ... } and left UseShellExecute at its default. The default is different on each runtime:

  • Mono / .NET Framework: UseShellExecute defaults to true. The process goes through ShellExecuteEx, which gives cmd.exe its own console window.
  • CoreCLR (.NET): UseShellExecute defaults to false. The process is started directly through CreateProcess and inherits the Editor's standard handles. The Editor is a GUI app and has no usable console, so cmd.exe gets no interactive console and no terminal window shows up.

Fix
Set UseShellExecute = true explicitly. This brings back the old Mono behaviour on both runtimes.

Linux: OpenTerminal was not implemented

OpenTerminal had no RuntimePlatform.LinuxEditor case, so on Linux it fell through to default and threw Don't know how to open terminal on LinuxEditor.

Claude provided default implementation so this works now just fine

2026-09-25.16-49-46.mp4

@u-pr u-pr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Harness Review

Needs changes

The change adds Linux terminal launching and adjusts Windows launching, but the Linux path mishandles some valid launcher settings and SDK directory names.

Reviewed commit c7443a9

🤖 Helpful? 👍/👎

Comment thread com.unity.mobile.android-logcat/Editor/AndroidLogcatUtilities.cs Outdated
Comment thread com.unity.mobile.android-logcat/Editor/AndroidLogcatUtilities.cs Outdated
@todi1856
todi1856 requested a review from IGuscin September 25, 2026 13:56
- Split $TERMINAL into executable and arguments without invoking a shell,
  so values like 'wezterm start' work.
- Pass the working directory as separate arguments via
  ProcessStartInfo.ArgumentList instead of embedding it in a quoted
  Arguments string, so paths containing quotes are preserved.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@todi1856

Copy link
Copy Markdown
Member Author

/review

@u-pr u-pr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Harness Review

Needs changes

The change adds Linux terminal support and updates Windows launching, with one reachable failure in the Linux terminal selection path.

Reviewed commit 7b60fdd

🤖 Helpful? 👍/👎

Comment thread com.unity.mobile.android-logcat/Editor/AndroidLogcatUtilities.cs

@IGuscin IGuscin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tomas verified that open terminal works properly, both on Win and Linux

@todi1856
todi1856 merged commit 63f7053 into master Sep 30, 2026
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants