feat(describe-pp): print reason files with `refmt` by anmonteiro · Pull Request #10322 · ocaml/dune

anmonteiro · 2024-03-28T05:03:25Z

this initial diff has refmt hardcoded in bin/describe_pp.ml
It might be better to add a new field to the dialect and model this use case in the dialect directly too

rgrinberg · 2024-03-28T11:19:49Z

test/blackbox-tests/test-cases/describe/describe-pp/describe-pp.t/run.t

  $ dune describe pp src/main_re.re
-  ;;Util.log "Hello, world!"
+  # 1 "_build/default/src/main_re.pp.re.ml"
+  # 1 "src/main_re.pp.re"


How come we're printing the location directives now?

the tests have a dedicated, fake refmt executable that I changed to match the real refmt by printing to stdout

anmonteiro · 2024-03-28T21:43:36Z

It might be better to add a new field to the dialect and model this use case in the dialect directly too

I changed the initial implementation to this:

added a field dump_ast in dialects
described those actions for OCaml and Reason
in describe pp, execute the dialect's dump_ast if it exists, falling back to OCaml's, known to exist

rgrinberg

LGTM. @nojb could you take a look at the dialect changes?

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

anmonteiro · 2024-03-28T22:21:38Z

I renamed dump_ast to print_ast, since dump_ast was ambiguous (could also be interpreted as "dump AST to a file")

anmonteiro · 2024-03-28T22:35:49Z

perhaps print_ast should be versioned?

nojb

The dialect changes LGTM. Thanks!

* feat(describe-pp): use the dialect printer if available Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * rename `dump_ast` to `print_ast` Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> --------- Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

@anmonteiro

CHANGES: ### Added - allow libraries with the same `(name ..)` in projects as long as they don't conflict during resolution (via `enabled_if`). (ocaml/dune#10307, @anmonteiro, @jchavarri) - `dune describe pp` now finds the exact module and the stanza it belongs to, instead of guessing the name of the preprocessed file. (ocaml/dune#10321, @anmonteiro) - Print the result of `dune describe pp` with the respective dialect printer. (ocaml/dune#10322, @anmonteiro) - Add new flag `--context` to `dune ocaml-merlin`, which allows to select a Dune context when requesting Merlin config. Add `dune describe contexts` subcommand. Introduce a field `generate_merlin_rules` for contexts declared in the workspace, that allows to optionally produce Merlin rules for other contexts besides the one selected for Merlin (ocaml/dune#10324, @jchavarri) - melange: add include paths for private library `.cmj` files during JS emission. (ocaml/dune#10416, @anmonteiro) - `dune ocaml-merlin`: communicate additional directives `SOURCE_ROOT`, `UNIT_NAME` (the actual name with wrapping) and `INDEX` with the paths to the index(es). (ocaml/dune#10422, @voodoos) - Add a new alias `@ocaml-index` that uses the `ocaml-index` binary to generate indexes that can be read by tools such as Merlin to provide project-wide references search. (ocaml/dune#10422, @voodoos) - merlin: add optional `(merlin_reader CMD)` construct to `(dialect)` stanza to configure a merlin reader (ocaml/dune#8567, @andreypopp) ### Changed - melange: treat private libraries with `(package ..)` as public libraries, fixing an issue where `import` paths were wrongly emitted. (ocaml/dune#10415, @anmonteiro) - install `.glob` files for Coq theories too (ocaml/dune#10602, @ejgallego) ### Fixed - Don't try to document non-existent libraries in doc-new target (ocaml/dune#10319, fixes ocaml/dune#10056, @jonludlam) - Make `dune-site`'s `load_all` function look for `META` files so that it doesn't fail on empty directories in the plugin directory (ocaml/dune#10458, fixes ocaml/dune#10457, @shym) - Fix incorrect warning for libraries defined inside non-existant directories using `(subdir ..)` and used by executables using `dune-build-info` (ocaml/dune#10525, @rgrinberg) - Don't try to take build lock when running `coq top --no-build` (ocaml/dune#10547, fixes ocaml/dune#7671, @lzy0505) - Make sure to truncate dune's lock file after locking and unlocking so that users cannot observe incorrect pid's (ocaml/dune#10575, @rgrinberg) - mdx: link mdx binary with `byte_complete`. This fixes `(libraries)` with foreign archives on Linux. (ocaml/dune#10586, fixes ocaml/dune#10582, @anmonteiro)

@anmonteiro

CHANGES: ### Added - allow libraries with the same `(name ..)` in projects as long as they don't conflict during resolution (via `enabled_if`). (ocaml/dune#10307, @anmonteiro, @jchavarri) - `dune describe pp` now finds the exact module and the stanza it belongs to, instead of guessing the name of the preprocessed file. (ocaml/dune#10321, @anmonteiro) - Print the result of `dune describe pp` with the respective dialect printer. (ocaml/dune#10322, @anmonteiro) - Add new flag `--context` to `dune ocaml-merlin`, which allows to select a Dune context when requesting Merlin config. Add `dune describe contexts` subcommand. Introduce a field `generate_merlin_rules` for contexts declared in the workspace, that allows to optionally produce Merlin rules for other contexts besides the one selected for Merlin (ocaml/dune#10324, @jchavarri) - melange: add include paths for private library `.cmj` files during JS emission. (ocaml/dune#10416, @anmonteiro) - `dune ocaml-merlin`: communicate additional directives `SOURCE_ROOT`, `UNIT_NAME` (the actual name with wrapping) and `INDEX` with the paths to the index(es). (ocaml/dune#10422, @voodoos) - Add a new alias `@ocaml-index` that uses the `ocaml-index` binary to generate indexes that can be read by tools such as Merlin to provide project-wide references search. (ocaml/dune#10422, @voodoos) - merlin: add optional `(merlin_reader CMD)` construct to `(dialect)` stanza to configure a merlin reader (ocaml/dune#8567, @andreypopp) ### Changed - melange: treat private libraries with `(package ..)` as public libraries, fixing an issue where `import` paths were wrongly emitted. (ocaml/dune#10415, @anmonteiro) - install `.glob` files for Coq theories too (ocaml/dune#10602, @ejgallego) ### Fixed - Don't try to document non-existent libraries in doc-new target (ocaml/dune#10319, fixes ocaml/dune#10056, @jonludlam) - Make `dune-site`'s `load_all` function look for `META` files so that it doesn't fail on empty directories in the plugin directory (ocaml/dune#10458, fixes ocaml/dune#10457, @shym) - Fix incorrect warning for libraries defined inside non-existant directories using `(subdir ..)` and used by executables using `dune-build-info` (ocaml/dune#10525, @rgrinberg) - Don't try to take build lock when running `coq top --no-build` (ocaml/dune#10547, fixes ocaml/dune#7671, @lzy0505) - Make sure to truncate dune's lock file after locking and unlocking so that users cannot observe incorrect pid's (ocaml/dune#10575, @rgrinberg) - mdx: link mdx binary with `byte_complete`. This fixes `(libraries)` with foreign archives on Linux. (ocaml/dune#10586, fixes ocaml/dune#10582, @anmonteiro) - virtual libraries: fix an issue where linking an executable involving several virtual libries would cause an error. (ocaml/dune#10581, fixes ocaml/dune#10460, @rgrinberg)

@anmonteiro

