Cygwin: Windows absolute paths not recognized as such

[mtzguido]

Hi, I noticed that is_absolute does not return true for Windows paths when in a Cygwin environment. For instance the program below prints false, when I would expect a true.

let _ =
  let open BatPathGen.OfString in
  let fn = "C:\\a\\b\\c" in
  let path = of_string fn in
  print_endline (string_of_bool (is_absolute path))

I think this is due to this snippet, which simply does not check for "Cygwin" too:

batteries-included/src/batPathGen.ml

Lines 495 to 497 in 00c591e

	let windows = match Sys.os_type with
	| "Win32" -> true
	| _ -> false

Is this intentional or maybe just an oversight...?

This was tested on a fresh Cygwin install with batteries 3.8.0, I can provide more info if needed.

[UnixJunkie]

Do you have a proposed fix that you can test?

[UnixJunkie]

I.e. a pull request. Not everybody has access to Cygwin.

[mtzguido]

Yes I can test and submit a PR today, adding "Cygwin" above does seem to fix it. I'll add some unit tests too but I suppose there's no automated Cygwin test run?

[UnixJunkie]

I do not manage batteries' CI tests so I don't know.

…

On Thu, Dec 19, 2024 at 17:53 Guido Martínez ***@***.***> wrote: Yes I can test and submit a PR today, adding "Cygwin" above does seem to fix it. I'll add some unit tests too but I suppose there's no automated Cygwin test run? — Reply to this email directly, view it on GitHub <#1136 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFUFAGOCZG4WKLXLZONIST2GMB2ZAVCNFSM6AAAAABT3ZTFCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJVGQYDQMZQGA> . You are receiving this because you commented.Message ID: ***@***.*** com>

[UnixJunkie]

If you can implement your proposed fix and test that it works; that's enough for me.

[mtzguido]

Sorry for the delay, that does fix this immediate issue but causes some more paths to be printed in Windows format, which I suppose is not desirable. I'm also not well versed in Cygwin at all, trying to make some sense of it before sending a PR here.