jtdaugherty / vty

A high-level ncurses alternative written in Haskell
BSD 3-Clause "New" or "Revised" License
319 stars 57 forks source link

vty-interactive-terminal-test looks like a good candidate to cross-platofrm migration #265

Closed ShrykeWindgrace closed 1 year ago

ShrykeWindgrace commented 1 year ago

I am not sure whether it is best put in vty or in vty-crossplatform, but it is definitely a useful test-suite.

I think there is a quick-win solution regardless of final package, because I already managed to run it under windows in the previous iteration.

jtdaugherty commented 1 year ago

Thanks for letting me know about this. I'll move it to vty-crossplatform.

ShrykeWindgrace commented 1 year ago

In my local tests, the cursorHideTest0 seems to be under the weather.

ShrykeWindgrace commented 1 year ago

Ha, found the culprit: the very first event that we receive on windows is an EvResize - hence for that UT stage "show the cursor ends so fast, that it is invisible for the naked eye.

Possible solutions:

jtdaugherty commented 1 year ago

It seems to me that the second option -- being prepared to skip the EvResize event and wait for another one -- would be best, since I think it should probably be okay to get an immediate resize event on startup. I don't know if that would require lots of the other tests cases to have to get updated, though.

ShrykeWindgrace commented 1 year ago

@jtdaugherty ok, the fix is just a couple of lines of code. If you plan to move this test this week, I'll wait a bit - otherwise, I'll open a PR soon. My implementation is, in fact, four PRs (in vty to drop the test, in crossplatform to reintroduce the test, and in unix/windows to expose necessary module exports).

It makes me wonder if there is there such a thing as a cross-project multiple target PR? 😃

jtdaugherty commented 1 year ago

in unix/windows to expose necessary module exports

What needs to change in those packages? I wouldn't expect any change to be necessary.

ShrykeWindgrace commented 1 year ago

in unix/windows to expose necessary module exports

What needs to change in those packages? I wouldn't expect any change to be necessary.

Come to think about that, maybe I can manage without modifying these two packages, use just careful imports in vty-crossplatform.

jtdaugherty commented 1 year ago

maybe I can manage without modifying these two packages, use just careful imports in vty-crossplatform

I'm still a bit confused as to why anything would need to change at all, other than just moving the test program between packages.

ShrykeWindgrace commented 1 year ago

Currently vty-crossplatform only exposes mkVty, and most of tests in that suite build only the Output without full Vty.

jtdaugherty commented 1 year ago

Hmm, okay. And it appears that if they did build a Vty and thus take over terminal input, that would break the model for that test program.

If this is going to necessitate adding more to the vty-crossplatform API, then I would like those additions to be clearly namespaced under a Testing module hierarchy or something like that (but not Internal), e.g., Graphics.Vty.CrossPlatform.Testing. It's important to me that the additions are clearly not intended for end-user consumption.

Another consideration is that if vty-crossplatform is going to get a new module exposing an Output constructor, that constructor needs to be abstract with respect to the "settings" types used by each platform package. So, in other words, we can't add mkOutput :: UnixSettings -> IO Output (or similarly for Windows), so it'd need to be mkOutput :: IO Output and that means it would need to use the "default" settings for that platform when building the Output. That looks like it ought to be okay, at least for now, because the interactive test program doesn't change any of the default settings when it constructs its Outputs.

jtdaugherty commented 1 year ago

Also, I just noticed some stale text in the program,

putStrLn $ "Done! Please email the " ++ outputFilePath ++ " file to coreyoconnor@gmail.com"

If you have time, I'd love it if you could just shorten that to "Done!" and potentially add a note that if there is a problem, then submit the output file to the Vty repository in a ticket rather than sending it to the previous maintainer's email address.

jtdaugherty commented 1 year ago

(That shows just how much I've looked at and used that program..)

jtdaugherty commented 1 year ago

In fact, there are numerous mentions of Corey's email address in that file. :(