Skip to content
This repository was archived by the owner on Jul 27, 2024. It is now read-only.

Define the domain for the theme-liquid-docs files#643

Merged
karreiro merged 3 commits intolanguage-server-docs-mainfrom
fix-639
Oct 5, 2022
Merged

Define the domain for the theme-liquid-docs files#643
karreiro merged 3 commits intolanguage-server-docs-mainfrom
fix-639

Conversation

@karreiro
Copy link
Contributor

@karreiro karreiro commented Sep 27, 2022

This PR introduces the domain for the theme-liquid-docs files.

We're introducing them as a ScopeIndex, so we may rely on it in other use cases than the documentation. Still, this PR also introduces the documentation module to support the language server.

Each kind of scope, in the ScopeIndex, is lazy loaded. Here's some early bench marks:

== Documentation.filter_doc('size')
1st call ~> 1.918ms
2nd call ~> 0.008ms
3rd call ~> 0.007ms

== Documentation.tag_doc('tablerow')
1st call ~> 0.540ms
2nd call ~> 0.004ms
3rd call ~> 0.002ms

== Documentation.object_doc('product')
1st call ~> 9.987ms
2nd call ~> 0.006ms
3d call  ~> 0.005ms

== Documentation.object_property_doc('product', 'available')
1st call ~> 9.799ms
2nd call ~> 0.012ms
3rd call ~> 0.017ms

Considering we're targeting 100ms (as a strict limit) and that the whole index doesn't reach 2MB in memory, I believe we may proceed with this implementation for now. Also, I believe we might warmup the ScopeIndex at the startup of the language server.

--

⚠️ Please, ignore the ScopeIndex#download_files method -- it's a temporary implementation to support the development of this feature.

@karreiro karreiro requested a review from mgmanzella September 27, 2022 11:41
Copy link
Contributor

@mgmanzella mgmanzella left a comment

Choose a reason for hiding this comment

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

🔥 love the data structure and how things are separated, had one comment!

Copy link
Contributor

@mgmanzella mgmanzella left a comment

Choose a reason for hiding this comment

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

🚀

@karreiro karreiro merged commit ee50185 into language-server-docs-main Oct 5, 2022
mgmanzella added a commit that referenced this pull request Dec 9, 2022
* Define the domain for the `theme-liquid-docs` files (#643)

* Introduce the `ObjectAttributeCompletionProvider` module (#654)

* Download theme-liquid-docs on package deploy or while running theme-check (#661)

* Download theme-liquid-docs on package deploy

* Add data fixtures to test downloading theme-liquid-docs

* Introduce the `AssignmentsCompletionProvider` to suggest variables (#667)

* Refresh theme-liquid-docs on theme-check startup (#671)

* Add async download in SourceManager#refresh and mechanism to notify SourceIndex liquid schema is out of date

* Tests for SourceIndex state classes and SourceManager#refresh, create helper for shared stubs

* Cleanup -- fix test dir name to source_index, aggr requires

* Remove class method mock (Net::HTTP.any_instance), affects other tests

* Change SourceIndex#local_path to #local_path! to indicate danger, fix syntax of filename const

* Suggest filters compatible with the object type (#669)

### WHY are these changes introduced?

Fixes #658

### WHAT is this pull request doing?

1. Adds logic to `FilterCompletionProvider` to…
- determine the type of the variable/literal (string, number, array, …) sitting before the filter separator ("input type")
- suggest filters that match the specific input type, and filters whose input type is _variable_ (i.e. for more than 1 specific type, e.g. ` | default`)

2. Displays deprecated filters too, so that users can still see the filter they wanted to use.

Co-authored-by: Morisa Manzella <manzella.morisa@gmail.com>
Co-authored-by: Julien Poitrin <dev@poitrin.com>
Co-authored-by: Julien Poitrin <julien.poitrin@shopify.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants