Skip to content

Conversation

@Lucccyo
Copy link

@Lucccyo Lucccyo commented Jan 8, 2026

This PR aims to improve the readability of the signature of signature_help.
It applies now ocamlformat to the signature.

@Lucccyo Lucccyo force-pushed the sig_help_bad_readability branch from 56f1940 to 326091b Compare January 8, 2026 14:37
@Lucccyo
Copy link
Author

Lucccyo commented Jan 8, 2026

@voodoos, let's open the discussion on whether the format is added at the right moment.

@Lucccyo Lucccyo force-pushed the sig_help_bad_readability branch 2 times, most recently from c886ecd to 1ec1dc2 Compare January 8, 2026 15:41
@Lucccyo
Copy link
Author

Lucccyo commented Jan 9, 2026

@voodoos, what were your ideas for managing the shift in the offsets ?

@voodoos
Copy link
Collaborator

voodoos commented Jan 9, 2026

I think we should first try the naive option: roughly parse the resulting string, accumulating the offsets of the parameters we found ? They should be separated by -> and we should ignore labels.

@Lucccyo Lucccyo force-pushed the sig_help_bad_readability branch from 1ec1dc2 to 4c7592d Compare January 13, 2026 09:13
@Lucccyo
Copy link
Author

Lucccyo commented Jan 13, 2026

Why should we ignore labels ?

@voodoos
Copy link
Collaborator

voodoos commented Jan 13, 2026

Why should we ignore labels ?

Looking at the current behavior: you are right, we don't :-)

@Lucccyo Lucccyo force-pushed the sig_help_bad_readability branch from 4c7592d to 6262de2 Compare January 14, 2026 13:25
@Lucccyo Lucccyo force-pushed the sig_help_bad_readability branch 2 times, most recently from 2aeaece to fce35f2 Compare January 16, 2026 09:42
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.

Looks good to me, the parsing function could probably be made a bit more efficient but it doesn't feel necessary right now.

@Lucccyo Lucccyo force-pushed the sig_help_bad_readability branch from fce35f2 to 153a23c Compare February 2, 2026 10:58
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.

2 participants