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

omni: find last racket keyword for completion #4293

Merged

Conversation

benknoble
Copy link
Contributor

Otherwise it finds the first keyword, which is usually not relevant to the
cursor position, and incorrectly calculates the completion position.

Where are the tests? Have you added tests? Have you updated the tests? Read the
comment above and the documentation referenced in it first. Write tests!

Seriously, read :help ale-dev and write tests.

Otherwise it finds the first keyword, which is usually not relevant to the
cursor position, and incorrectly calculates the completion position.
@benknoble
Copy link
Contributor Author

cc @hsanson

Is there something wrong with the NeoVim 06 CI?

@hsanson
Copy link
Contributor

hsanson commented Sep 8, 2022

@benknoble

Is there something wrong with the NeoVim 06 CI?

Sorry, the problem is my lack of Github knowledge. I finally found how to remove that check requirement. We now check for neovim 0.7 instead.

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

I do not use racket so I trust this has been tested by the author.

@hsanson hsanson merged commit e99f262 into dense-analysis:master Sep 8, 2022
benknoble added a commit to benknoble/ale that referenced this pull request Nov 3, 2022
Consider a file like
```
 #lang racket
(require racket/gui)
```
Type `Go(eventspace-`.

Pressing <C-x><C-o> to trigger omnicomplete should suggest
```
eventspace-handler-thread
eventspace-shutdown?
eventspace-event-evt
```

It does not (instead producing "top-level" completions, as if
`(eventspace-` wasn't even there).

Debugging, `ale#completion#OmniFunc(1, '')` correctly returns `1`, but
when given `(0, 'eventspace-')` it returns either the empty list or
generic completion results as described above. I'm not entirely sure of
the mechanism, but it seems that b:ale_completion_info.prefix is the
key, and that this is set by `ale#completion#GetPrefix`.

Calling `ale#completion#GetPrefix('racket', line('.'), col('.'))` with
the cursor on a space _after_ `eventspace-` returned `''`!

Now, it returns `eventspace-` and the completions work correctly again.

Ref dense-analysis#4293, dense-analysis#4186, dense-analysis#3870
benknoble added a commit to benknoble/ale that referenced this pull request Nov 3, 2022
Consider a file like
```
 #lang racket
(require racket/gui)
```
Type `Go(eventspace-`.

Pressing <C-x><C-o> to trigger omnicomplete should suggest
```
eventspace-handler-thread
eventspace-shutdown?
eventspace-event-evt
```

It does not (instead producing "top-level" completions, as if
`(eventspace-` wasn't even there).

Debugging, place the cursor on a space _after_. Now
`ale#completion#OmniFunc(1, '')` correctly returns `1`, but when given
`(0, 'eventspace-')` it returns either the empty list or generic
completion results as described above. I'm not entirely sure of the
mechanism, but it seems that `b:ale_completion_info.prefix` is the key,
and that this is set by `ale#completion#GetPrefix`. Calling
`ale#completion#GetPrefix('racket', line('.'), col('.'))` returned `''`!

Now, it returns `eventspace-` and the completions work correctly again.

Ref dense-analysis#4293, dense-analysis#4186, dense-analysis#3870
hsanson pushed a commit that referenced this pull request Nov 5, 2022
Consider a file like
```
 #lang racket
(require racket/gui)
```
Type `Go(eventspace-`.

Pressing <C-x><C-o> to trigger omnicomplete should suggest
```
eventspace-handler-thread
eventspace-shutdown?
eventspace-event-evt
```

It does not (instead producing "top-level" completions, as if
`(eventspace-` wasn't even there).

Debugging, place the cursor on a space _after_. Now
`ale#completion#OmniFunc(1, '')` correctly returns `1`, but when given
`(0, 'eventspace-')` it returns either the empty list or generic
completion results as described above. I'm not entirely sure of the
mechanism, but it seems that `b:ale_completion_info.prefix` is the key,
and that this is set by `ale#completion#GetPrefix`. Calling
`ale#completion#GetPrefix('racket', line('.'), col('.'))` returned `''`!

Now, it returns `eventspace-` and the completions work correctly again.

Ref #4293, #4186, #3870
@benknoble benknoble deleted the fix-racket-completion-start branch October 4, 2023 19:35
mnikulin pushed a commit to mnikulin/ale that referenced this pull request Nov 12, 2023
Otherwise it finds the first keyword, which is usually not relevant to the
cursor position, and incorrectly calculates the completion position.
mnikulin pushed a commit to mnikulin/ale that referenced this pull request Nov 12, 2023
Consider a file like
```
 #lang racket
(require racket/gui)
```
Type `Go(eventspace-`.

Pressing <C-x><C-o> to trigger omnicomplete should suggest
```
eventspace-handler-thread
eventspace-shutdown?
eventspace-event-evt
```

It does not (instead producing "top-level" completions, as if
`(eventspace-` wasn't even there).

Debugging, place the cursor on a space _after_. Now
`ale#completion#OmniFunc(1, '')` correctly returns `1`, but when given
`(0, 'eventspace-')` it returns either the empty list or generic
completion results as described above. I'm not entirely sure of the
mechanism, but it seems that `b:ale_completion_info.prefix` is the key,
and that this is set by `ale#completion#GetPrefix`. Calling
`ale#completion#GetPrefix('racket', line('.'), col('.'))` returned `''`!

Now, it returns `eventspace-` and the completions work correctly again.

Ref dense-analysis#4293, dense-analysis#4186, dense-analysis#3870
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.

None yet

2 participants