Skip to content
This repository has been archived by the owner on Mar 1, 2019. It is now read-only.

Commit

Permalink
improve .merlin file generator
Browse files Browse the repository at this point in the history
Provide more FLG directives. Don’t sort the whole list of lines at the
end to keep the directives more in the order we want.
  • Loading branch information
agarwal committed Nov 15, 2016
1 parent 45dfffe commit 52f5f57
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 24 deletions.
97 changes: 74 additions & 23 deletions lib/project.ml
Original file line number Diff line number Diff line change
Expand Up @@ -284,37 +284,88 @@ end
(******************************************************************************)
type content = string list

let merlin_file ?short_paths items : string list =
[
["B +threads"; "PKG solvuu-build"];

(if short_paths = Some () then ["FLG -short-paths"] else []) ;

(* libs *)
List.map (filter_libs items) ~f:(fun x ->
[
sprintf "S %s" x.dir;
sprintf "B _build/%s" x.dir;
sprintf "B _build/%s" (dirname x.dir);
]
) |> List.concat;
let merlin_file ?safe_string ?short_paths ?thread ?w items : string list =
let pick ~cmp l =
let l' = List.sort_uniq ~cmp:(Option.compare cmp) l in
match l' with
| [] -> (* [items] is empty *)
None
| [x] -> (* all [items] specify same value, possibly None *)
x
| _ -> (* [items] have different values, randomly pick last *)
List.last_exn l
in
let safe_string : unit option = match safe_string with
| Some x -> x
| None ->
List.map items ~f:(function
| Lib x -> x.safe_string
| App x -> x.safe_string
) |>
pick ~cmp:Unit.compare
in
let short_paths : unit option = match short_paths with
| Some x -> x
| None ->
List.map items ~f:(function
| Lib x -> x.short_paths
| App x -> x.short_paths
) |>
pick ~cmp:Unit.compare
in
let thread : unit option = match thread with
| Some x -> x
| None ->
List.map items ~f:(function
| Lib x -> x.thread
| App x -> x.thread
) |>
pick ~cmp:Unit.compare
in
let w : string option = match w with
| Some x -> x
| None ->
List.map items ~f:(function
| Lib x -> x.w
| App x -> x.w
) |>
pick ~cmp:String.compare
in

(* apps *)
List.map (filter_apps items) ~f:(fun x ->
[
sprintf "S %s" (dirname x.file);
sprintf "B _build/%s" (dirname x.file);
]
) |> List.concat;
[
(match thread with Some () -> ["B +threads"] | None -> []);
(match safe_string with Some () -> ["FLG -safe-string"] | None -> []);
(match short_paths with Some () -> ["FLG -short-paths"] | None -> []);
(match w with Some w -> [sprintf "FLG -w %s" w] | None -> []);

(* source directories *)
List.map items ~f:(function
| Lib x -> sprintf "S %s" x.dir
| App x -> sprintf "S %s" (dirname x.file)
) |>
List.sort_uniq ~cmp:String.compare;

(* build directories *)
List.map items ~f:(function
| Lib x ->
[
sprintf "B _build/%s" x.dir;
sprintf "B _build/%s" (dirname x.dir);
]
| App x ->
[sprintf "B _build/%s" (dirname x.file)]
) |>
List.concat |>
List.sort_uniq ~cmp:String.compare;

(* findlib packages *)
(
all_findlib_pkgs items
"solvuu-build"::(all_findlib_pkgs items)
|> List.map ~f:(sprintf "PKG %s")
|> List.sort_uniq ~cmp:String.compare
);
]
|> List.concat
|> List.sort_uniq ~cmp:String.compare

let meta_file ~version libs : Fl_metascanner.pkg_expr option =
let def ?(preds=[]) var value =
Expand Down
19 changes: 18 additions & 1 deletion lib/project.mli
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,24 @@ val app
type content = string list
(** Content of a file represented as a list of lines. *)

val merlin_file : ?short_paths:unit -> item list -> content
(** [merlin_file items] returns content of a .merlin file. It provides
S and B lines for merlin to find the source and build directories
of all [items] and PKG lines to let merlin search over all findlib
packages used by [items] (the package "solvuu-build" is also
added).
The optional arguments correspond to flags that can be specified
for each item of [items]. If you set them differently, it isn't
clear what value to use in the .merlin file. By default, we use a
heuristic to pick, but you can provide a value explicitly if
desired. *)
val merlin_file
: ?safe_string:unit option
-> ?short_paths:unit option
-> ?thread:unit option
-> ?w:string option
-> item list
-> content

val meta_file : version:string -> lib list -> Fl_metascanner.pkg_expr option
(** Return a findlib META file for given libs, where [version] should
Expand Down

4 comments on commit 52f5f57

@pveber
Copy link
Contributor

@pveber pveber commented on 52f5f57 Nov 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand why the safe_string and co. are now both optional and of type unit option. Is it a way to disable an option that might be set in some component (and that would therefore be picked for the merlin file)? If so, how about introducing a type like [ `yes | `no | `auto ] for such arguments, with default at `auto?

@agarwal
Copy link
Member Author

@agarwal agarwal commented on 52f5f57 Dec 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your interpretation is correct:

  • Some (Some ()) means set the option.
  • Some None means don't set the option.
  • None means use a heuristic to set the option.

I agree this is confusing but the benefit is that the inner unit option type is the same as the type of the corresponding fields in a lib or app. If we use [ `yes | `no | `auto ], I'd be inclined to replace unit option everywhere with [ `yes | `no ], but I'm not sure that is better. The unit option type is rather intuitive to represent the type of a command line flag that can be given or not.

Would better documentation resolve the matter? What if we simply added the above explicit list of values and their meanings.

@pveber
Copy link
Contributor

@pveber pveber commented on 52f5f57 Apr 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rather a cosmetic motivation, to be fair. I think what would make the best sense to me would be [`yes | `no] option, where Some `yes would mean put the option, Some `no don't put it and None leave the default is the underlying tool. But again, my point is really cosmetic only. There's nothing here that documentation couldn't fix.

@agarwal
Copy link
Member Author

@agarwal agarwal commented on 52f5f57 Apr 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added for consideration in #62.

Please sign in to comment.