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

Introduce a use keyword for specific opens #11558

Closed
mimoo opened this issue Sep 23, 2022 · 10 comments
Closed

Introduce a use keyword for specific opens #11558

mimoo opened this issue Sep 23, 2022 · 10 comments

Comments

@mimoo
Copy link

mimoo commented Sep 23, 2022

I'd like to propose the introduction of a new keyword use to avoid opening everything in the scope. This is inspired by Rust and would improve readability. For example:

Bad:

open Hexstring

let () = 
	let b = Bytes.of_string "\x01\x02" in
	let _ = encode b

Better:

use Hexstring.{encode, decode}

let () = 
	let b = Bytes.of_string "\x01\x02" in
	let _ = encode b
@mimoo
Copy link
Author

mimoo commented Sep 23, 2022

(I'm not too sure how hard this would be to implement but I'd be willing to give it a shot if it's not too crazy)

@nojb
Copy link
Contributor

nojb commented Sep 23, 2022

In OCaml you can do:

let encode = Hexstring.encode
let decode = Hexstring.decode

Or, shorter:

let encode, decode = Hexstring.(encode, decode)

@yallop
Copy link
Member

yallop commented Sep 23, 2022

Relatedly, @stedolan's proposal in #10013 would allow

open Hexstring.(struct let encode, decode end)

which is one character shorter and (more importantly) avoids repeating the names. We could perhaps extend it further to allow the following:

let Hexstring.(encode, decode)

@Octachron
Copy link
Member

A new keyword is major breaking change. Moreover, the keyword does not seem required in your syntax proposal, extending open might work:

open Hexstring.{encode, decode}

However, the .{} is used for indexing Bigarray and it might be better to avoid encroaching that existing syntax.
Currently, an encoding of your proposal would be to open a module constrained by a signature:

open (Hextring: sig val encode: Bytes.t -> Bytes.t val decode: Bytes.t -> Bytes.t end)

This is clearly a too heavy syntactically. But it might be a better starting point for extending the syntax, maybe something like

open (Hextring:[ val encode; val decode ])

This syntax also has the advantage of removing the ambiguity between importing types, values, classes, class types or module types in your proposal:

module M = struct
  type t
  module type t
  class t
  end
  use M.{t} (* which `t` is that *)

@yallop proposal can be also extended to remove this ambiguity if we allow

type M.(t)
module type M.(t)
class M.(t)

@mimoo
Copy link
Author

mimoo commented Sep 23, 2022

reusing open is ambiguous though, as it would work differently depending depending on context:

open Hexstring (* opens the content of Hexstring *)
open Hexstring.{OtherModule} (* does this open the content of OtherModule, or does it import OtherModule into scope? *)

@dbuenzli
Copy link
Contributor

Bad:
[...]
Better:
[...]

Maybe best is:

let () = 
  let b = Bytes.of_string "\x01\x02" in
  let _ = Hexstring.encode b

Or if you really have to:

let () = 
  let open Hexstring in
  let b = Bytes.of_string "\x01\x02" in
  let _ = encode b

Topmost selective imports are a mental burden detrimental to code reading, you need to maintain all of these in your brain to read the code.

@mimoo
Copy link
Author

mimoo commented Sep 23, 2022

The problem is that today people abuse open, so either you forbid that (which you can't anymore) or you let people import selectively instead of the whole module

@dbuenzli
Copy link
Contributor

The problem is that today people abuse open

It seems to me that nowadays people are sufficiently aware of its problem not to abuse it. Most topmost opens are used for namespacing, eDSLs or let operators.

so either you forbid that (which you can't anymore) or you let people import selectively instead of the whole module

When reading code below the fold having a bulk open or a selective one makes no difference, both are a mental burden. That's the reason why I don't think this proposal is a good solution.

There are more options than you suggest. You can devise linters to forbid bulk opens or devise a warning for when an open introduces named value identifiers in the global scope.

@johnyob
Copy link
Contributor

johnyob commented Dec 24, 2022

I'm also in favour of adding something like @Octachron's proposal.

I have an (unmaintained) ppx for it: https://github.com/johnyob/ppx-open. But like ppx_import it cannot import types from modules defined in the current file.

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 Dec 25, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2024
@ocaml ocaml locked and limited conversation to collaborators Jan 24, 2024
@Octachron Octachron converted this issue into discussion #12938 Jan 24, 2024

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

6 participants