Skip to content

Conversation

BurntSushi
Copy link
Member

We achieve this by setting the "sort text" field of every completion.
Since we are trying to be smart about the order, we want the client to
respect our order.

Prior to this change, VS Code was re-sorting completions in
lexicographic order. This in turn resulted in dunder attributes
appearing before "normal" attributes.

@BurntSushi
Copy link
Member Author

Demo:

vs-code-completion-order.mp4

@BurntSushi
Copy link
Member Author

I wonder if this means we can revert #18480? cc @MichaReiser

Copy link
Contributor

github-actions bot commented Jun 24, 2025

mypy_primer results

No ecosystem changes detected ✅

@MichaReiser
Copy link
Member

MichaReiser commented Jun 24, 2025

I wonder if this means we can revert #18480? cc @MichaReiser

I don't think we can because this and #18480 do the same at the API boundary (but they're different boundaries). We could add the sortText to the CompletionItem that we return internally so that we don't need to repeat the implementation twice, but I don't think that's worth it.

rust analyzer does the same, but in a little fancier way:

https://github.com/rust-lang/rust-analyzer/blob/59ef84506deb45e6d4336935d0d4be54c3386814/crates/rust-analyzer/src/lsp/to_proto.rs#L455-L471

@MichaReiser MichaReiser added server Related to the LSP server ty Multi-file analysis & type inference labels Jun 24, 2025
@BurntSushi BurntSushi force-pushed the ag/completion-vs-code-sort branch from 087d42d to b9cc19d Compare June 24, 2025 15:14
We achieve this by setting the "sort text" field of every completion.
Since we are trying to be smart about the order, we want the client to
respect our order.

Prior to this change, VS Code was re-sorting completions in
lexicographic order. This in turn resulted in dunder attributes
appearing before "normal" attributes.
@BurntSushi BurntSushi force-pushed the ag/completion-vs-code-sort branch from b9cc19d to b96a890 Compare June 24, 2025 15:19
@AlexWaygood
Copy link
Member

you probably want reviews from Micha and/or Dhruv when it comes to the bits of autocompletion that involve actually talking to the LSP client -- the only LSP I know anything about is the Liskov Substitution Principle 😛

@AlexWaygood AlexWaygood removed their request for review June 24, 2025 15:24
@BurntSushi BurntSushi merged commit 1461137 into main Jun 24, 2025
36 checks passed
@BurntSushi BurntSushi deleted the ag/completion-vs-code-sort branch June 24, 2025 15:31
dcreager added a commit that referenced this pull request Jun 24, 2025
* main:
  [ty] Fix false positives when subscripting an object inferred as having an `Intersection` type (#18920)
  [`flake8-use-pathlib`] Add autofix for `PTH202` (#18763)
  [ty] Add relative import completion tests
  [ty] Clarify what "cursor" means
  [ty] Add a cursor test builder
  [ty] Enforce sort order of completions (#18917)
  [formatter] Fix missing blank lines before decorated classes in .pyi files (#18888)
  Apply fix availability and applicability when adding to `DiagnosticGuard` and remove `NoqaCode::rule` (#18834)
  py-fuzzer: allow relative executable paths (#18915)
  [ty] Change `environment.root` to accept multiple paths (#18913)
  [ty] Rename `src.root` setting to `environment.root` (#18760)
  Use file path for detecting package root (#18914)
  Consider virtual path for various server actions (#18910)
  [ty] Introduce `UnionType::try_from_elements` and `UnionType::try_map` (#18911)
  [ty] Support narrowing on `isinstance()`/`issubclass()` if the second argument is a dynamic, intersection, union or typevar type (#18900)
  [ty] Add decorator check for implicit attribute assignments (#18587)
  [`ruff`] Trigger `RUF037` for empty string and byte strings (#18862)
  [ty] Avoid duplicate diagnostic in unpacking (#18897)
  [`pyupgrade`] Extend version detection to include `sys.version_info.major` (`UP036`) (#18633)
  [`ruff`] Frozen Dataclass default should be valid (`RUF009`) (#18735)
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.

3 participants