{"action":"created","issue":{"url":"https://api.github.com/repos/jaredly/reason-language-server/issues/173","repository_url":"https://api.github.com/repos/jaredly/reason-language-server","labels_url":"https://api.github.com/repos/jaredly/reason-language-server/issues/173/labels{/name}","comments_url":"https://api.github.com/repos/jaredly/reason-language-server/issues/173/comments","events_url":"https://api.github.com/repos/jaredly/reason-language-server/issues/173/events","html_url":"https://github.com/jaredly/reason-language-server/pull/173","id":378075476,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI4ODY3ODAx","number":173,"title":"Support opam+dune projects","user":{"login":"ryanartecona","id":889991,"node_id":"MDQ6VXNlcjg4OTk5MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/889991?v=4","gravatar_id":"","url":"https://api.github.com/users/ryanartecona","html_url":"https://github.com/ryanartecona","followers_url":"https://api.github.com/users/ryanartecona/followers","following_url":"https://api.github.com/users/ryanartecona/following{/other_user}","gists_url":"https://api.github.com/users/ryanartecona/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanartecona/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanartecona/subscriptions","organizations_url":"https://api.github.com/users/ryanartecona/orgs","repos_url":"https://api.github.com/users/ryanartecona/repos","events_url":"https://api.github.com/users/ryanartecona/events{/privacy}","received_events_url":"https://api.github.com/users/ryanartecona/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2018-11-06T23:17:32Z","updated_at":"2018-11-07T23:34:13Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/jaredly/reason-language-server/pulls/173","html_url":"https://github.com/jaredly/reason-language-server/pull/173","diff_url":"https://github.com/jaredly/reason-language-server/pull/173.diff","patch_url":"https://github.com/jaredly/reason-language-server/pull/173.patch"},"body":"Closes #152 \r\n\r\nI tried to set up a new reason project with dune and either opam or esy a few days ago and couldn't get either of the available VSCode plugins working. I thought it was something wrong with my own environment for a while, so I dug in and tried to diagnose and eventually found what was needed to make it work.\r\n\r\nMost of the code change volume here is in adding a new `examples/example-opam-dune-project`, so I recommend browsing by commit.\r\n\r\nThis adds preliminary support for opam+dune projects with no esy involved at all, and I believe it may fix some things for esy+dune projects too. I've tested things manually in the in-repo example projects, but more testing would be welcome.\r\n\r\nI've only tested with very recent versions of tools involved, i.e. esy 0.3.4, opam 2.0, and dune 1.5. In particular, this support relies on opam 2.x support for local switches (IIUC). \r\n\r\nNotes:\r\n- Detection of opam vs. esy is done early on based on whether an `_esy` dir and/or an `_opam` dir exists at the project root, where opam is chosen only if `_opam` exists but `_esy` is absent, and with the default falling back to using esy. The `_opam` dir is where a local switch lives. If a user wants to use a global switch, it seems reasonable to require them to symlink `./_opam` to that global switch's path manually (which is explicitly supported in opam 2.x) so that the package manager detection works properly here. Not sure where that should be documented, though.\r\n- The above detection runs before shelling out to `esy` at all. I found that recent esy will create some directories it expects to manage, i.e. `./_esy` when it runs anything at all even if the project doesn't have the necessary esy config files, which confuses the package manager detection above.\r\n- I also updated how local source files are mapped to module names and to expected compilation artifact file paths so that it worked for me. I'm not sure if this breaks anything to be honest, but it didn't work for me at all before, so I believe it should now work at least with latest dune & opam.\r\n\r\n    Say I have a dune project with a lib defined in `src/dune` with a name of `mylib` and a single file at `src/myFile.re`. This would generate a compiled artifacts dir at `_build/default/src/.mylib.objs`. In there would be a file `mylib.cmt` (no trailing underscores) for the top level lib module, and a file `mylib__MyFile.cmt` (note casing) for the lone wrapped module. In source code, doing an autocompletion lookup for `MyFile<.>` should look in the module given in the `mylib__MyFile.cmt` file. The updates here reflect these observations. This gets things working at least for local modules, but things still aren't 100% right for modules coming from opam dependencies or the ocaml stdlib, though I'm less keen to dig into that. I think there may have been some casing discrepancy in module/file naming that was masked by the case-insensitive filesystem in macOS, but unsure.\r\n- I also made the `buildCommand` for opam+dune projects use `opam exec -- dune build @install --root .` (using `jbuild` if `dune` isn't present) since I think that's a sane default. I know it won't work for all projects, but I also would expect users to customize their autorebuild command or let builds happen outside of RLS, so improvements there seem safely out of scope to me."},"comment":{"url":"https://api.github.com/repos/jaredly/reason-language-server/issues/comments/436817156","html_url":"https://github.com/jaredly/reason-language-server/pull/173#issuecomment-436817156","issue_url":"https://api.github.com/repos/jaredly/reason-language-server/issues/173","id":436817156,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNjgxNzE1Ng==","user":{"login":"ryanartecona","id":889991,"node_id":"MDQ6VXNlcjg4OTk5MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/889991?v=4","gravatar_id":"","url":"https://api.github.com/users/ryanartecona","html_url":"https://github.com/ryanartecona","followers_url":"https://api.github.com/users/ryanartecona/followers","following_url":"https://api.github.com/users/ryanartecona/following{/other_user}","gists_url":"https://api.github.com/users/ryanartecona/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanartecona/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanartecona/subscriptions","organizations_url":"https://api.github.com/users/ryanartecona/orgs","repos_url":"https://api.github.com/users/ryanartecona/repos","events_url":"https://api.github.com/users/ryanartecona/events{/privacy}","received_events_url":"https://api.github.com/users/ryanartecona/received_events","type":"User","site_admin":false},"created_at":"2018-11-07T23:34:13Z","updated_at":"2018-11-07T23:34:13Z","author_association":"NONE","body":"> My argument is in favor of people that want to use RLS without VS Code.\r\n\r\nAh, right. Makes sense.\r\n\r\n> My only problem with `package.json` is that it could mean no Esy too. Imagine a monorepo where a `package.json` is only used for frontent code.\r\n\r\nI agree.\r\n\r\nI wonder if we can get 90% of the way on autodetection if we're careful with precedence. How about:\r\n\r\n- (future) if a user override setting is present, just use what's specified and don't run detection\r\n- `_opam` present, `_esy` absent: use `Opam`, which will get its config & build artifacts from the local switch (or symlink)\r\n- `_esy` present: use `Esy(_)`, which will grab config from `esy command-env --json` whether the editor was launched from inside the esy env or not\r\n- `$OPAM_SWITCH_PREFIX` present: use the switch given in the env var, collecting config in much the same way as the `_opam` local switch case, i.e. using `opam config var lib` and respecting `$DUNE_BUILD_DIR` if present. This would probably just use `refmt` and `ocamlopt.opt` from whatever's on `$PATH`, assuming the editor would be launched in an env with `opam env` stuff already applied.\r\n- otherwise: I think if our cases are covered above, we could choose to default to opam or esy or display an error saying the build system could not be detected\r\n\r\nI _think_ this would cover the cases discussed above, including a global opam switch case. I think this avoids inferring too much from the presence of a `package.json` which could be irrelevant to esy, and similarly a `*.opam` file that may not imply opam is used as a dev tool at all. Thoughts?"}}
{"actor":{"display_login":"ryanartecona"}}