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

Refresh theme-liquid-docs on theme-check startup #671

Merged
merged 5 commits into from
Dec 9, 2022

Conversation

mgmanzella
Copy link
Contributor

@mgmanzella mgmanzella commented Nov 28, 2022

Summary

Closes #651

Refreshes docs async on theme-check startup, after discussing with @karreiro we decided it's out of scope to periodically refresh while theme-check is running

Since this is a medium size PR, I broke this down into 4 commits:

Tophatting Steps

  • In your terminal run rake download_theme_liquid_docs if you dont have the version of theme-liquid-docs with a revision file
  • In your local copy of theme-check, modify SourceManager#refresh to add sleep(30) in the thread
  • Modify the revision file at data/shopify_liquid/documentation/latest.json to change the revision key to "out_of_date_sha"
  • Modify the local copy of your docs to change the documentation for a particular object (eg modify collections documentation)
  • Restart theme-check and open a theme
  • Confirm autocomplete still works
  • Confirm the docs are downloaded, the revision is updated in latest.json, and the docs are updated in theme-check when using autocomplete

@mgmanzella mgmanzella force-pushed the 651-refresh-theme-liquid-docs branch 2 times, most recently from 95f2009 to b33d541 Compare November 30, 2022 00:00
@mgmanzella mgmanzella marked this pull request as ready for review November 30, 2022 00:01
@mgmanzella mgmanzella requested a review from a team November 30, 2022 00:01
Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Thank you, @mgmanzella! Excellent stuff 🔥 🚀

I've left just some minor comments, please, let me know what you think about them :)

lib/theme_check/shopify_liquid/source_index.rb Outdated Show resolved Hide resolved
lib/theme_check/shopify_liquid/source_manager.rb Outdated Show resolved Hide resolved
lib/theme_check/shopify_liquid/source_manager.rb Outdated Show resolved Hide resolved
@mgmanzella mgmanzella force-pushed the 651-refresh-theme-liquid-docs branch 2 times, most recently from 941f704 to 3857702 Compare December 6, 2022 17:56
Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Thank you, @mgmanzella! Great stuff! It's very rewarding to check it in action 🚀

demo

Copy link
Contributor

@Poitrin Poitrin left a comment

Choose a reason for hiding this comment

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

🎩 ing passed ✅
I just have some nitpicks/remarks :)

lib/theme_check/shopify_liquid/source_manager.rb Outdated Show resolved Hide resolved
lib/theme_check/shopify_liquid/source_index.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@Poitrin Poitrin left a comment

Choose a reason for hiding this comment

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

Thanks for your changes and feedback, LGTM!

@mgmanzella mgmanzella merged commit b7988fe into language-server-docs-main Dec 9, 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 <[email protected]>
Co-authored-by: Julien Poitrin <[email protected]>
Co-authored-by: Julien Poitrin <[email protected]>
@mgmanzella mgmanzella deleted the 651-refresh-theme-liquid-docs branch December 12, 2022 19:18
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.

3 participants