Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve functor printing. #2683

Merged
merged 11 commits into from Apr 19, 2023
Merged

Improve functor printing. #2683

merged 11 commits into from Apr 19, 2023

Conversation

SanderSpies
Copy link
Contributor

@SanderSpies SanderSpies commented Feb 23, 2023 •
edited

Reduces the amount of parentheses around functor usage.

Related to: #2682.

@SanderSpies SanderSpies changed the title Improve empty functor printing. Improve functor printing. Feb 23, 2023
@SanderSpies SanderSpies marked this pull request as ready for review February 23, 2023 12:19
@anmonteiro
Copy link
Member

Thanks, looks good. Can you add an entry to HISTORY.md?

@SanderSpies
Copy link
Contributor Author

@anmonteiro I added a history entry. I hope 3.9 makes sense as a next version.

@SanderSpies
Copy link
Contributor Author

OCaml 4.03 CI is having problems with installing nodejs and npm.

davesnx
davesnx approved these changes Mar 28, 2023
View reviewed changes
Copy link
Member

@davesnx davesnx left a comment

Choose a reason for hiding this comment

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

There are a bunch of conflicts regarding old tests, It should be safe to rebase and move those tests cases into the test/ folder

src/reason-parser/reason_pprint_ast.ml Outdated
Comment on lines 7557 to 7558
makeList
~break:Layout.Always_rec
~break:(if List.length s = 0 then Layout.IfNeed else Layout.Always_rec)
Copy link
Member

Choose a reason for hiding this comment

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

I have seen this logic in more than a few places, maybe we could try to do this check inside makeList and see the snapshots?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies for the late reply. I've added this to the changes of this PR.

@SanderSpies
Merge branch 'master' into sanderspies/fix_2682
79387e0 
* master: (38 commits)
  chore: remove old BS_NO_COMPILER_PATCH flag (reasonml#2710)
  Improve printing of modules types with one line inside (reasonml#2709)
  generate opam files with dune (reasonml#2704)
  Fix version on refmt (reasonml#2701)
  Drop the result dependency (reasonml#2703)
  Remove old CI and test.sh (reasonml#2705)
  Migrate tests to cram suite (reasonml#2694)
  Make sure win doesnt break when importing (reasonml#2700)
  Fix top level extensions (reasonml#2693)
  Install before importing deps
  Update package json and install esy normally
  Re-arrange esy install
  Reduce install esy time
  Use master branch instead of main
  Ignore _export from esy
  Rename refmt_test to test
  Add esy-ci and opam-ci
  Remove jbuild-ignore
  chore: update nix flake (reasonml#2692)
  chore(readme): clarify 3.9 is unreleased
  ...
@anmonteiro anmonteiro merged commit 82dd9aa into reasonml:master Apr 19, 2023
24 checks passed
davesnx added a commit that referenced this pull request Apr 25, 2023
@davesnx
Merge branch 'master' of github.com:/reasonml/reason
da042bc 
* 'master' of github.com:/reasonml/reason:
  chore: port to ppxlib (#2711)
  fix: binary parser (#2713)
  Improve functor printing. (#2683)
  chore: remove old BS_NO_COMPILER_PATCH flag (#2710)
  Improve printing of modules types with one line inside (#2709)
  generate opam files with dune (#2704)
SanderSpies added a commit to SanderSpies/reason that referenced this pull request May 4, 2023
@SanderSpies
Merge branch 'master' into sanderspies/pprint
8a113c9 
* master:
  fix: binary parser (reasonml#2713)
  Improve functor printing. (reasonml#2683)
  chore: remove old BS_NO_COMPILER_PATCH flag (reasonml#2710)
  Improve printing of modules types with one line inside (reasonml#2709)
  generate opam files with dune (reasonml#2704)
  Fix version on refmt (reasonml#2701)
  Drop the result dependency (reasonml#2703)
  Remove old CI and test.sh (reasonml#2705)
  Migrate tests to cram suite (reasonml#2694)
  Make sure win doesnt break when importing (reasonml#2700)
  Fix top level extensions (reasonml#2693)
  Install before importing deps
  Update package json and install esy normally
  Re-arrange esy install
  Reduce install esy time
  Use master branch instead of main
  Ignore _export from esy
  Rename refmt_test to test
  Add esy-ci and opam-ci
  Remove jbuild-ignore
@anmonteiro anmonteiro mentioned this pull request Jun 17, 2023
Closed
anmonteiro added a commit to anmonteiro/opam-repository that referenced this pull request Jun 17, 2023
@anmonteiro
[new release] rtop and reason (3.9.0)
7c1c414 
CHANGES:

- Reduce the amount of parentheses around functor usage (@SanderSpies, [reasonml/reason#2683](reasonml/reason#2683))
- Print module type body on separate line (@SanderSpies, [reasonml/reason#2709](reasonml/reason#2709))
- Fix missing patterns around contraint pattern (a pattern with a type annotation).
- Fix top level extension printing
- Remove the dependency on the `result` package, which isn't needed for OCaml
  4.03 and above (@anmonteiro) [reasonml/reason#2703](reasonml/reason#2703)
- Fix the binary parser by converting to the internal AST version used by
  Reason (@anmonteiro) [reasonml/reason#2713](reasonml/reason#2713)
- Port Reason to `ppxlib` (@anmonteiro, [reasonml/reason#2711](reasonml/reason#2711))
- Support OCaml 5.1 (@anmonteiro, [reasonml/reason#2714](reasonml/reason#2714))
@anmonteiro anmonteiro mentioned this pull request Jun 18, 2023
Merged
anmonteiro added a commit to anmonteiro/opam-repository that referenced this pull request Jun 18, 2023
@anmonteiro
[new release] rtop and reason (3.9.0)
f094160 
CHANGES:

- Reduce the amount of parentheses around functor usage (@SanderSpies, [reasonml/reason#2683](reasonml/reason#2683))
- Print module type body on separate line (@SanderSpies, [reasonml/reason#2709](reasonml/reason#2709))
- Fix missing patterns around contraint pattern (a pattern with a type annotation).
- Fix top level extension printing
- Remove the dependency on the `result` package, which isn't needed for OCaml
  4.03 and above (@anmonteiro) [reasonml/reason#2703](reasonml/reason#2703)
- Fix the binary parser by converting to the internal AST version used by
  Reason (@anmonteiro) [reasonml/reason#2713](reasonml/reason#2713)
- Port Reason to `ppxlib` (@anmonteiro, [reasonml/reason#2711](reasonml/reason#2711))
- Support OCaml 5.1 (@anmonteiro, [reasonml/reason#2714](reasonml/reason#2714))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anmonteiro anmonteiro anmonteiro left review comments

@davesnx davesnx davesnx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@SanderSpies @anmonteiro @davesnx