Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows console history via F7 does not work in terminal mode #1097

Closed
mariusgreuel opened this issue Sep 6, 2022 · 10 comments
Closed

Windows console history via F7 does not work in terminal mode #1097

mariusgreuel opened this issue Sep 6, 2022 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@mariusgreuel
Copy link
Contributor

When I use the console shortcut F7 in terminal mode to bring up the history, the history shows, but when selecting an item, it seems only the first character is being read, and not the entire command. Other editing commands such as cursor up/down, F3, etc. seem to work.

@mariusgreuel mariusgreuel added the bug Something isn't working label Sep 6, 2022
@mariusgreuel mariusgreuel self-assigned this Sep 6, 2022
@mariusgreuel
Copy link
Contributor Author

OK, something totally weird is happening upon F7: Using the avrdude code with

char input[256] = { 0 };
char* returns = fgets(input, sizeof(input), stdin);

and selecting a string of '123' from the history gives me a input buffer with

  Name Value Type
  [0] 49 '1' char
  [1] 0 '\0' char
  [2] 0 '\0' char
  [3] 0 '\0' char
  [4] 50 '2' char
  [5] 0 '\0' char
  [6] 0 '\0' char
  [7] 0 '\0' char
  [8] 51 '3' char
  [9] 0 '\0' char
  [10] 10 '\n' char
  [11] 0 '\0' char

@mariusgreuel
Copy link
Contributor Author

Hm, did some playing and it seems ReadFile and ReadConsoleA is broken on Windows 10. When I use the Unicode API ReadConsoleW, F7 works.

The glory details...
#include <stdio.h>
#include <string.h>
#include <malloc.h>
#include <windows.h>

#define USE_UNICODE 1

char* terminal_get_input(const char* prompt)
{
    printf("%s", prompt);
#if USE_UNICODE
    DWORD dwCharsRead = 0;
    WCHAR wcBuffer[256] = { 0 };
    if (ReadConsoleW(
        GetStdHandle(STD_INPUT_HANDLE),
        wcBuffer,
        ARRAYSIZE(wcBuffer),
        &dwCharsRead,
        NULL))
    {
        CHAR acBuffer[256] = { 0 };
        int nChars = WideCharToMultiByte(
            GetConsoleCP(),
            0,
            wcBuffer,
            dwCharsRead,
            acBuffer,
            ARRAYSIZE(acBuffer) - 1,
            NULL,
            NULL);
        if (nChars > 0)
        {
            acBuffer[nChars] = 0;
            return _strdup(acBuffer);
        }
    }

    return NULL;
#else
    DWORD dwCharsRead = 0;
    CHAR acBuffer[256] = { 0 };
    if (ReadConsoleA(
        GetStdHandle(STD_INPUT_HANDLE),
        acBuffer,
        ARRAYSIZE(acBuffer) - 1,
        &dwCharsRead,
        NULL))
    {
        if (dwCharsRead > 0)
        {
            acBuffer[dwCharsRead] = 0;
            return _strdup(acBuffer);
        }
    }

    return NULL;
#endif

#if OLD_IMPLEMENTATION
    char input[256] = { 0 };
    char* returns = fgets(input, sizeof(input), stdin);
    if (returns)
    {
        return _strdup(input);
    }
    else
    {
        return NULL;
    }
#endif
}

int main()
{
    while (1)
    {
        char* buffer = terminal_get_input("avrdude> ");
        printf("Got: '%s'\n", buffer);
        free(buffer);
    }
}

@mcuee
Copy link
Collaborator

mcuee commented Sep 7, 2022

Yes I can reproduce on my Windows 10 Laptop (using CMD or Windows Terminal). After selecting the option, only the first character is read.

avrdude> dump eeprom

>>> d
Usage: d <memory> <addr> <len>
       d <memory> <addr> ...
       d <memory> <addr>
       d <memory> ...
       d <memory>

F7_avrdude_cmd

@mcuee
Copy link
Collaborator

mcuee commented Dec 31, 2022

I think this can be a low priority. If we can get #1186 fixed, then I believe it does not really matter much any more.

@mcuee
Copy link
Collaborator

mcuee commented Jan 4, 2023

#1264 seems to be able to fix this issue.

@stefanrueger
Copy link
Collaborator

| something totally weird is happening upon F7

I suspect this is '123' in 32-bit unicode, not? Hence your success when using the Unicode API ReadConsoleW...

@mariusgreuel
Copy link
Contributor Author

I suspect this is '123' in 32-bit unicode, not?

No, it's random. Sometimes the first few characters are returned (i.e. as ANSI bytes), and the rest is missing (which may look like UTF32?).

I tried reading from the Unicode stream std::wcin, but it appears it is still using ReadFile and not ReadConsoleW, so this did not work. Anyway, I will not spend more time on this, as converting a stream into a line is not trivial. I submitted a bug report to Microsoft, lets see if anybody cares.

https://aka.ms/AAj3zlf

@mcuee
Copy link
Collaborator

mcuee commented Jan 5, 2023

PR #1264 seems to sort out #1097 as well.

  1. F7 will bring in the history. It is interesting that it may have the history of previous runs.
    Capture1

  2. If I click the last entry, it will present in the command line. Once I click RETURN, it will run.
    Capture2

@mcuee
Copy link
Collaborator

mcuee commented Mar 28, 2023

@mariusgreuel

Can we close this issue? At least the MSVC version now works with F7.

For MinGW version, we have #1271. Even if F7 does not work under MinGW, I would not think it is a real issue.

@mcuee
Copy link
Collaborator

mcuee commented Jun 20, 2023

I will close this issue now since it works with MSVC version.

I do not think it is a real issue for MinGW build.

@mcuee mcuee closed this as completed Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants