Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filename.{Unix,Win32,Cygwin} should be exported #6287

Closed
vicuna opened this issue Jan 6, 2014 · 9 comments
Closed

Filename.{Unix,Win32,Cygwin} should be exported #6287

vicuna opened this issue Jan 6, 2014 · 9 comments

Comments

@vicuna
Copy link

vicuna commented Jan 6, 2014

Original bug ID: 6287
Reporter: furuse
Status: confirmed (set by @damiendoligez on 2014-07-16T12:56:43Z)
Resolution: open
Priority: normal
Severity: feature
Version: 4.01.0
Category: standard library
Tags: patch, junior_job
Monitored by: @dra27

Bug description

stdlib's Filename provides filename handling functions specific for the current OS, but actually it defines for the 3 main flavors: Unix, Win32 and Cygwin. But they are hidden. They should be exported, probably with a small tweak to have the same signatures (for example, has_drive is only defined in Win32.)

I got some bug reports about my software that it does not run under Win32, and it was due to a typical failure of ignoring the drive name. I wrote a fix but it was lousy to test it since Filename.Win32 is not accessible. So far I copied filename.ml since its license matches with one of my software. But it is just a workaround.

File attachments

@vicuna
Copy link
Author

vicuna commented Feb 19, 2014

Comment author: @damiendoligez

Indeed, when running a program under cygwin that also launches native Windows executables, you need both Filename.Unix.quote and Filename.Win32.quote.

@vicuna
Copy link
Author

vicuna commented Jun 24, 2014

Comment author: nevor

I have attached a patch that expose the common set of functions augmented with the concat function.

@vicuna
Copy link
Author

vicuna commented Jul 16, 2014

Comment author: @damiendoligez

Re: filename.mli

Ouch. If we want to export them with the same signature (and I agree we do), we should define one "module type" for this signature and then declare the three modules with this signature, instead of triplicating the declarations.

So don't apply the patch as-is.

@vicuna
Copy link
Author

vicuna commented Jul 16, 2014

Comment author: nevor

Indeed, I thought about this but didn't know how detailed the documentation had to be (w.r.t. differences between implementations). I have attached a new proposition.

@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 15, 2020
@nojb
Copy link
Contributor

nojb commented May 26, 2020

What do people think about this request? The current implementation of Filename already defines the modules for each system, it would be just a matter of exposing them in the interface.

@nojb
Copy link
Contributor

nojb commented May 26, 2020

(my own personal opinion is that indeed there are some cases where having these modules available would be useful, but it is not something that comes up very often).

@hcarty
Copy link
Member

hcarty commented May 28, 2020

I'd like to have access to these submodules as well!

@dra27
Copy link
Member

dra27 commented Aug 3, 2022

I've a branch and even a document about it from around when #9719 was opened. Hopefully at some point post-5.0 it might even become a PR (trying to expose this properly revealed several subtle issues certainly with Cygwin's filename handling).

@shindere shindere changed the title FIlename.{Unix,Win32,Cygwin} should be exported Flename.{Unix,Win32,Cygwin} should be exported Aug 4, 2022
@shindere shindere changed the title Flename.{Unix,Win32,Cygwin} should be exported Filename.{Unix,Win32,Cygwin} should be exported Aug 4, 2022
@ocaml ocaml locked and limited conversation to collaborators Jan 26, 2023
@nojb nojb converted this issue into discussion #11940 Jan 26, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

5 participants