Skip to content

Autocomplete Feature #282

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

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Autocomplete Feature #282

wants to merge 31 commits into from

Conversation

FastestMolasses
Copy link
Member

@FastestMolasses FastestMolasses commented Dec 18, 2024

Description

Adds the first iteration of the autocomplete feature.

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

(Khan) From the demo completion provider:
Screenshot 2025-07-24 at 2 29 06 PM
Screenshot 2025-07-24 at 2 28 58 PM

@FastestMolasses FastestMolasses marked this pull request as ready for review December 29, 2024 13:12
@dankinsoid
Copy link

@FastestMolasses hi! I forked from your branch and found a crash:
StyledRangeStore+Internals.swift line 44
_guts[_guts.index(after: index)] fails with out of bounds error

this check fixes:
let nextIndex = _guts.index(after: index)
guard nextIndex < _guts.endIndex else { return }

@FastestMolasses
Copy link
Member Author

This might have been a bug that was fixed in a later commit. I'll merge main into this soon and check it out, this PR is currently on hold until the Language Server Installation PR gets merged.

@tom-ludwig
Copy link
Member

Hey @FastestMolasses was the status on the PR? Is it still up for review?

austincondiff
austincondiff previously approved these changes May 30, 2025
Copy link
Collaborator

@austincondiff austincondiff 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! Just merge the conflicts.

@thecoolwinter
Copy link
Collaborator

@FastestMolasses hi! I forked from your branch and found a crash: StyledRangeStore+Internals.swift line 44 _guts[_guts.index(after: index)] fails with out of bounds error

this check fixes: let nextIndex = _guts.index(after: index) guard nextIndex < _guts.endIndex else { return }

@FastestMolasses is right, this was due to another component in the editor, which I believe I fixed.

@thecoolwinter
Copy link
Collaborator

@FastestMolasses is this just waiting for a parameter to be able to pass in a suggestion delegate? I'd be happy to fill that out if that's all that's left.

@FastestMolasses
Copy link
Member Author

There's still the bug with the file contents not being updated. I have some refactors that fixes a few issues, I'll commit that and request reviews.

@thecoolwinter
Copy link
Collaborator

thecoolwinter commented Jul 22, 2025

@FastestMolasses if it's alright I'm going to push this one across the finish line. I'd like to get "Jump to Definition" going and I'll need the itembox for that too.

@FastestMolasses
Copy link
Member Author

No problem, committed my refactors, just needs a review.

@thecoolwinter
Copy link
Collaborator

thecoolwinter commented Jul 23, 2025

Ah shoot I didn't realize you had changes ready. I made a refactor making it so CESE controls displaying the autocomplete window and upgraded the code suggestion delegate to only need to reply with suggestions instead of managing windows too.

I could merge it here still, or I could open a PR to merge into this itembox branch to review first. Do you have a preference?

@thecoolwinter
Copy link
Collaborator

This is what I came up with for the delegate:

`CodeSuggestionDelegate`

public protocol CodeSuggestionDelegate: AnyObject {
    func completionTriggerCharacters() -> Set<String>

    func completionSuggestionsRequested(
        textView: TextViewController,
        cursorPosition: CursorPosition
    ) async -> (windowPosition: CursorPosition, items: [CodeSuggestionEntry])?

    // This can't be async, we need it to be snappy. At most, it should just be filtering completion items
    func completionOnCursorMove(
        textView: TextViewController,
        cursorPosition: CursorPosition
    ) -> [CodeSuggestionEntry]?

    // Optional
    func completionWindowDidClose()

    func completionWindowApplyCompletion(
        item: CodeSuggestionEntry,
        textView: TextViewController,
        cursorPosition: CursorPosition
    )
    // Optional
    func completionWindowDidSelect(item: CodeSuggestionEntry)
}

public extension CodeSuggestionDelegate {
    func completionTriggerCharacters() -> Set<String> { [] }
    func completionWindowDidClose() { }
    func completionWindowDidSelect(item: CodeSuggestionEntry) { }
}

@thecoolwinter
Copy link
Collaborator

thecoolwinter commented Jul 23, 2025

I have the required changes for CodeEditApp/CodeEdit#1949 too. It reduces the required loc for the AutoCompleteCoordinator significantly.

I'll commit my changes and we can revert if needed.

- Creates a `SuggestionViewController` for managing the view contents of the window.
- Renames `SuggestionControllerDelegate` to `CodeSuggestionDelegate`
- Moves `CodeSuggestionEntry` to its own file.
- Removes a few magic numbers
- Removes the `horizontalOffset` parameter when moving the window. We'll rely on the suggestion delegate to tell us where to place the window's top-left corner.
@thecoolwinter
Copy link
Collaborator

I'd say this is ready to merge. @FastestMolasses I made changes but I can't request your review on your own pr so if you'd like changes please comment!

@thecoolwinter
Copy link
Collaborator

thecoolwinter commented Jul 24, 2025

I made one last change bringing the suggestion UI element down into CESE. The image and color determination are still up to the API caller, but this makes this component much easier to reuse.

I also figured out some more things about automatically setting the window and row size. The row size is now dynamic based on the user's selected font. It also updates the window size based on that dynamic row height.

The window's width is now determined by the smallest suggestion element's font width. This means it's dynamic down to a minimum size and up to a maximum size, similar to Xcode.

I also included a popover mode that this PR doesn't use but I'd already implemented it so I kept it in here.

@FastestMolasses
Copy link
Member Author

Looks good, lets merge it!

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.

5 participants