microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.78k stars 8.34k forks source link

Command history not saved when tab is closed on msys64 #17856

Closed alshdavid closed 2 months ago

alshdavid commented 2 months ago

Windows Terminal version

1.20.11781.0

Windows build number

10.0.19045.0

Other Software

After setting up msys64 and adding it to Windows Terminal, closing the tab does not store the command history.

However, if I run exit rather than closing the tab/window, the history is saved. The history is also saved when closing the window when using the classic Windows command prompt.

This comment explains that Windows Terminal is sending the wrong signal to msys64.

Steps to reproduce

Install msys64 and add the profile to Windows Terminal

https://www.msys2.org/docs/terminals/

{
  "guid": "{55355a23-abac-48c2-b092-70c8cfc95684}",
  "hidden": false,
  "name": "zsh",
  "icon": "C:\\Users\\alshdavid\\.local\\msys64\\msys2.ico",
  "closeOnExit": "always",
  "commandline": "C:\\Users\\alshdavid\\.local\\msys64\\msys2_shell.cmd -defterm -here -no-start -mingw64 -shell zsh",
  "startingDirectory": "C:/Users/%USERNAME%",
  "environment": {
    "MSYS": "winsymlinks:nativestrict",
    "CHERE_INVOKING": "1",
    "MSYSTEM": "MINGW64"
  }
}

Expected Behavior

History is saved

Actual Behavior

History is not saved

https://github.com/user-attachments/assets/55ed4b7b-716c-4fbb-a11f-376ff9c1ad61

DHowett commented 2 months ago

@mominshaikhdevs please stop pinging random engineers on the team. We all have our notifications configured to make sure that we receive the things that are relevant to us, and we have a teamwide triage session every week. We will almost certainly see something, without you having to loop one of us in explicitly.

If you're seeing an issue that needs a timely response (and I do mean timely, like "people are going to be hurt"), Microsoft offers a number of paid support options.

lhecker commented 2 months ago

I just tested this with the following code:

#define WIN32_LEAN_AND_MEAN
#include <Windows.h>
#include <cstdio>

static wchar_t path[MAX_PATH];

int main() {
    ExpandEnvironmentStringsW(L"%USERPROFILE%\\Downloads\\status.txt", &path[0], ARRAYSIZE(path));
    wprintf(L"Writing to: %s\nExit now...\n", &path[0]);

    SetConsoleCtrlHandler(
        [](DWORD event) -> BOOL {
            const char* msg = nullptr;
            switch (event) {
            case CTRL_C_EVENT:
                msg = "CTRL_C_EVENT";
                break;
            case CTRL_BREAK_EVENT:
                msg = "CTRL_BREAK_EVENT";
                break;
            case CTRL_CLOSE_EVENT:
                msg = "CTRL_CLOSE_EVENT";
                break;
            case CTRL_LOGOFF_EVENT:
                msg = "CTRL_LOGOFF_EVENT";
                break;
            case CTRL_SHUTDOWN_EVENT:
                msg = "CTRL_SHUTDOWN_EVENT";
                break;
            default:
                msg = "default";
                break;
            }

            const auto f = CreateFileW(&path[0], GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
            WriteFile(f, msg, (DWORD)strlen(msg), nullptr, nullptr);
            CloseHandle(f);
            return FALSE;
        },
        TRUE
    );

    Sleep(INFINITE);
    return 0;
}

It results in CTRL_CLOSE_EVENT, which is the expected event. If a SIGHUP event is not generated, then I believe that's a bug in Cygwin, because it implements a POSIX environment on top of Windows. To me that means that it's Cygwin's job to translate SetConsoleCtrlHandler events to signal events.

FYI signal events like SIGBREAK and SIGHUP don't exist on Windows. Their support inside the CRT (C runtime = C library) is faked, based on SetConsoleCtrlHandler. SIGBREAK is simply any ctrl event that isn't a CTRL_C_EVENT which is just completely wrong. But the console implementation in the CRT is overall bad for many reasons, so it shouldn't be too unexpected.

As such, I'll be closing the issue for now. Let me know if you have any questions or if you think that I made a mistake.

mominshaikhdevs commented 2 months ago

please stop pinging random engineers on the team.

@DHowett AFAIR I mentioned that particular engineer in github issues just twice in total. the "two pingings ever" were not done with the intention to either indicate that the engineers are careless or to annoy them. See, one of the most crucial repo for modern windows application developers that is WinUI and their ever rising and piling up github issues-and-their-fixes rates suggest that they are understaffed. I assumed the same here.

We will almost certainly see something, without you having to loop one of us in explicitly.

and my assumption was wrong. IDKT. good to know. I hope for the best for windows terminal that can finally rival the Unix Clones ones. good luck.

DHowett commented 2 months ago

@mominshaikhdevs Sorry for singling you out, and misremembering how many times you'd done that. Thanks for keeping us honest. I know that some of our sister teams have not been very good about that. 🙂