really_X is a terrible naming scheme IMO. Can we come up with a better naming scheme? How about input_n and input_string_n?

I'm not married to the existing name, but input_n is not any better IMO.

Let me make the case for it: it mirrors C's printf/snprintf scheme ie. it succinctly signifies that there is something different about the function, and it can easily be remembered that the difference concerns the number of input elements.

please don't emulate anything from C's IO, it's terrible :p
I think Lwt has input_exactly or something to this taste, which is maybe more clear. I think input_n doesn't mean anything, and is worse than really_input which at least is familiar.

input_exactly sounds a lot better than really_input. really_input sounds really... bad.

It is confusing to have both read_line : unit -> string option and read_lines : string -> binary:bool -> string list. Not sure what is the best way to handle this? 1) rename one of them, 2) omit one of them, 3) ...

I agree. There's nothing to signify that read_lines concerns a file -- you have to go in and read the documentation. read_file_lines and read_file_all or file_read_lines and file_read_all would make more sense to me.

Can we make read_* take t rather than unit? It's slightly more verbose when reading from stdin, but also explicit about where the data are coming from.

Or they could be named read_float_stdin, etc.

@hcarty The functions taking t as argument are already there with prefix input_*. Another possibility is to put all the functions specialised to stdin, stdout and stderr in their own module, say Stdio.

write_all inconsistently has the filename arg, which I guess makes sense since you want to distinguish the arguments, but I think if it's here, it should go in the analogous functions.

I personally think these 2 should still be file_write_lines and file_write_all. A little longer, but you're saving on opening the file, and it helps clarify their purpose.

If you keep the argument name, it could be shortened to file for convenience -- it's obvious that it's a name from the type.

I'd rather have the label remain filename because it's explicit. It can't be confused with file_content or anything else.

Did you mean files ?

I understand the desire to have only the flags that matter for input channels, but I don't see this way of doing as a very forward looking way w.r.t. functional abstraction.

I'd rather have an open_flags type that can be extended later if needed. This allows clients to abstract an open-like function simply under the signature string -> In_channel.open_flags list -> In_channel.t. I suspect extending open_flags will break less code than extending open_function if that was to occur at some point in the future.

Another argument against this design is that this is the kind of information you may want to have in configuration files and/or read from user interfaces, have under other forms in your code to be mapped on that representation or simply have higher-level data structures that hold that information. In that case having a datatype for specifying it leads to much more convenient code to write for the client -- this especially applies for Out_channel.

Also understanding the TBA of with_file is that it will do the exception safety open/close dance, a bare open' : (string -> t) open_function should be provided.

Besides if you can create a file by reading you likely want to add a mode argument.

To give a concrete example here is a function that uses its own defaults for (Unix) open flags but still allows the client to override them. It would be quite annoying to expose and implement this function with the type 'a open_function way of doing this PR suggests.

Makes sense, I will switch back to having an open_flag type.

I added a flag type back, with different flags for input and output channels. I left in the "non-blocking" flag even if it does not fit very well with an "option"-based API... I'm not sure what to do about that.

I would rather call this input_to_string.

Sounds fine; done.

What about read_iter_lines ? What about read_fold_lines ? etc. etc. I think these function are useless, writing with_file file input_lines isn't egregious (see @gasche's recent comment about "data structures that have delicate creation ceremonies"). Also the names clash with the convention for stdin input functions (but more on these below).

They have been removed.

These function feel ad-hoc. They bundle multiple effects and only work only on stdin I would move on to drop them.

They have been removed.

See discussion in In_channel. Flags are better to specify these things.

create opens which may create a file or not... I'd rather have open'. We are working with system calls here not simply only creating the channel abstraction, I prefer if we make it clear which system call is being used here.

I switched to open_ (could use open' as well, I just had forgotten about it).

filename doesn't make any sense here, it's not a filename it's a file path. Again this is simply with_file file output_lines, no need to provide this (especially, note that you didn't provide any of the many possible options for output file creation in these signature...).

Agree, will remove these.

I would elect to remove all of these stdin/stderr specific functions. I can't count the number of times I used them and then had to change my code because I wanted to perform my ouput on another channel (e.g. switching fromstdin to stderr). If you don't provide them people are more likely to write their code on a generic channel value which is good for them®.

I tend to agree with this argument.

@alainfrisch opined that specifying the channel argument could be heavy, but am not really convinced to be honest.

I will remove all the std{in,out,err}-specific functions from this PR.

output_line : t -> string -> unit symmetric to input_line would be nice to have.

Thanks, I added this one as well.

I add it here, so I don't forget to re-check later.

If we accept the with_file functions (and I really welcome their introduction) we should mention that the user has to be careful and avoid capturing the environment, for example leaking no-longer open channels.

I won't fight about this but I would rather use ' here. _ is meant to be a space. These final spaces always leave me wanting for more.

Headings start at 1 in modules.

Also I think it would be nice to uniformize the docs strings to the format "[func a0 a1] description" of the resulting value or the effect (for unit).

Fixed, but the docstrings could use some more work I think.

Is 0 really the right default ?

Create's default is not documented.

Is 0o666 really the right default ? I would rather go for 0o644.

OK, fixed.

On the other hand 0o666 is the one used by Stdlib.open_out, so there is a slight potential for confusion here.

The permissions given to open are modified by the user's umask. 0o666 is the traditional "friendly" set of permissions. Most users have a 0o022 umask that will turn write permissions off for group and for others, resulting in the expected 0o644 permissions. But some users want their files to be group-writable by default, and have 0o002 umask, and there is no reason to annoy them by forcing the permissions to 0o644.

Makes sense, thanks for the explanation. Will set the default back to 0o666.

The default create permissions are not documented.

A discussion about which newline character that is may be useful here.

Added a line about that.

A discussion about which newline we are talking about would be nice here.

Added a line about that.

I suggest to add All errors arising during closing are ignored. (idem for in in_channel.mli).

Fixed.

After "for reading" I would add "according to [flags] (defaults to [[]])." One thing that could be useful to document here aswell is the cloexec behaviour on these handles.

Fixed.