Skip to content
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

FEAT: Improve behavior of search / function on tree #124

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

KevsterAmp
Copy link

Current Behavior

Running the search command / will expand all databases and find all possibilities of tables within the whole server, throughout all databases.

The issue arises when there are multple instances of table names within multiple databases. it becomes hard to determine which database the focused table is from since the database is expanded, and if there are 20+ tables you cannot see the DB name unless you move up. 

Implemented Behavior

To mark a database as "selected" It has to be the currNode and it should be expanded.

  • If there is no database selected. The search function will search throughout the names of all the databases ONLY.
  • If there is a database selected. The search function will search thorughout the names of all the tables within the selected database.

I believe this is a better implementation since its most common to search a table within a specific database, rather than all the databases.

I can also include in this PR to move the command that "searches on all databases" to CTRL + /.

If there are no selected database (highlighted db node should be
expanded): function will search on database names only
else, the function will search within the tables of the selected
database
@KevsterAmp
Copy link
Author

KevsterAmp commented Nov 17, 2024

Found a bug that I'm not sure how to fix.
When you expand and collapse database node/s, the app seems to record all the tables within it. So when you run a search command even if there are no selected database. It will include the tables within the previously expanded in its search.

At first glance, i think has to be related tree.state.searchFoundNodes. I'll try to look at the issue more later in the day

@KevsterAmp
Copy link
Author

Thoughts on this? @jorgerojas26 Thanks

Comment on lines +343 to +350
var searchStartNode *tview.TreeNode
if currNode != nil && currNode.IsExpanded() {
// search between tables if theres a curentdatabase selected
searchStartNode = currNode
} else {
// if there's no current database selected, search within the database names only
searchStartNode = rootNode
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var searchStartNode *tview.TreeNode
if currNode != nil && currNode.IsExpanded() {
// search between tables if theres a curentdatabase selected
searchStartNode = currNode
} else {
// if there's no current database selected, search within the database names only
searchStartNode = rootNode
}
// search within the database names only by default
searchStartNode := rootNode
if currNode != nil && currNode.IsExpanded() {
// search between tables if there is a database selected
searchStartNode = currNode
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] Search tables (/) within a specific database
2 participants