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

Code completion should be case-sensitive if the language is case-sensitive #626

Open
SCWells72 opened this issue Nov 25, 2024 · 8 comments
Labels
completion enhancement New feature or request

Comments

@SCWells72
Copy link
Contributor

I noticed that offered code completions are often incorrect/invalid for the current completion prefix, specifically due to case-sensitivity of the underlying language, e.g., JavaScript and TypeScript are case-sensitive. It looks like LSPCompletionFeature#addLookupItem is always using a case-insensitive prefix matcher.

If LSP itself doesn't give an indication of whether or not a language is case-[in]sensitive, it might make sense to add that to the LanguageServerDefinition as a flag with defaults set in all provided templates, then use the value of that flag when adding completion lookup items.

Again, I'm happy to help with this either way, so just let me know if that state can/should be derived from LSP or added to the language server definition (or elsewhere).

@angelozerr
Copy link
Contributor

angelozerr commented Nov 25, 2024

At first I think we should have a new method LSPCompletionFeature#isCaseSensitive(psiFile file) to provide the capability to customize it as we want.

By default it should return false or perhaps IJ provide a settings for that?

For the case of user defined language server, we could use as you said a flag from template json, but what about language server which are created without template?

Perhaps we will need a new settings tab for user defined language server?

@SCWells72
Copy link
Contributor Author

At first I think we should have a new method LSPCompletionFeature#isCaseSensitive(psiFile file) to provide the capability to customize it as we want.

Make sense. I had assumed that all "features" were inferred from LSP, but it sounds like they can be a mix of what's discovered via LSP and what's configured locally, no?

By default it should return false or perhaps IJ provide a settings for that?

It's a global setting in the IDE itself:

Image

and case-sensitivity varies by-language, so I don't think the IDE-level setting should be used

For the case of user defined language server, we could use as you said a flag from template json, but what about language server which are created without template?

Perhaps we will need a new settings tab for user defined language server?

I was thinking it might just be a flag in the Configuration tab since that's already named quite generically as "Configuration", e.g.:

Image

Thoughts?

@angelozerr
Copy link
Contributor

At first I think we should have a new method LSPCompletionFeature#isCaseSensitive(psiFile file) to provide the capability to customize it as we want.

Make sense. I had assumed that all "features" were inferred from LSP, but it sounds like they can be a mix of what's discovered via LSP and what's configured locally, no?

By default it should return false or perhaps IJ provide a settings for that?

It's a global setting in the IDE itself:

Image

and case-sensitivity varies by-language, so I don't think the IDE-level setting should be used

Is there a language settings for that?

For the case of user defined language server, we could use as you said a flag from template json, but what about language server which are created without template?

Perhaps we will need a new settings tab for user defined language server?

I was thinking it might just be a flag in the Configuration tab since that's already named quite generically as "Configuration", e.g.:

Image

Thoughts?

Configuration tab is used to configure language server,, not the client feature like completion case sensitive.

And I think we will have in the future other settings.

I think we should have

  • Configuration tab like today which configures the lsp language server.
  • A new tab (@fbricon have you some idea about this name) which configure client features like case sensitive.

@SCWells72
Copy link
Contributor Author

Is there a language settings for that?

No. That's a global/IDE-level setting. Otherwise the completion provider(s)/contributor(s) for each respective language -- typically supported in a first-class manner with local tokenizers/parsers/etc. -- know whether each language is case-sensitive or not and add completions for those langauges as appropriate. For example, my plugin supports several case-insensitive languages -- Salesforce Apex, SOQL, and SOSL -- and several case-sensitive languages -- JavaScript, TypeScript, CSS, etc. -- for which it has completion providers/contributors, and I have to ensure that completions are added with the correct case-sensitivity settings for things to behave appropriate for that language.

Configuration tab is used to configure language server,, not the client feature like completion case sensitive.

And I think we will have in the future other settings.

I think we should have

  • Configuration tab like today which configures the lsp language server.
  • A new tab (@fbricon have you some idea about this name) which configure client features like case sensitive.

That's totally fine. Chew on what that tab should be conceptually and let me know your thoughts.

@angelozerr
Copy link
Contributor

Is there a language settings for that?

No. That's a global/IDE-level setting. Otherwise the completion provider(s)/contributor(s) for each respective language -- typically supported in a first-class manner with local tokenizers/parsers/etc. -- know whether each language is case-sensitive or not and add completions for those langauges as appropriate. For example, my plugin supports several case-insensitive languages -- Salesforce Apex, SOQL, and SOSL -- and several case-sensitive languages -- JavaScript, TypeScript, CSS, etc. -- for which it has completion providers/contributors, and I have to ensure that completions are added with the correct case-sensitivity settings for things to behave appropriate for that language.

Ok thank for your explanation. So I think default implementation should use this global settings.

Configuration tab is used to configure language server,, not the client feature like completion case sensitive.

And I think we will have in the future other settings.

I think we should have

  • Configuration tab like today which configures the lsp language server.
  • A new tab (@fbricon have you some idea about this name) which configure client features like case sensitive.

That's totally fine. Chew on what that tab should be conceptually and let me know your thoughts.

Glad you like the idea. Here a simple proposition:

  • Server Configuration (existing configuration tab)
  • Client Configuration

But perhaps name are too long?

@angelozerr angelozerr added enhancement New feature or request completion labels Nov 26, 2024
@angelozerr
Copy link
Contributor

I wonder if instead of having a checkbox we should have a JSON files:

Writting a JSON file can be ugly but if we improve it with:

  • JSON syntax coloration
  • JSON syntax validation
  • JSON completion based on JSON Schema

it will be easy to configure it (like vscode does).

My fear is that this configuration becomes complex. For instance for caseSensitive completion settings, we could imagine to set a caseSensitive per languageId.

Doing that with UI can take some times, with JSON it will easier.

@SCWells72 what do you think about that?

@angelozerr
Copy link
Contributor

@SCWells72 after discussion with @fbricon I think the best idea is to

  • keep Configuration tab
  • Create a Server tab inside the Configuration tab. This server tab will contains the same UI than existing Configuration tab
  • Create a Client tab inside Configuration tab to add caseSensitive for instance option (or JSON editor).

@SCWells72
Copy link
Contributor Author

@angelozerr that's fine with me if that's the way you want to go.

Regarding the use of JSON for client-side configuration, if you want my opinion, I'm not a huge fan. Even with completion/validation against a JSON schema, I think it's just not as simple or intuitive as a dedicated form and well-labeled UI components. But...this is not my project, so I truly do defer to you all on what you'd prefer. No issues on my side either way.

Before I dive into potential UI rework, let me ask a core question: Do you all have any issues if those forms are created using JetBrains' Swing UI editor? I use it for all of the dialogs/forms in my own plugin and find it to be very easy to build and maintain Swing UIs, e.g., one from my plugin:

Image

I wanted to confirm one way or the other to ensure that there's no throwaway work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completion enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants