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.

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

Elisp: make merlin-construct use the user's configured completion interface #1598

Merged
merged 3 commits into from
May 10, 2023

Conversation

bcc32
Copy link
Contributor

@bcc32 bcc32 commented May 2, 2023

The current merlin-construct interface is somewhat awkward. It requires the user to switch buffers, and forces the use of the old-style built-in completion buffer, rather than using whatever completion interface the user has set up (e.g., ivy, helm).

Also make the following small improvements:

  1. Fix some docstring typos
  2. Clean up unused variable
  3. Signal an error if the hole has no constructors, instead of just presenting an empty list of completions

@voodoos voodoos self-requested a review May 5, 2023 18:21
@voodoos
Copy link
Collaborator

voodoos commented May 9, 2023

Thank you, the code is much cleaner that way !

@3Rafal could you test it on your setup ?

@3Rafal
Copy link
Collaborator

3Rafal commented May 10, 2023

@voodoos , it works on my machine™ 😉

thanks @bcc32 for this contribution! your implementation works so much better 🙌🏻

Copy link
Collaborator

@voodoos voodoos left a comment

Choose a reason for hiding this comment

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

LGTM.

Could you add a changelog entry ?

bcc32 added 3 commits May 10, 2023 11:13
The POINT argument was indeed unused, as `point` is distinct from
`(point)`.
Rather than directly constructing a completion buffer the way the
default `completion--in-region` function does, use `completing-read`
to select a candidate constructor.  This will respect the user's
configuration of `completing-read-function`.

This also changes merlin-construct to signal an error if there are no
candidate constructors.
@bcc32 bcc32 force-pushed the construct-completing-read branch from cc0fa32 to 0810505 Compare May 10, 2023 15:15
@bcc32
Copy link
Contributor Author

bcc32 commented May 10, 2023

Done, and rebased. Thanks!

@voodoos voodoos merged commit 4bd7b7f into ocaml:master May 10, 2023
@bcc32 bcc32 deleted the construct-completing-read branch May 10, 2023 19:59
voodoos added a commit to voodoos/merlin that referenced this pull request May 26, 2023
voodoos added a commit to voodoos/opam-repository that referenced this pull request May 26, 2023
CHANGES:

