Skip to content

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Aug 20, 2025

This is a fairly simple but effective way to add docstrings to like 95% of completions from initial experimentation.

Fixes astral-sh/ty#1036

Although ironically this approach does not work specifically for print and I haven't looked into why.

@Gankra Gankra requested a review from carljm as a code owner August 20, 2025 17:49
@Gankra Gankra added the server Related to the LSP server label Aug 20, 2025
@Gankra Gankra added the ty Multi-file analysis & type inference label Aug 20, 2025
Copy link
Contributor

github-actions bot commented Aug 20, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

Copy link
Contributor

github-actions bot commented Aug 20, 2025

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

@Gankra
Copy link
Contributor Author

Gankra commented Aug 20, 2025

Screen.Recording.2025-08-20.at.1.52.03.PM.mov

@@ -421,7 +421,7 @@ impl Workspace {
.into_iter()
.map(|completion| Completion {
kind: completion.kind(&self.db).map(CompletionKind::from),
name: completion.name.into(),
name: completion.inner.name.into(),
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to also hook up documentation in the playground. You have to add a new documentation field to Completion, then propagate the field here

insertText: completion.name,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think done? Never tested the playground.

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Nice


pub struct DetailedCompletion<'db> {
pub inner: Completion<'db>,
pub documentation: Option<Docstring>,
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this a field on Completion instead that defaults to None: Given that completion is the only method constructing the completions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently that doesn't work out because Docstring is defined in ty_ide and Completion is defined in ty_python_semantic. The crate boundaries have gotten quite wobbly 😅

@Gankra Gankra merged commit 859475f into main Aug 20, 2025
38 checks passed
@Gankra Gankra deleted the gankra/doc-comp branch August 20, 2025 21:00
dcreager added a commit that referenced this pull request Aug 21, 2025
* main: (29 commits)
  [ty] add docstrings to completions based on type (#20008)
  [`pyupgrade`] Avoid reporting `__future__` features as unnecessary when they are used (`UP010`) (#19769)
  [`flake8-use-pathlib`] Add fixes for `PTH102` and `PTH103` (#19514)
  [ty] correctly ignore field specifiers when not specified (#20002)
  `Option::unwrap` is now const (#20007)
  [ty] Re-arrange "list modules" implementation for Salsa caching
  [ty] Test "list modules" versus "resolve module" in every mdtest
  [ty] Wire up "list modules" API to make module completions work
  [ty] Tweak some completion tests
  [ty] Add "list modules" implementation
  [ty] Lightly expose `FileModule` and `NamespacePackage` fields
  [ty] Add some more helper routines to `ModulePath`
  [ty] Fix a bug when converting `ModulePath` to `ModuleName`
  [ty] Split out another constructor for `ModuleName`
  [ty] Add stub-file tests to existing module resolver
  [ty] Expose some routines in the module resolver
  [ty] Add more path helper functions
  [`flake8-annotations`] Remove unused import in example (`ANN401`) (#20000)
  [ty] distinguish base conda from child conda (#19990)
  [ty] Fix server hang (#19991)
  ...
second-ed pushed a commit to second-ed/ruff that referenced this pull request Sep 9, 2025
This is a fairly simple but effective way to add docstrings to like 95%
of completions from initial experimentation.

Fixes astral-sh/ty#1036

Although ironically this approach *does not* work specifically for
`print` and I haven't looked into why.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Related to the LSP server ty Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return documentation with CompletionItem
2 participants