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

Completion for Active Record .where queries #526

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

Conversation

ChallaHalla
Copy link

@ChallaHalla ChallaHalla commented Nov 15, 2024

Closes: #369

Adding ActiveRecord .where completion suggestions.

I've chosen to only provide suggestions once the user begins typing the name of the column to filter by. This is slightly different than the spec in the ticket. I don't think the call_node_enter listener can be used if suggestions are to be provided before the user begins typing a column name.

Happy to investigate this further! But I think this can be added as a follow up in another PR since the approach for when to begin suggesting seems like it should be different.

Here are some videos of how this looks!

vscode-demo.mov
neo-vim-demo.mov

lib/ruby_lsp/ruby_lsp_rails/completion.rb Outdated Show resolved Hide resolved
lib/ruby_lsp/ruby_lsp_rails/completion.rb Outdated Show resolved Hide resolved
@andyw8
Copy link
Contributor

andyw8 commented Nov 26, 2024

This may also help: Shopify/ruby-lsp#2913

@ChallaHalla
Copy link
Author

Planning on finishing and hopefully shipping this PR during hackdays next week!

@ChallaHalla ChallaHalla changed the title 369 completion for activerecord queries Completion for Active Record .where queries Dec 12, 2024
@ChallaHalla ChallaHalla force-pushed the 369-completion-for-activerecord-queries branch 2 times, most recently from 0b09971 to 896ba66 Compare December 12, 2024 05:00
@ChallaHalla ChallaHalla marked this pull request as ready for review December 12, 2024 05:00
@ChallaHalla ChallaHalla requested a review from a team as a code owner December 12, 2024 05:00
@ChallaHalla ChallaHalla force-pushed the 369-completion-for-activerecord-queries branch 3 times, most recently from 7077b8b to 44cbede Compare December 12, 2024 05:32
@ChallaHalla ChallaHalla force-pushed the 369-completion-for-activerecord-queries branch from 44cbede to def281a Compare December 12, 2024 05:32
@ChallaHalla ChallaHalla requested a review from andyw8 December 12, 2024 05:34
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

This is great. Left some style comments

lib/ruby_lsp/ruby_lsp_rails/completion.rb Outdated Show resolved Hide resolved
lib/ruby_lsp/ruby_lsp_rails/completion.rb Outdated Show resolved Hide resolved
lib/ruby_lsp/ruby_lsp_rails/completion.rb Outdated Show resolved Hide resolved
lib/ruby_lsp/ruby_lsp_rails/completion.rb Outdated Show resolved Hide resolved
lib/ruby_lsp/ruby_lsp_rails/completion.rb Outdated Show resolved Hide resolved
lib/ruby_lsp/ruby_lsp_rails/completion.rb Outdated Show resolved Hide resolved
Comment on lines +48 to +51
if arguments
indexed_call_node_args = index_call_node_args(arguments: arguments)
return if indexed_call_node_args.values.any? { |v| v == node }
end
Copy link
Member

Choose a reason for hiding this comment

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

What is this early return checking for exactly? Is it that we're not processing arguments inside a nested call inside of the where?

Copy link
Author

@ChallaHalla ChallaHalla Dec 13, 2024

Choose a reason for hiding this comment

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

This early return is meant to prevent suggestions when the user is typing in an argument value within a .where in general. E.g when the user is typing User.where(id: creat they shouldn't receive a column name suggestion for created_at.

Copy link
Author

Choose a reason for hiding this comment

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

Covered in this test case here

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.

Completion for ActiveRecord queries
3 participants