Fri May 26 15:23:42 CEST 2023

  + merlin binary
    - Allow monadic IO in dot protocol (ocaml/merlin#1581)
    - Add a `scope` option to the `occurrences` command in preparation for
      the upcoming `project-wide-occurrences` feature (ocaml/merlin#1596)
    - Construct bool-typed holes as `false` instead of `true` in the
      `construct` command, for consistency (ocaml/merlin#1599).
    - Add a hook to configure system command for spawning ppxes when Merlin is
      used as a library. (ocaml/merlin#1585)
    - Implement an all-or-nothing cache for the PPX phase (ocaml/merlin#1584)
    - Cleanup functors caches when backtracking, to avoid memory leaks
      (ocaml/merlin#1609, fixes ocaml/merlin#1529 and ocaml-lsp#1032)
    - Fix `construct` results ordering for sum types sand poly variants (ocaml/merlin#1603)
    - Fix object method completion not working (ocaml/merlin#1606, fixes ocaml/merlin#1575)
    - Improve context detection for package types (ocaml/merlin#1608, fixes ocaml/merlin#1607)
    - Fix incorrect locations for string literals (ocaml/merlin#1574)
    - Fixed an issue that caused `errors` to erroneously alert about missing
      `cmi` files (ocaml/merlin#1577)
    - Prevent destruct from crashing on closed variant types (ocaml/merlin#1602,
      fixes ocaml/merlin#1601)
    - Improve longident parsing (ocaml/merlin#1612, fixes ocaml/merlin#945)
  + editor modes
    - emacs: call the user's configured completion UI in
      `merlin-construct` (ocaml/merlin#1598)
  + test suite
    - Add missing dependency to a test using ppxlib (ocaml/merlin#1583)
    - Add tests for the new PPX phase cache (ocaml/merlin#1584)
    - Add and update tests for `construct` ordering (ocaml/merlin#1603)
voodoos added a commit to voodoos/opam-repository that referenced this pull request May 26, 2023
CHANGES:

Fri May 26 15:23:42 CEST 2023

  + merlin binary
    - Allow monadic IO in dot protocol (ocaml/merlin#1581)
    - Add a `scope` option to the `occurrences` command in preparation for
      the upcoming `project-wide-occurrences` feature (ocaml/merlin#1596)
    - Construct bool-typed holes as `false` instead of `true` in the
      `construct` command, for consistency (ocaml/merlin#1599).
    - Add a hook to configure system command for spawning ppxes when Merlin is
      used as a library. (ocaml/merlin#1585)
    - Implement an all-or-nothing cache for the PPX phase (ocaml/merlin#1584)
    - Cleanup functors caches when backtracking, to avoid memory leaks
      (ocaml/merlin#1609, fixes ocaml/merlin#1529 and ocaml-lsp#1032)
    - Fix `construct` results ordering for sum types sand poly variants (ocaml/merlin#1603)
    - Fix object method completion not working (ocaml/merlin#1606, fixes ocaml/merlin#1575)
    - Improve context detection for package types (ocaml/merlin#1608, fixes ocaml/merlin#1607)
    - Fix incorrect locations for string literals (ocaml/merlin#1574)
    - Fixed an issue that caused `errors` to erroneously alert about missing
      `cmi` files (ocaml/merlin#1577)
    - Prevent destruct from crashing on closed variant types (ocaml/merlin#1602,
      fixes ocaml/merlin#1601)
    - Improve longident parsing (ocaml/merlin#1612, fixes ocaml/merlin#945)
  + editor modes
    - emacs: call the user's configured completion UI in
      `merlin-construct` (ocaml/merlin#1598)
  + test suite
    - Add missing dependency to a test using ppxlib (ocaml/merlin#1583)
    - Add tests for the new PPX phase cache (ocaml/merlin#1584)
    - Add and update tests for `construct` ordering (ocaml/merlin#1603)
voodoos added a commit to voodoos/opam-repository that referenced this pull request May 26, 2023
CHANGES:

Fri May 26 15:23:42 CEST 2023

  + merlin binary
    - Allow monadic IO in dot protocol (ocaml/merlin#1581)
    - Add a `scope` option to the `occurrences` command in preparation for
      the upcoming `project-wide-occurrences` feature (ocaml/merlin#1596)
    - Construct bool-typed holes as `false` instead of `true` in the
      `construct` command, for consistency (ocaml/merlin#1599).
    - Add a hook to configure system command for spawning ppxes when Merlin is
      used as a library. (ocaml/merlin#1585)
    - Implement an all-or-nothing cache for the PPX phase (ocaml/merlin#1584)
    - Cleanup functors caches when backtracking, to avoid memory leaks
      (ocaml/merlin#1609, fixes ocaml/merlin#1529 and ocaml-lsp#1032)
    - Fix `construct` results ordering for sum types sand poly variants (ocaml/merlin#1603)
    - Fix object method completion not working (ocaml/merlin#1606, fixes ocaml/merlin#1575)
    - Improve context detection for package types (ocaml/merlin#1608, fixes ocaml/merlin#1607)
    - Fix incorrect locations for string literals (ocaml/merlin#1574)
    - Fixed an issue that caused `errors` to erroneously alert about missing
      `cmi` files (ocaml/merlin#1577)
    - Prevent destruct from crashing on closed variant types (ocaml/merlin#1602,
      fixes ocaml/merlin#1601)
    - Improve longident parsing (ocaml/merlin#1612, fixes ocaml/merlin#945)
  + editor modes
    - emacs: call the user's configured completion UI in
      `merlin-construct` (ocaml/merlin#1598)
  + test suite
    - Add missing dependency to a test using ppxlib (ocaml/merlin#1583)
    - Add tests for the new PPX phase cache (ocaml/merlin#1584)
    - Add and update tests for `construct` ordering (ocaml/merlin#1603)

[new release] merlin, merlin-lib and dot-merlin-reader (4.9-414)

CHANGES:

Fri May 26 15:23:42 CEST 2023

  + merlin binary
    - Allow monadic IO in dot protocol (ocaml/merlin#1581)
    - Add a `scope` option to the `occurrences` command in preparation for
      the upcoming `project-wide-occurrences` feature (ocaml/merlin#1596)
    - Construct bool-typed holes as `false` instead of `true` in the
      `construct` command, for consistency (ocaml/merlin#1599).
    - Add a hook to configure system command for spawning ppxes when Merlin is
      used as a library. (ocaml/merlin#1585)
    - Implement an all-or-nothing cache for the PPX phase (ocaml/merlin#1584)
    - Cleanup functors caches when backtracking, to avoid memory leaks
      (ocaml/merlin#1609, fixes ocaml/merlin#1529 and ocaml-lsp#1032)
    - Fix `construct` results ordering for sum types sand poly variants (ocaml/merlin#1603)
    - Fix object method completion not working (ocaml/merlin#1606, fixes ocaml/merlin#1575)
    - Improve context detection for package types (ocaml/merlin#1608, fixes ocaml/merlin#1607)
    - Fix incorrect locations for string literals (ocaml/merlin#1574)
    - Fixed an issue that caused `errors` to erroneously alert about missing
      `cmi` files (ocaml/merlin#1577)
    - Prevent destruct from crashing on closed variant types (ocaml/merlin#1602,
      fixes ocaml/merlin#1601)
    - Improve longident parsing (ocaml/merlin#1612, fixes ocaml/merlin#945)
  + editor modes
    - emacs: call the user's configured completion UI in
      `merlin-construct` (ocaml/merlin#1598)
  + test suite
    - Add missing dependency to a test using ppxlib (ocaml/merlin#1583)
    - Add tests for the new PPX phase cache (ocaml/merlin#1584)
    - Add and update tests for `construct` ordering (ocaml/merlin#1603)
voodoos added a commit to voodoos/opam-repository that referenced this pull request May 31, 2023
CHANGES:

unreleased

  + merlin binary
    - Preview support for OCaml 5.1-alpha1. Short path is temporary disabled and
      inline records might not behave as expected.
    - Allow monadic IO in dot protocol (ocaml/merlin#1581)
    - Add a `scope` option to the `occurrences` command in preparation for
      the upcoming `project-wide-occurrences` feature (ocaml/merlin#1596)
    - Construct bool-typed holes as `false` instead of `true` in the
      `construct` command, for consistency (ocaml/merlin#1599).
    - Add a hook to configure system command for spawning ppxes when Merlin is
      used as a library. (ocaml/merlin#1585)
    - Implement an all-or-nothing cache for the PPX phase (ocaml/merlin#1584)
    - Cleanup functors caches when backtracking, to avoid memory leaks
      (ocaml/merlin#1609, fixes ocaml/merlin#1529 and ocaml-lsp#1032)
    - Fix `construct` results ordering for sum types sand poly variants (ocaml/merlin#1603)
    - Fix object method completion not working (ocaml/merlin#1606, fixes ocaml/merlin#1575)
    - Improve context detection for package types (ocaml/merlin#1608, fixes ocaml/merlin#1607)
    - Fix incorrect locations for string literals (ocaml/merlin#1574)
    - Fixed an issue that caused `errors` to erroneously alert about missing
      `cmi` files (ocaml/merlin#1577)
    - Prevent destruct from crashing on closed variant types (ocaml/merlin#1602,
      fixes ocaml/merlin#1601)
    - Improve longident parsing (ocaml/merlin#1612, fixes ocaml/merlin#945)
  + editor modes
    - emacs: call the user's configured completion UI in
      `merlin-construct` (ocaml/merlin#1598)
  + test suite
    - Add missing dependency to a test using ppxlib (ocaml/merlin#1583)
    - Add tests for the new PPX phase cache (ocaml/merlin#1584)
    - Add and update tests for `construct` ordering (ocaml/merlin#1603)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants