(Maybe String) parameters in API functions about win32 HOT 18 CLOSED

Just wanted to pitch my vote for the "breaking changes" approach. Haskell on Win32 is already a tad bit rough. If we pick an ugly backwards compatible approach, I deeply fear we would sacrifice "forward compatibility" (i.e. Win32 usage would only decrease because of superfluous complexity).

from win32.

Thanks in advance for doing this!

It's a shame to have to add so many new functions to the API just for this. The type signatures for all those functions probably should have had Maybe String instead of String to begin with. But I agree, I don't see any choice at this point. Do you have an estimate for about how many new functions would need to be added to the API all together?

We should agree on a standard naming convention for API functions that take Maybe String instead of String for some of their arguments. E.g., something like findWindowOpt corresponding to findWindow, etc.

Some of those functions might use newTString instead of withTString. You'd need a new version of that, too.

I think you're right that newOptCString and friends ought to go in base. That should be discussed on the [email protected] mailing list. Reference this issue in your post. It could be that someone there knows some reason why this wasn't done to begin with.

Even if that does make it into base, you don't want the Win32 library to depend on a late version of base. So you would need to define those functions here also, probably in System.Win32.Types, wrapped in some CPP that defines them if the version of base is old and otherwise imports them.

from win32.

This search query leads me to believe there are a little under 150 such functions in the WinApi.

I am not sure if the Haskell wrapper supports them all though, but it wouldn't hurt adding them along the way if they're feasible to me.

I'll try to "cross-search" the results in the link above with the functions wrapped here and add an Opt version as first step as you suggested and raise a pull request.

Thank you for assistance :)

from win32.

@a3f,

In cases like your example of findWindow I usually re wrap the lower-level variant (c_findWindow in this case) within my own local Windows module. The higher-level function's source can be used as a template, so it's usually pretty straightforward.

I agree with you that it would be nice to have more convenient ways of supplying null pointers to common functions like this one.

from win32.

@a3f, @ygale,

I am wondering if it would be better to just change the signatures for these functions, instead of adding a whole lot of new, nearly-duplicate functions to accomplish this goal.

One reason (and a bit off-topic) for this is I noticed a very large number of functions (100+?) that have incorrect type signatures, usually due to using Int instead of CInt or another type, and also some more serious offenses. I was working on a pull request for that, about 1/2 done right now. Anyway, the only way to fix those is to change the type signatures (breaking backwards-compatibility), so why not just bite the bullet and do the same here?

from win32.

My thoughts are similar to @m37 here, probably they should be Maybe String in the API properly. The hacky workarounds don't seem to make it that much easier to be compatible, and make it nasty forevermore. I think the number of people using Win32 directly is relatively low (in contrast to the vast number that use it indirectly) so fixing the relatively small number of people impacted should be feasible. [Disclaimer, I don't think it breaks any of my code, so I might be a little bit less concerned than I otherwise would be.]

from win32.

@ndmitchell I also agree that the approach of @m37 is really the best, at least theoretically. We also would feel very little direct impact. But do you have any estimate of how much breakage there would likely be on Hackage, and in Windows builds of GHC, if we go that way?

from win32.

On hackage there are 74 dependencies on Win32: http://packdeps.haskellers.com/reverse/Win32 - I guess a random sampling would probably give us the number that use relevant packages. No idea about GHC, but I suspect that's not too bad at all. Note that upgrading in a reverse compatible way should only be 5 lines, roughly:

#if Win32_min_version(2,5,0)
just = Just
#else
just = id
#endif

Then just add just in front of the necessary arguments.

from win32.

Elliot Robertson posted this on the libraries list:

A quick reverse search [0] shows 74 packages having Win32 as a direct dependency, 3 of which are out of date. The big ones that stand out to me are directory (which will be up to Phil and me to fix) and process (which is maintained by Michael Snoyman (@snoyberg, ping)). Both are GHC dependencies, and each has 800+ reverse dependencies, so the release timing there will need to be well managed (possibly to coincide with the next major GHC RC?).

This being said, I'm definitely in favor of better Win32 support. If it takes some extra effort to fix long-standing defects, that's the price of doing business.

from win32.

I don't have lots of experience using the Win32 library directly, though I have touched it a few times. I don't have a strong feeling on this issue, but I tend to agreeing with this being a change worth making (despite my normal hesitancy around breaking changes). If there's a branch of win32 available to test against, I'd be happy to test out process against it and see how much breakage ensues.

from win32.

A few other important potentially affected packages that caught my eye:

• cabal-install
• haskeline
• unix-compat
• time
• HTTP

from win32.

I posted to the café and to reddit to make sure that everyone who needs to know about this will know. So far everyone seems to be positive, so I think it's time to work on getting an actual patch ready. @a3f and @m37, do you already have some work? Can you coordinate on it, or someone just do it?

from win32.

In via reddit and for whatever my barely-invested newbie's opinion is worth, this seems like a great idea. I'd be on the fence if it were just the Maybe change, but @m37 pointing out those other type issues puts it over the top.

from win32.

@ygale,

I do have some code that I was working on, it's been a bit since I touched it but I think I'm about 1/2 way done. It doesn't include the changes to Maybe types like @a3f was working on, so I think if @a3f works on that part, and I continue on the rest, we probably won't overlap and our work would merge nicely. I'm open to other suggestions though.

from win32.

@ygale,

I also had some code done but then got lazy. I am willing to get this patch done though (I would dare say it mostly is).

I faced a design choice for which I need some input though:
In Foreign.C.String, we have:
peekCAString :: CString -> IO String
Now it ought to be either
peekCAString :: CString -> Maybe (IO String)
or
peekCAString :: CString -> IO (Maybe String)

Same goes for the other functions. Which one should we go for?
Maybe (IO String) makes the most sense, as there is no IO involved in case of a nullPtr, you just return Nothing.
IO (Maybe String) appears to be easier to work with.

I agree with @m37, I don't think that the eventual patches would interfere much with each other. I also agree on the point that the older signatures should be removed.

@mikesteele81
Ye, that's what I went with.

from win32.

@a3f, I would go with the type signature that matches the function's intended use, namely peekCAString :: CString -> IO (Maybe String), particularly as, if someone needed the Maybe (IO String) version for some reason, it's quite a trivial function to recreate.

from win32.

Because this is a breaking change, I suggest keeping the existing Win32 as old-win32 for a couple of releases at least. I would expect the support cost to be small, since there would be little or no changes to the old (often incorrect) functions.

from win32.

Hmm does anyone know what ever happened to this proposal/change?

from win32.