Add per-server and per-database search scope - #8
Merged
Merged
Conversation
A Scope button on the tool window toolbar opens a checkbox tree of the connected servers and their databases. Unchecking a database drops it from search results immediately and stops it being indexed; re-checking spot-indexes just that database rather than triggering a full refresh. Scope is an exclusion list (default-include), persisted to %AppData%\SqlPilot\scope.txt, so an empty store behaves exactly as before and new databases on an in-scope server are still picked up automatically. - SqlPilot.Core/Scope: ISearchScopeStore + SearchScopeStore, mirroring FavoritesStore. Composite keys "S|<server>" and "D|<server>|<database>". - SearchEngine takes an optional ISearchScopeStore and skips out-of-scope index buckets; adds ClearDatabase and GetIndexedDatabaseCount. - LineStore.LoadSettings now splits at the first *unescaped* separator — a plain IndexOf split the new composite keys mid-key. - RefreshIndexAsync filters each server's database list through the store before building index tasks. Database names are still enumerated for out-of-scope servers (one cheap query) to keep the tree and status line accurate. - Status line reports "x of y database(s) from n of m server(s)" when scope excludes anything, and the old wording when it doesn't. - UI.Demo goes multi-server so the tree is exercisable without SSMS.
Cleanup pass over the scope feature — no behaviour change. - Extract the scope TreeView into SqlPilot.UI/Controls/ScopeControl, following the SearchControl pattern. It was copy-pasted into the tool window and the demo, 24 of 28 lines identical. The control is theme-neutral so each host keeps its own colours. - Give LineStore the escape scheme it already owned: Join/Split replace SearchScopeStore's private copy of Esc/Unesc plus a hand-rolled splitter. The on-disk format is unchanged. - Move the scope guard into SearchEngine.RefreshIndexAsync. The engine holds the store, so callers can no longer forget to filter. - Collapse RefreshIndexAsync's per-server body, SpotIndexServerAsync and SpotIndexDatabaseAsync into one IndexServerAsync + a CreateProvider helper; same in the demo. - Re-checking a server now reuses the database list already in the tree instead of a fresh connect + metadata query, and an out-of-scope server we've already seen isn't re-enumerated at all. - Drop GetIndexedDatabaseCount: it was on ISearchEngine with no caller but a test. The status line derives its counts from the scope tree. - Correct the comment claiming excluded buckets stay indexed (hosts drop them), and scope the store's class comment to the storage rule it actually owns.
- Every scope checkbox now carries AutomationProperties.Name/LabeledBy and the tree items carry a Name. The labels are siblings of the checkboxes, so a screen reader previously announced 16 unnamed "checkbox, checked" nodes and each tree item read as "SqlPilot.UI.ViewModels.ServerScopeNode". - Alt+S toggles the scope panel, focusing the tree when it opens and returning to the search box when it closes. Registered as an in-window KeyBinding, not a global hotkey, so it can't clash with an SSMS or Windows shortcut. - The Scope button is now the Segoe MDL2 filter glyph, matching the icon set ObjectTypeToIconConverter already uses. Icon-only, hence the explicit automation name and tooltip. - Move RelayInputCommand to SqlPilot.UI so both hosts can use it. The copy in SqlPilot.Package was unused and absent from the Legacy csproj, so calling it from the shared tool window would have broken the SSMS 18/20 build. - Fix the search placeholder, which advertised Ctrl+Shift+D while the code and README both say Ctrl+D.
Cleanup pass over the previous commit — no behaviour change. - Delete RelayInputCommand. CommunityToolkit.Mvvm is already a PackageReference of SqlPilot.UI, already on build/SqlPilotDlls.txt, and its RelayCommand(Action) has identical semantics. Moving the type was the right direction; deleting it was the right depth. - Drop AutomationProperties.LabeledBy and the two x:Name labels feeding it. FrameworkElementAutomationPeer.GetNameCore only consults LabeledBy when Name is empty, and Name is always set here, so it was never read. - Drop AutomationProperties.HelpText. GetHelpTextCore falls back to ToolTip only when HelpText is empty, so the literal was hiding the better string: the Scope button now reports "(Alt+S)" and each database reports its server, which is what tells two same-named databases apart. - Drop the server checkbox ToolTip, which repeated the label beside it. - Replace both ScopeButton_Toggled handlers with a Visibility binding on IsChecked, deleting the duplicated code-behind in each host. - FocusTree: check for an empty tree before forcing a layout pass, since that case never needed a container.
IndexDatabasesAsync posts per-database progress with a fire-and-forget Dispatcher.BeginInvoke, so a late one could run after the caller had already written "Indexed N objects in ...", leaving the stale "Indexing ..." text on screen until the next refresh. Drain the queue below their priority before returning. Pre-existing, but the scope re-check path made it reliable: with a single database the spot-index finishes fast enough that the stale text always won. Also record the SSMS 22 findings from verifying this against real installs: the package registers but never loads there (SSMS 18/20 are fine), and Process.Modules is not a valid check for whether an extension loaded.
IndexDatabasesAsync is entered from the UI thread and none of its awaits leave it, so the progress update was posting from the UI thread to the UI thread. That post is what could outrun the caller's final status, so assigning directly removes the race rather than draining the queue behind it, and the drain added in 7a1eb9a goes away with it. MergeServer now asks the store IsDatabaseIncluded instead of rebuilding that rule from the raw exclusion list, and matches existing nodes through a dictionary rather than a scan per database. New databases are inserted at their sorted position, so the re-sort pass over a live-bound collection is gone. DatabaseScopeNode drops the _isSyncing flag for a hand-written property: the silent path writes the field, the bound path means the user clicked. Also: LoadObjects now shares Split, so an object whose name contains "|" round trips like the settings file already did; SearchAsync only parses a bucket key when a filter or scope will read it; and the demo stops re-filtering by scope ahead of the engine that owns the same store.
Every SSMS 22 launch behind that section had been started with -E, a switch SSMS 22 no longer has. The window I attached to was the usage-error dialog, which shares the IDE's title, so nothing had loaded because the IDE had not started. The corrected findings live in the fix/ssms22-autoload-context branch. The Process.Modules warning stands and stays.
Both branches added the same bullet at the same line; the autoload branch carries the fuller version alongside the SSMS 22 command-line notes.
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (23)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5.
Adds a Scope panel (filter icon next to Refresh, or
Alt+S) with a checkbox tree of every connected server and its databases. Unchecking a database drops it from the index and from search results immediately; unchecking a server drops all of it. Re-checking spot-indexes just that database — no full re-index. Exclusions persist across SSMS restarts, and a database that appears later on an in-scope server is indexed automatically. The status line readsIndexed N objects in X of Y database(s) from A of B server(s)and collapses to the old wording when nothing is excluded.Design
%AppData%\SqlPilot\scope.txtstores only exclusions (S|<server>,D|<server>|<database>), so an empty or missing file behaves exactly like today. Nothing changes for existing users until they uncheck something.SearchEnginetakes an optionalISearchScopeStoreas a third constructor argument and enforces it at both index time (RefreshIndexAsyncskips excluded databases) and search time (out-of-scope buckets are skipped).SearchFilteris unchanged.SqlPilotSettingsand the Tools > Options page on purpose — it's dynamic state keyed to whatever happens to be connected.ScopeControl(SqlPilot.UI) drives both the SSMS tool window and the demo app; every checkbox and tree item has an automation name for screen readers.Also in here
LineStore.LoadSettingssplit keys at the first|even when escaped — a latent bug the composite scope keys exposed. Fixed with escape-awareJoin/Split, andLoadObjectsnow uses the same scheme.Indexing …on screen; visible on a scope re-check. Root cause was posting to the dispatcher from a continuation already on the UI thread — now a direct assignment.Ctrl+Shift+D; the working hotkey isCtrl+D(the.vsctthat declared the other binding is compiled by neither csproj).Verification
Verified in real SSMS 18, 20 and 22 against LocalDB through UI Automation: exclude / re-check / server toggle / tri-state,
scope.txtround trip across a restart, a database created while SSMS was down showing up checked and indexed on Refresh,Alt+Sfocus handling, and search results filtering accordingly. Also driven from the demo app. Core tests go from 44 to 83.