[ty] Default ty.inlayHints.* server settings to true
#19910
Conversation
dhruvmanila
requested review from
carljm,
MichaReiser,
sharkdp and
dcreager
as code owners
dhruvmanila
added
internal
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-08-14 08:33:10.270535018 +0000
+++ new-output.txt 2025-08-14 08:33:10.336535119 +0000
@@ -1,5 +1,5 @@
WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors.
-fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/918d35d/src/function/execute.rs:215:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(18ab)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/918d35d/src/function/execute.rs:215:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(48c8)): execute: too many cycle iterations`
_directives_deprecated_library.py:15:31: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `int`
_directives_deprecated_library.py:30:26: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `str`
_directives_deprecated_library.py:36:41: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Self@__add__` |
|
| impl Default for InlayHintSettings { | ||
| fn default() -> Self { | ||
| Self { | ||
| variable_types: true, | ||
| function_argument_names: true, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not entirely sure about enabling all of them by default. E.g. pylance all inlays by default (which I'm also not sure if that's great). We probably want something in between where we enable the non noisy inlays by default but keep all others disabled.
Having said that. I think defaulting those two settings to true makes sense.
I assume the new default implementation matches the defaults in the VS code settings?
There was a problem hiding this comment.
Hmm, good point.
I think I have it backwards. Both of them are disabled by default in Pylance.
There was a problem hiding this comment.
As discussed in our 1:1, we've decided to keep it enabled by default, and can change it in the future before GA based on user feedback.
There was a problem hiding this comment.
Hmm, I'm a bit surprised by this change, and a bit unhappy with that resolution. We discussed this a fair bit as a team in the original PR adding inlay hints (#17214 (comment), and there was also discussion on Discord as well IIRC), and I thought we came to a consensus that we should keep them disabled by default
There was a problem hiding this comment.
Oh, I think I forgot about that discussion, thanks for the reminder. I'll revert this change back for now.
There was a problem hiding this comment.
I think it's a good call out that we should think about why we enable which inlays by default and we can create an issue to revisit this. In the meantime, I'd prefer to defer this discussion to later.
There was a problem hiding this comment.
That sounds good, I can create an issue and proceed with the release as usual.
There was a problem hiding this comment.
I'm afraid I'm still of the opinion that this is a change that I would definitely not want to see in my editor with what I understand the current state of this feature as being; that it would take me some time to figure out how to change the settings in VSCode (I can never find my way to the right setting on the rare occasion I need to change something); that this is going to be quite surprising to users coming from Pylance; and that this is going to negatively impact most users of the ty server.
There was a problem hiding this comment.
Just for context, the variable type hints were enabled by default when it got added and it only got disabled by default in #19780 which I did not intend to make. That was the main motivation of this PR which is to turn it back on to return the behavior in the released ty version.
There was a problem hiding this comment.
Just for context, the variable type hints were enabled by default when it got added and it only got disabled by default in #19780 which I did not intend to make. That was the main motivation of this PR which is to turn it back on to return the behavior in the released ty version.
Ahhh, this I did not realise. I'm definitely fine with restoring the status quo, in that case, and continuing the discussion in an issue elsewhere.
* main: Feature/build riscv64 bin (#19819) [ty] Add caching to `CodeGeneratorKind::matches()` (#19912) [ty] Rename `functionArgumentNames` to `callArgumentNames` inlay hint setting (#19911) [ty] Default `ty.inlayHints.*` server settings to true (#19910) [ty] Remove py-fuzzer skips for seeds that are no longer slow (#19906) [ty] fix deferred name loading in PEP695 generic classes/functions (#19888) [ty] Add some additional type safety to `CycleDetector` (#19903) [`flake8-blind-except`] Fix `BLE001` false-positive on `raise ... from None` (#19755) [ty] resolve docstrings for modules (#19898)
Summary
This PR changes the default of
ty.inlayHints.*settings totrue.I somehow missed this in my initial PR.
This is marked as
internalbecause it's not yet released.