Skip to content
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

TS Server API proposal: Commit Characters #27623

Closed
mjbvz opened this issue Oct 8, 2018 · 3 comments · Fixed by #59339
Closed

TS Server API proposal: Commit Characters #27623

mjbvz opened this issue Oct 8, 2018 · 3 comments · Fixed by #59339
Assignees
Labels
Committed The team has roadmapped this issue Domain: TSServer Issues related to the TSServer Fix Available A PR has been opened for this issue In Discussion Not yet reached consensus Rescheduled This issue was previously scheduled to an earlier milestone Suggestion An idea for TypeScript

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Oct 8, 2018

Problem

VS Code (and Visual Studio) have the concept of commit characters for suggestions. This is a set of characters that, when typed, accept the current suggestion. For example, a suggestion for a function may treat ( as a commit character.

VS Code currently computes these commit characters itself using the metadata that typescript returns for each completion entry. This has two problems:

  • This logic is VS Code specific and may be out of sync with how VS and other editors handle commit characters.

  • There are some cases where it is difficult to determine what the commit characters should be. A classic example is:

const a = {
    b: 1,
    .|
}

Where the user has just typed .. Currently we show a suggestion for b when the user types the .. If the user then types another . as part of a spread, we incorrectly end up accepting that completion, which leaves the code as:

const a = {
    b: 1,
    .b.
}

TypeScript, with its access to the ast, should be able to determine that . should not be a commit character in this case

Proposal

Introduce the concept of a commit characters to the TypeScript server aAPI. This would be an optional set of characters returned on each CompletionEntry:

 interface CompletionEntry {
     ...

     commitCharacters?: ReadonlyArray<string>;
}

Return these commit characters on completions. Here's an approximation of how VS Code computes commit characters:

  • . and ; are commit characters for most completion types except for keywords and a few others.
  • Additionally, ( and , are commit characters for variables.
  • There are no commit characters if isNewIdentifierLocation is set.
@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus Domain: TSServer Issues related to the TSServer labels Oct 9, 2018
@mjbvz
Copy link
Contributor Author

mjbvz commented Oct 15, 2018

If there is concern about the cost of computing the commit characters or about returning them on every suggestion, we can return them on CompletionEntryDetails instead.

@DanielRosenwasser
Copy link
Member

Something to keep in mind (which I think we've discussed) is the fact that some of the motivating scenarios are just plain bugs (e.g. maybe we shouldn't be providing completions after . in an object literal). Maybe #31244 is a better one, but it doesn't seem like the rules listed here would apply to that.

@RyanCavanaugh RyanCavanaugh added Committed The team has roadmapped this issue and removed In Discussion Not yet reached consensus labels May 5, 2021
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.4.0 milestone May 5, 2021
@RyanCavanaugh RyanCavanaugh assigned orta and armanio123 and unassigned orta May 5, 2021
@andrewbranch andrewbranch added the Rescheduled This issue was previously scheduled to an earlier milestone label Aug 30, 2021
@armanio123
Copy link
Member

Closing issue. As discussed offline, we lean towards consistency on the commit characters and gather more user data if further changes are necessary.

Check commit microsoft/vscode@76885d7 for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue Domain: TSServer Issues related to the TSServer Fix Available A PR has been opened for this issue In Discussion Not yet reached consensus Rescheduled This issue was previously scheduled to an earlier milestone Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants