Skip to content

Comments

feat: add search knowledge base title#2392

Merged
mcdurdin merged 16 commits intomasterfrom
kb-search
Jan 26, 2026
Merged

feat: add search knowledge base title#2392
mcdurdin merged 16 commits intomasterfrom
kb-search

Conversation

@Meng-Heng
Copy link
Collaborator

I made this to be able to search for the kb article's title.

This PR is ready for reviews.

Test-bot: skip

Before search
Screenshot 2569-01-14 at 12 09 24 in the afternoon

When search
Screenshot 2569-01-14 at 12 09 39 in the afternoon

@keymanapp-test-bot
Copy link

User Test Results

Test specification and instructions

User tests are not required

@github-project-automation github-project-automation bot moved this to Todo in Keyman Jan 14, 2026
@keymanapp-test-bot keymanapp-test-bot bot added this to the A19S20 milestone Jan 14, 2026
@ermshiperete
Copy link
Contributor

Looking at the screenshots, I think it would be helpful to see the filter. For example add a paragraph between the search field and the results that displays after a search. The text could be something like Showing results for "howto":

@Meng-Heng
Copy link
Collaborator Author

@ermshiperete, I decided to add the query onto the input instead. Would this be fine?
Screenshot 2569-01-15 at 2 14 25 in the afternoon


$searchTarget = strtolower("KMKB{$id} {$title}");
if ($query !== '' && stripos($searchTarget, $query) === false) {
continue; // skip this if there isn't a query and a query match
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to fclose($handle); before we continue so that we don't leak $handle?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure. What will be the issue if it is leaked?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the worst case the server uses more and more resources and eventually can crash because it runs out of file handles. Whether or not that happens depends on the implementation of the server, how often processes get restarted etc, but leaks always smell.

I'm not that familiar with php and don't know if fclose is mainly cosmetic and php does some kind of garbage collection in the background, but if there is a fclose and we call it elsewhere, I'd call it here as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for the explanation

@mcdurdin mcdurdin modified the milestones: A19S20, A19S21 Jan 25, 2026
@mcdurdin mcdurdin merged commit 29cf16f into master Jan 26, 2026
5 checks passed
@mcdurdin mcdurdin deleted the kb-search branch January 26, 2026 00:40
@github-project-automation github-project-automation bot moved this from Todo to Done in Keyman Jan 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants