-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add LLM instructions file suggestion #669
base: main
Are you sure you want to change the base?
Conversation
a7f5b8b
to
81ad912
Compare
the early return was preventing the language server initialization when the instrunction file was already there
1edad8c
to
288f786
Compare
… editor decorations'
…lm-instructions.ts' + unit tests - Add 'getShopifyThemeRootDirs' and 'fileExists' APIs
- Adjust prompt and similarity - Test Shopify Magic
👋 Hey @charlespwd @frandiox, this PR is ready for review now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great, thanks a lot for all the improvements and tests! The fact that it uses up to date liquid docs is such a nice change 🙌
I'm having certificate issues to run this in my new laptop though, so I couldn't 🎩 yet :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally able to test it, great job 🎉
I've tried to close the hover without much luck either!
While testing, I found a few minor things:
- I got into this situation a couple of times when applying one suggestion after another. cmd+z and click again fixes the situation... no idea why it happens in the first place :/
- I saw you added a comment to avoid the "Keep xyz when is already valid"... however this still happens often:
Should we maybe compare the suggestion with the code-to-be-replaced and skip it if it's the same? 🤔
- Not important but do we rename
sidefix
to something else?
Thank you for the review, @frandiox! I've applied the suggestions :) also, I've created this to handle scenario 1, slightly updated the prompt to prevent the scenario 2 (I couldn't get those "Keep <suggestion...>" anymore in my theme), and I've renamed Thanks again for the review! 🙌 |
What are you adding in this PR?
This PR introduces support for:
.cursorrules/copilot-instructions.md
which works with multiple themes opened in the same workspace.cursorrules/copilot-instructions.md
files are generated based on https://github.com/Shopify/theme-liquid-docsClick here to check a demo of this feature.
What's next? Any follow-up issues?
After clicking on “Quick fix,” close the hover dialog automatically (so users may see the suggestions immediately) — I don't think that's possible, but I'm planning to revisit this when we get this work on main.
Before you deploy
changeset