search

dbuenzli / down

An OCaml toplevel (REPL) upgrade
http://erratique.ch/software/down
ISC License
81 stars 3 forks source link

Initial support for Win32 #35

Closed nojb closed 4 months ago

nojb commented 4 months ago

See the discussion in #34.

The list of things that do not work is long: resize events, mouse events, completion (tab) makes it go completely haywire, etc. But the bare minimum works. I did it before @jonahbeckford revealed his previous work, so things are done somewhat differently and/or in an overly naïve way (but note that with this simple patch arrows keys do work).

I don't think this patch is good enough to claim that Down works on Windows, but may serve as a stepping stone for further improvements.

I leave some comments inline below.

nojb commented 4 months ago

down

nojb commented 4 months ago

BTW, you will notice there is a lot of flickering at the moment; I'm not sure what to do about it. I found https://stackoverflow.com/questions/73800269/flicker-free-console-updates-with-virtual-terminal-sequences after some random googling, but am not sure it is applicable.

nojb commented 4 months ago

I pushed 9bc5120d09673ccedce8f02dea95ca83e6c21813 to fix the flickering (it is not perfect but it gets rid of most of it).

flicker

nojb commented 4 months ago

completion (tab) makes it go completely haywire,

It turns out that when pressing TAB, Down invokes where.exe through Sys.command, ie through cmd.exe. The invocation completely messes up the rendering of Down (see "Before the fix" below). Through trial and error, I found that resetting the console to "raw" mode after the invocation fixes the issue. This kind of makes sense because console state is global (shared by all programs running on the console) and it is probably the case that cmd.exe messes with the state of the console when invoked. For good measure, I restore the console to its original state before invoking any external program and reset it afterwards (see b8987b4de9baa118ae7d8cb1e2441831c59bdaf6)

Before the fix:

bug

After the fix:

fix

nojb commented 4 months ago

Completion and doc lookup (using ocp-index) is now working, if one uses the patched https://github.com/OCamlPro/ocp-index/pull/170.

completion

I think this makes Down usable on Windows (there are still things to improve of course, but it is a start). @dbuenzli let me know if you would like me to do any changes, or feel free to pick the fixes and integrate them directly after massaging them into a shape that you are happy with :)

dbuenzli commented 4 months ago

Altogether that looks quite an unintrusive Windows support to me which makes me happy. If you want to stop here (please confirm) I'm happy to merge this and make a release. If you have in your head things that can be improved for other people to grab, please don't hesitate to open more specific issues.

dbuenzli commented 4 months ago

If you have in your head things that can be improved for other people to grab, please don't hesitate to open more specific issues.

More precisely it would be nice to have an issue that records exactly what doesn't work as expected to avoid too much bug reporting noise and so that I can refer to it to warn people appropriately somewhere from the docs.

nojb commented 4 months ago

Altogether that looks quite an unintrusive Windows support to me which makes me happy. If you want to stop here (please confirm) I'm happy to merge this and make a release.

Yes, I confirm. I think this is a good check point.

If you have in your head things that can be improved for other people to grab, please don't hesitate to open more specific issues.

More precisely it would be nice to have an issue that records exactly what doesn't work as expected to avoid too much bug reporting noise and so that I can refer to it to warn people appropriately somewhere from the docs.

I played with it a bit more today and I couldn't find any obvious problems. I will open an issue if I come across anything. Looking forward, if you agree, I propose the following: whenever a Windows issue is reported here, assign it to me or just ping me, and I will take care of both triaging it and investigating possible fixes.

jonahbeckford commented 4 months ago

Wow. Thanks @nojb !

dbuenzli commented 4 months ago

I propose the following: whenever a Windows issue is reported here, assign it to me or just ping me, and I will take care of both triaging it and investigating possible fixes.

That's fine with me. Could you perhaps just make an issue about the status of window resize on windows.

dbuenzli commented 4 months ago

Your patches are in, thanks.

nojb commented 4 months ago

I will take a closer look at resizing later and open an issue, thanks!