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

SwaggerEditor@next: autocompletion works incorrectly #4216

Closed
char0n opened this issue Jun 19, 2023 · 5 comments
Closed

SwaggerEditor@next: autocompletion works incorrectly #4216

char0n opened this issue Jun 19, 2023 · 5 comments

Comments

@char0n
Copy link
Member

char0n commented Jun 19, 2023

Autocompletion works inproperly on next editor. If I write e.g. aaa it shows all words which contains few a. I expect it should show only words which contains ‘aaa' in a row

Steps To Reproduce

  1. Open next editor (OAS 3 or OAS 3.1)
  2. Start writing e.g. 'aaa'

Expected results
I expect it should show only words which contains ‘aaa' in a row. - see screen3 and screen4(examples from SH ACE editor)

Actual results
it shows all words which contains few a - see screen1 and screen2

screen1
screen1

screen2
screen2

screen3

screen3

screen4

screen4

Ref SWG-8475

frantuma added a commit to swagger-api/apidom that referenced this issue Jul 15, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 16, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 17, 2023
frantuma added a commit to swagger-api/apidom that referenced this issue Jul 17, 2023
@frantuma frantuma changed the title SwaggerEditor@next: autocompletion works incorectly SwaggerEditor@next: autocompletion works incorrectly Jul 17, 2023
@frantuma
Copy link
Member

@kowalczyk-krzysztof @char0n @ponelat

This APIDom PR #2954 introduces support for language service "strict" filtering, by setting enableLSPFilter: true in completionContext (within languageServiceContext):

export class ApiDOMWorker {
  static apiDOMContext = {
    validatorProviders: [],
    completionProviders: [],
    performanceLogs: false,
    logLevel: apidomLS.LogLevel.WARN,
    defaultLanguageContent: {
      namespace: 'asyncapi',
    },
    completionContext: {
      maxNumberOfItems: 100,
      enableLSPFilter: true,
    },
  };

An mentioned in swagger-api/apidom#2954 (comment) and copying from comment:

This is an opinionated behaviour, as the default for LSP completions is to leave the filtering logic to the editor/client; Monaco editor always performs fuzzy matching against the filterText value and/or the current word, and in the #4216 it still displays non "strict matching" items, which is possibly a desired behavior in many cases (e.g it still provides hints in case of typos etc).

This is quite debated though.

As a user I would probably favor the standard behavior with fuzzy matching, and IMO this would be kept disabled by default, still allowing apps to enable it, maybe even providing a user option. This is not a strong opinion though.

#4289 adds a comment/placeholder demonstrating usage, in case decision is to implement it by default we will update PR to enable the flag.

@frantuma
Copy link
Member

For reference see also microsoft/vscode#141224 and microsoft/vscode#64367

@char0n
Copy link
Member Author

char0n commented Jul 18, 2023

As a user I would probably favor the standard behavior with fuzzy matching, and IMO this would be kept disabled by default, still allowing apps to enable it, maybe even providing a user option. This is not a strong opinion though.

Yep, I do agree with keeping the current behavior (fuzzy matching) and giving the editor-monaco-language-apidom plugin capability to configure it on mount. I'll follow up with #4289 and allow to make this configurable, with default value of false.

@char0n
Copy link
Member Author

char0n commented Jul 18, 2023

Here is a PR that enables overriding ApiDOM Context configuration options to be able to enable enableLSPFilter option to be overriden. Other ApiDOM context options are now overridable as well. Documentation is part of the PR.

#4290

@frantuma
Copy link
Member

closing as completed in swagger-api/apidom#2954 and #4290

swagger-bot pushed a commit that referenced this issue Jul 20, 2023
# [5.0.0-alpha.69](v5.0.0-alpha.68...v5.0.0-alpha.69) (2023-07-20)

### Bug Fixes

* **docker:** update libtiff to non-vulnerable version ([#4296](#4296)) ([4da5bdb](4da5bdb))

### Features

* **monaco-language-apidom:** allow override ApiDOM Context ([#4290](#4290)) ([1a2081b](1a2081b)), closes [#4216](#4216)
* **monaco-language-apidom:** move completionContext to APIDomContext ([36f86bb](36f86bb))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants