Question: What code conventions to use?

[glennsl]

1. Should names be js-y (forEach) or ml-y (for_each)? Reason is going for js-y, and the npm ecosystem is obviously js-y, yet many bindings seem to go for ml-y naming.
2. What about file naming? BS has some constraints here, e.g. no dasherized filenames allowed.
3. Standard prefix or postfix for module signatures? E.g. IModule, Module_Sig or MODULE
4. Should types follow the same convention as functions and variables, or be different?
5. Constants?
6. Prefer short names for brevity or long names for clarity?
7. Should comments be placed before or after the code they address.
8. Tabs or spaces? 2 or 4 spaces per indent level?
9. Prefer piping or intermediate let bindings?
10. Prefer labeled and optional arguments when several arguments have same type? Append a unit argument when all arguments are labeled?
11. Prefer let f : int -> string = fun x ->... over let f (x: int): string = ...?
12. Place vertical bar | indented or leveled with the enclosing structure in variant defintions, match expressions and piping?
13. How to break long lines?
14. Exit branches first or last in a pattern match?
15. Named convention for "overloading" externals (https://github.com/BuckleTypes/wiki/issues/4#issuecomment-289005151)
16. Naming convention for list destructuring (https://github.com/BuckleTypes/wiki/issues/4#issuecomment-305549808)

[chenglou]

The last two are solved by refmt; no need to establish conventions?

[glennsl]

refmt isn't even consistent with itself though

This is how it formats ml and reason, respectively:

let _ =
  match Background.Refmt.refmt input with
  | ("Failure",error) -> cb error None None
  | (conversion,outText) ->
      (match conversion |> (Js.String.split "to") with
       | [|inLang;outLang|] -> cb outText (Some inLang) (Some outLang)
       | _ -> ())
type 'a case =
  | StrictEqual of 'a* 'a
  | NotStrictEqual of 'a* 'a
  | Equal of 'a* 'a
  | NotEqual of 'a* 'a

switch (Background.Refmt.refmt input) {
| ("Failure", error) => cb error None None
| (conversion, outText) =>
  switch (conversion |> Js.String.split "to") {
  | [|inLang, outLang|] => cb outText (Some inLang) (Some outLang)
  | _ => ()
  }
};

type case 'a =
  | StrictEqual 'a 'a
  | NotStrictEqual 'a 'a
  | Equal 'a 'a
  | NotEqual 'a 'a;

And chained pipe operators is pretty broken:

• https://github.com/facebook/reason/issues/1045
• https://github.com/facebook/reason/issues/1066

But the convention and what refmt does SHOULD of course be the same.

[chenglou]

We can't do much for the ocaml formatter; we currently don't control/have the energy to make it work well (plus, it doesn't even preserve comments). Pipe being broken is indeed bad, but it should be considered as a refmt bug. Formatting conventions should be filed to the reason repo as issues.

[glennsl]

Sorry, I didn't mean for this to be an issue to discuss and establish every detail of convention, but rather to point out some frequently asked questions that ought to be answered by an FAQ. And as you point out, refmt isn't going to provide an answer for BuckleScript in general, just Reason specifically.

[chenglou]

hey no need to be sorry, I was trying to be mean or anything lol

[glennsl]

(Sorry,) I might be part canadian (I'm kind of in the right latitude at least)

[chenglou]

Sorry, I'm Canadian too

[chenglou]

Ok let's see

1. For Reason work, let's go with camelCase. As an aside, I haven't seen people complain about this too much, which is nice.

2. No dash, no space. camelCase (just to be consistent with above). First letter must be uncapitalized to avoid case-sensitive FS problem. This needs to be encoded in the build system/tools, somehow: https://github.com/bloomberg/bucklescript/issues/913 lots of people will skip this wiki.

3. No idea. Let's not establish this one yet.

4. Yeah.

5. justLikeThisSinceEverythingIsConstantByDefault

6. Not sure yet.

7. Not sure yet either. Should be solved through refmt: https://github.com/facebook/reason/issues/1015

8. refmt

9. Not sure either.

10. Yes, prefer that. For appending unit: necessary for optional (will warn anyway; we'll just provide a good message like in BetterErrors). No unit needed for non-optional labelled functions otherwise.

11. Those two examples happen to be the same. In general they're not, so they need two separate AST representations. The former allows room for polymorphic type variables.

12. refmt

13. refmt

Rough draft of answer. Do the answers make sense?

[glennsl]

Regarding 10. Non-optional labeled functions doesn't need an extra unit argument, but @bobzhang once told me it's an OCaml convention to do so still. I don't know why, but I've been doing that for the Js FFI. So should it be preferred? (And if so there should be some reasoning behind it)

11: Same question, when the two forms are equivalent, which should be preferred?

[glennsl]

Another convention question: Should the "exit" branch(es) of a pattern match come first or last? E.g.

match get_opt foo with
| Some value ->
  (* do something with value
   * over several lines
   *)
| None -> ()

vs.

match get_opt foo with
| None -> ()
| Some value ->
  (* do something with value
   * over several lines
   *)

The latter is clearer, especially if there's nested matches, but sometimes the "exit" branches will be a wildcard match, in which case it has to come last. Should that then be the convention just for consistency, or should it be contextual?

[chenglou]

None isn't an exit, and sometimes the None branch is the spanning multiple lines. I'd say let's not impose a convention for this one yet.

[glennsl]

Yea I'm not saying None is "an exit", just using it as an example of a branch that "exits".

This is somewhat similar to the imperative dilemma:

if (thing) {
  /* do things with thing */
}

vs.

if (!thing) {
  return;
}
/* do things with thing */

(I prefer the latter for imperative code)

I've no problem with not instantly getting this answered though, I'm just putting it somewhere for safekeeping :)

[ul]

15. What about names of external instances for multi-type-with-optional-args functions, like https://nodejs.org/dist/latest-v7.x/docs/api/buffer.html#buffer_buf_copy_target_targetstart_sourcestart_sourceend ? My attempts ended with long unpronounceable (huh, this word itself is such one) suffix-piles. Thanks to @glennsl they are better now, but good to have smth. like explicit conventions for consistency and reduced mental exercises when write every multi-external binding.

[glennsl]

Another one to ponder: There are several conventions among functional programmers for how to name variables when destructuring a list:

• head::tail, with the short-form h::t
• first::rest
• x::xs (and y::ys etc.)
• The ocaml stdlib seems to be using a::l and a1::l1, a2::l2 etc.

We should probably try to standardize on one of these.