Skip to content

Type Enclosing Command #14

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

Merged
merged 13 commits into from
Jan 14, 2025
Merged

Type Enclosing Command #14

merged 13 commits into from
Jan 14, 2025

Conversation

xvw
Copy link
Member

@xvw xvw commented Jan 10, 2025

The branch exposes the ocaml-eglot-type-enclosing function, which displays the type of the expression under the cursor.

  • ocaml-eglot-type-enclosing-increase-verbosity (C-c
    C-t): to increase the verbosity of the type observed
  • ocaml-eglot-type-enclosing-grow (C-↑): to grow the
    expression
  • ocaml-eglot-type-enclosing-shrink (C-↓): to shrink the
    expression
  • ocaml-eglot-type-enclosing-copy (C-w): to copy the
    type expression to the kill-ring (clipboard)

Decisions

At present, the control of the number of fines is reset to zero each time the enclosures are varied. However calling C-c C-t will increase the verbosity on the current enclosing type. It's easily possible to maintain verbosity control by ‘requests’ if you think that's better (which I don't).

More

Currently, there are no queries to ask for the type of an identifier (initially usable with the prefix argument in Merlin, ie: C-u C-c C-t will ask for an identifier, List.map and will display ('a -> “b) -> ”a list -> ’b list.

But as it's possible to search by type, I'm not sure that the feature is ‘so necessary’.

@xvw xvw force-pushed the type-enclosing-final branch 2 times, most recently from c624581 to 4a3ff5c Compare January 10, 2025 17:55
@xvw xvw requested a review from voodoos January 10, 2025 17:55
@gr-im
Copy link

gr-im commented Jan 12, 2025

At present, the control of the number of fines is reset to zero each time the enclosures are varied

WDYT about keeping verbosity from enclosings to enclosings? And restoring it to 0 only when re-triggering the command? (and offering a transient binding for C-<left> and C-<right> to increase/decrease verbosity for current enclosing?)

@voodoos
Copy link
Member

voodoos commented Jan 13, 2025

I had similar thoughts when integrating the feature in vscode and I don't have a strong preference:

  • The current behavior is the same as the existing client (I think), so memory muscle will be rewarded.
  • The proposed 2D navigation scheme with the arrows and verbosity memory does seems nicer.

I mean, it's probably the best time to introduce this kind of UX changes, as long as we keep the "multiple presses on C-t augments verbosity"...

@xvw
Copy link
Member Author

xvw commented Jan 13, 2025

@gr-im, @voodoos
Behaviour appended on 48c95e0

Copy link
Member

@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.

Just some doc comments right now, I will try to do a more in depth-review tomorrow.

We should really pay attention to the docstring of interactive functions :-)

@xvw xvw force-pushed the type-enclosing-final branch from 05da9e5 to c53d9a2 Compare January 14, 2025 15:40
Copy link
Member

@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.

Thanks, looks good to me. I have one last question about the handling of verbosity...

Copy link

@gr-im gr-im left a comment

Choose a reason for hiding this comment

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

Thank you so much for considering my suggestion for the verbosity, it really will be more ergonomic!
In an ideal world, it would be nice to limit it when it calculates a type identical to the previous one (so we don't have to go back too far) but I think that can be left to another PR?

@xvw xvw merged commit 87f62b1 into main Jan 14, 2025
4 checks passed
@xvw xvw deleted the type-enclosing-final branch January 14, 2025 16:01
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