Update everything-search extension#24494
Conversation
|
Thank you for your contribution! 🎉 🔔 @anastasiuspernat you might want to have a look. You can use this guide to learn how to check out the Pull Request locally in order to test it. 📋 Quick checkout commandsBRANCH="ext/everything-search"
FORK_URL="https://github.com/LorryGailius/extensions.git"
EXTENSION_NAME="everything-search"
REPO_NAME="extensions"
git clone -n --depth=1 --filter=tree:0 -b $BRANCH $FORK_URL
cd $REPO_NAME
git sparse-checkout set --no-cone "extensions/$EXTENSION_NAME"
git checkout
cd "extensions/$EXTENSION_NAME"
npm install && npm run devDue to our current reduced availability, the initial review may take up to 10-15 business days. |
Greptile SummaryThis PR adds regex search support, configurable max result counts, and an experimental SDK-based search method to the Everything Search extension. While the feature additions are well-intentioned and the CLI implementation is solid, the PR introduces three critical issues that need addressing:
The core implementation is otherwise solid: the CLI regex/maxResults handling is correct, SDK module loading properly handles architecture detection, and preferences configuration follows the established patterns. However, the missing SDK sort mappings are a functional defect that directly impacts users choosing to use the experimental SDK feature. Confidence Score: 2/5
Important Files Changed
|
There was a problem hiding this comment.
Additional Comments (4)
-
src/utils/everything-sdk/everything-sdk-utils.ts, line 40-73 (link)logic: Missing sort type mappings for several options available in preferences. When users select "Attributes (A-Z)", "Attributes (Z-A)", "File List File Name (A-Z/Z-A)", "Run Count", "Date Recently Changed", or "Date Run" from SDK mode, the function falls back to NAME_ASCENDING instead of mapping to the correct sort type.
The switch statement should include cases for:
-sort attributes-ascending→ SortType.ATTRIBUTES_ASCENDING-sort attributes-descending→ SortType.ATTRIBUTES_DESCENDING-sort file-list-file-name-ascending→ SortType.FILE_LIST_FILENAME_ASCENDING-sort file-list-file-name-descending→ SortType.FILE_LIST_FILENAME_DESCENDING-sort run-count-ascending→ SortType.RUN_COUNT_ASCENDING-sort run-count-descending→ SortType.RUN_COUNT_DESCENDING-sort date-recently-changed-ascending→ SortType.DATE_RECENTLY_CHANGED_ASCENDING-sort date-recently-changed-descending→ SortType.DATE_RECENTLY_CHANGED_DESCENDING-sort date-run-ascending→ SortType.DATE_RUN_ASCENDING-sort date-run-descending→ SortType.DATE_RUN_DESCENDING
-
CHANGELOG.md, line 1-12 (link)syntax: Changelog entry title must use
{PR_MERGE_DATE}placeholder instead of literal date, and should be placed at the top in descending version order with most recent version first.Context Used: Rule from
dashboard- What: Changelog entries must use{PR_MERGE_DATE}placeholder in titles, be placed at the top of th... (source) -
src/components/SearchResult.tsx, line 98-101 (link)syntax: Platform-specific shortcut keys use incorrect capitalization:
windowsshould beWindows(capital W). This will cause the keyboard shortcuts to fail on Windows.Context Used: Rule from
dashboard- What: Ensure platform keys in shortcut objects use correct capitalization:WindowsandmacOS, no... (source) -
src/components/SearchResult.tsx, line 118 (link)syntax: Platform-specific shortcut on line 118 uses incorrect capitalization:
windowsshould beWindows. This will cause keyboard shortcuts to fail on Windows.Fix: Change line 118 from
windows:toWindows:Context Used: Rule from
dashboard- What: Ensure platform keys in shortcut objects use correct capitalization:WindowsandmacOS, no... (source)
15 files reviewed, 4 comments
There was a problem hiding this comment.
suggestion: script to fetch this
Can we add a script and a command in package.json to fetch this?
Keep the file in the assets folder under version control. A fetch script will help other maintainers.
There was a problem hiding this comment.
Do you mean fetch the prebuilt binaries from LorryGailius/everything-search-sdk-node or re-build them with a script? Problem is to build the binaries on machine it requires additional setup beforehand so distribution of it is quite above my knowledge
There was a problem hiding this comment.
No, I mean to fetch from the source (https://github.com/LorryGailius/everything-search-sdk-node) which you are already doing, but if it can be documented or added a script in package, it would be helpful to other maintainers.
There was a problem hiding this comment.
suggestion: script to fetch this
Can we add a script and a command in package.json to fetch this?
Keep the file in the assets folder under version control. A fetch script will help other maintainers.
|
This pull request has been automatically marked as stale because it did not have any recent activity. It will be closed if no further activity occurs in the next 7 days to keep our backlog clean 😊 |
|
Hey @LorryGailius 👋 Thanks for your contribution 🔥 Is this ready for review? |
|
This pull request has been automatically marked as stale because it did not have any recent activity. It will be closed if no further activity occurs in the next 7 days to keep our backlog clean 😊 |
|
Closing this as it is not currently ready for release, there are some prominent issues i need to investigate. Will open a new PR, once I have handled some intricacies |
Description
.nodemodule builds are available at LorryGailius/everything-search-sdk-node. For now the feature will be flagged as experimental and after further testing from me and users interested in this, will be moved to be the primary way of communication between the extension and the application.Took inspiration from how to import native node modules from #135
Screencast
Checklist
npm run buildand tested this distribution build in Raycastassetsfolder are used by the extension itselfREADMEare located outside the metadata folder if they were not generated with our metadata tool