CHANGES: ### Added - allow libraries with the same `(name ..)` in projects as long as they don't conflict during resolution (via `enabled_if`). (ocaml/dune#10307, @anmonteiro, @jchavarri) - `dune describe pp` now finds the exact module and the stanza it belongs to, instead of guessing the name of the preprocessed file. (ocaml/dune#10321, @anmonteiro) - Print the result of `dune describe pp` with the respective dialect printer. (ocaml/dune#10322, @anmonteiro) - Add new flag `--context` to `dune ocaml-merlin`, which allows to select a Dune context when requesting Merlin config. Add `dune describe contexts` subcommand. Introduce a field `generate_merlin_rules` for contexts declared in the workspace, that allows to optionally produce Merlin rules for other contexts besides the one selected for Merlin (ocaml/dune#10324, @jchavarri) - melange: add include paths for private library `.cmj` files during JS emission. (ocaml/dune#10416, @anmonteiro) - `dune ocaml-merlin`: communicate additional directives `SOURCE_ROOT`, `UNIT_NAME` (the actual name with wrapping) and `INDEX` with the paths to the index(es). (ocaml/dune#10422, @voodoos) - Add a new alias `@ocaml-index` that uses the `ocaml-index` binary to generate indexes that can be read by tools such as Merlin to provide project-wide references search. (ocaml/dune#10422, @voodoos) - merlin: add optional `(merlin_reader CMD)` construct to `(dialect)` stanza to configure a merlin reader (ocaml/dune#8567, @andreypopp) ### Changed - melange: treat private libraries with `(package ..)` as public libraries, fixing an issue where `import` paths were wrongly emitted. (ocaml/dune#10415, @anmonteiro) - install `.glob` files for Coq theories too (ocaml/dune#10602, @ejgallego) ### Fixed - Don't try to document non-existent libraries in doc-new target (ocaml/dune#10319, fixes ocaml/dune#10056, @jonludlam) - Make `dune-site`'s `load_all` function look for `META` files so that it doesn't fail on empty directories in the plugin directory (ocaml/dune#10458, fixes ocaml/dune#10457, @shym) - Fix incorrect warning for libraries defined inside non-existant directories using `(subdir ..)` and used by executables using `dune-build-info` (ocaml/dune#10525, @rgrinberg) - Don't try to take build lock when running `coq top --no-build` (ocaml/dune#10547, fixes ocaml/dune#7671, @lzy0505) - Make sure to truncate dune's lock file after locking and unlocking so that users cannot observe incorrect pid's (ocaml/dune#10575, @rgrinberg) - mdx: link mdx binary with `byte_complete`. This fixes `(libraries)` with foreign archives on Linux. (ocaml/dune#10586, fixes ocaml/dune#10582, @anmonteiro) - virtual libraries: fix an issue where linking an executable involving several virtual libries would cause an error. (ocaml/dune#10581, fixes ocaml/dune#10460, @rgrinberg)

anmonteiro requested review from emillon and rgrinberg March 28, 2024 05:03

rgrinberg reviewed Mar 28, 2024

View reviewed changes

anmonteiro force-pushed the anmonteiro/describe-pp-refmt branch 3 times, most recently from 33f1c35 to 909b6cc Compare March 28, 2024 21:41

rgrinberg requested a review from nojb March 28, 2024 22:05

rgrinberg approved these changes Mar 28, 2024

View reviewed changes

feat(describe-pp): use the dialect printer if available

3e4e152

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

anmonteiro force-pushed the anmonteiro/describe-pp-refmt branch from 491b4cf to 3e4e152 Compare March 28, 2024 22:16

rename dump_ast to print_ast

51d5e5b

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

nojb approved these changes Mar 29, 2024

View reviewed changes

anmonteiro enabled auto-merge (squash) March 29, 2024 21:59

Merge branch 'main' into anmonteiro/describe-pp-refmt

e57f34f

anmonteiro merged commit 122c022 into ocaml:main Mar 29, 2024

anmonteiro deleted the anmonteiro/describe-pp-refmt branch March 29, 2024 21:59

This was referenced Mar 30, 2024

fix(describe-pp): show output on successive runs #10340

Merged

dune describe pp creates an empty profile.dump file at the root of the project #7430

Closed

rgrinberg added this to the 3.16.0 milestone Apr 1, 2024

gridbugs mentioned this pull request Jun 5, 2024

[new release] dune (15 packages) (3.16.0~alpha1) ocaml/opam-repository#26022

Closed

emillon mentioned this pull request Jun 17, 2024

[new release] dune (15 packages) (3.16.0) ocaml/opam-repository#26101

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(describe-pp): print reason files with `refmt`#10322

feat(describe-pp): print reason files with `refmt`#10322
anmonteiro merged 3 commits intoocaml:mainfrom
anmonteiro:anmonteiro/describe-pp-refmt

anmonteiro commented Mar 28, 2024 •

edited

Loading

Uh oh!

rgrinberg Mar 28, 2024

Uh oh!

anmonteiro Mar 28, 2024 •

edited

Loading

Uh oh!

anmonteiro commented Mar 28, 2024

Uh oh!

rgrinberg left a comment

Uh oh!

anmonteiro commented Mar 28, 2024

Uh oh!

anmonteiro commented Mar 28, 2024

Uh oh!

nojb left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

anmonteiro commented Mar 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgrinberg Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

anmonteiro Mar 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anmonteiro commented Mar 28, 2024

Uh oh!

rgrinberg left a comment

Choose a reason for hiding this comment

Uh oh!

anmonteiro commented Mar 28, 2024

Uh oh!

anmonteiro commented Mar 28, 2024

Uh oh!

nojb left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anmonteiro commented Mar 28, 2024 •

edited

Loading

anmonteiro Mar 28, 2024 •

edited

Loading