CMake: Include readline library for MSVC #1259
Conversation
|
Cool progress! @mariusgreuel I don't read cmake, so am useless for reviewing the changes (I looked at them and they look good, but I don't know). | The readline history is not preserved over sessions I don't think it is in Linux either. This would probably need to be specifically programmed using readline functions; whether one wanted such a thing is another question. All good on that front. | The reconnect feature does not seem to work (using manual reset) Do you mean pressing rest on the AVR-board whilst live on | The keep alive feature is not very reliable, it seems to eventually time out after some 20 to 30 seconds or so That's the only thing I would try to fix; I am sure you know how keep-alive works, so it's probably worthwhile investigating this loop and double-checking my massaging of your I am particularly pleased that this no longer needs an explicit |
|
| If you have a Windows PC at your hands, could you give this PR a try? Is there anything specific you would like me to try? Sadly no... Can you leave the terminal with ctrl-D or ctrl-Z (the EOF character for text input)? |
Just wondering whether the Windows |
|
@stefanrueger and @mariusgreuel The changes in CMake look good to me. Issue 2 is annoying. Issue 1 and Issue 3 are acceptable for me as of now, since there is the same issue under MSYS2 with GNU Readline. I have found a few issues under Windows.
|
|
The following is with MSYS2 mingw64 readline as a comparison.
|
|
@stefanrueger and @mariusgreuel So to me the main problem is with piping (can not quit properly), which is kind of annoying.
As for timeout, it does happen to MSYS2 readline version occasionally as well without hitting special modifier keys (CTRL-Shift-Alt etc). I have not reproduced the issue with this PR yet with the Optiboot 4.4 bootloader but I think it will happen sometimes. Once you hit CTRL/Shift/ALT or even CAPLOCK, I can see the keep-alive fast blinking LEDs are gone and it will cause cause timeout. |
I did a test just now, after 30 minutes it did not time out. But sometime later (not so sure when, say after about one hour), it did time out. |
|
@mcuee Thanks for the extra testing. I looked a little at the problem with time-outs yesterday, and it appears that avrdude gets stuck while calling It is a problem with the WIN32 |
|
@mariusgreuel If you have control over |
Apparently it's possible to roll it yourself: From https://tiswww.case.edu/php/chet/readline/readline.html Variable: If non-zero, Readline will call indirectly through this pointer to get a character from the input stream. By default, it is set to rl_getc, the default Readline character input function (see Character Input). In general, an application that sets rl_getc_function should consider setting rl_input_available_hook as well. |
|
To debug the pipe issue, I was thinking that the older version of readline used here (5.00) may have some issues similar to libedit, as seen in PR #1207. So I apply the following patch and it indeed sorted out the pipe issue. WIth this mod, it seems to me this PR is good enough for 7.0 release. We can fix the remaining issues later. But of course it will be even better to fix the WIN32
|
|
@mcuee Good catch. I've changed the threshold straight away, which reduces the problem set for Windows. |
mariusgreuel
force-pushed
the
pr-msvc-readline
branch
from
4c12b46
to
fa22b79
Compare
|
I managed to filter the relevant events in I have to do some more digging... |
Yes that one seems to be the hard part. I will agree with you here not to merge this PR into 7.1 release. You have done a lot to get this PR into this One possible debugging option to see if Windows GNU Readline 8.0 really fix the issue is to dynamic link to the MSYY2 MinGW |
@mariusgreuel Sounds great. Would you want to merge these into the current |
|
Current PR behavior.
|
Ahhh - reverse incremental search in the history of previously typed commands. Type fox and you will get the last |
|
| 4. Reset the chip and the terminal session can still be alive with a warning message (need to hit RETURN) This is not really a feature, and we shouldn't expect this to work... |
|
Since this PR is not ready, the official 7.1 Windows binary will be using MSVC build without GNU Readline support.
This will be the same as default MSYS2 mingw32/mingw64 build, which is without GNU Readline support. @stefanrueger and @mariusgreuel Another thing is that the shell command But I am okay if you think no need to change. |
|
The arduino-pack version will use MinGW32 and GNU Readline, similar to MSYS2 mingw32 build with GNU Readline.
But to me it is still quite acceptable to live with Issue No 4 for Arduino, since they do not really care about terminal mode. |
|
Just FYI as well. I believe the terminal mode limitation is of no concerns to Arduino. |
|
Just wondering if you know what will be the equivalent for the following two under Windows CMD prompt. Thanks. |
|
As I do not see how to get this PR production ready anytime soon, here is an old idea that I would like to resurrect: #1264 |
|
Wonderful job. PR #1264 exceeded my expectations! I will only request a minor version update as it works better than Readline 5.0 for Windows. |
|
Closing this in favor of #1264. |
In the end I convert this into a batch file for Windows CMD and Powershell. |
Happy new year, everybody! 🥳
Here is my first shot at readline support for MSVC.
Basically, its based on readline 5.0, with the Windows patches from Readline for Windows, plus some additions from me, including:
I have not spent much time with this library, but some things appear to work:
-c urclock - tHowever, there are also a few issues with it, so I do not think its production ready:
@stefanrueger If you have a Windows PC at your hands, could you give this PR a try? Is there anything specific you would like me to try?
Closes #1186