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

IEP-1022: Improve sdkconfig search functionality #819

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Conversation

sigmaaa
Copy link
Collaborator

@sigmaaa sigmaaa commented Sep 14, 2023

Description

Improved sdkconfig filter functionality. Now filtering allows compare not only the Kconfig elements' title but their names (UI labels) as well.

Fixes # (IEP-1022)

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How has this been tested?

open sdkconfig editor. Try to search different components by label's name. For example "After flashing" gives
image

Test Configuration:

  • ESP-IDF Version:
  • OS (Windows,Linux and macOS):

Dependent components impacted by this PR:

  • sdkconfig

Checklist

  • PR Self Reviewed
  • Applied Code formatting
  • Added Documentation
  • Added Unit Test
  • Verified on all platforms - Windows,Linux and macOS

Summary by CodeRabbit

  • New Feature: Enhanced the search functionality in the SDK Configuration UI. Users can now search for KConfigMenuItem objects not only by their title property but also by their name property, providing a more comprehensive and flexible search experience.

@coderabbitai
Copy link

coderabbitai bot commented Sep 14, 2023

Walkthrough

The changes primarily focus on enhancing the search functionality in the SDKConfigurationFilter class. The isLeafMatch and recursiveMatch methods now consider both name and title properties of KConfigMenuItem for a match, improving the overall search accuracy.

Changes

File Summary
.../sdk/config/ui/SDKConfigurationFilter.java Enhanced search functionality by considering both name and title properties of KConfigMenuItem in isLeafMatch and recursiveMatch methods. Removed an unnecessary empty line.

🐇💻

In the land of code, where logic intertwines,
A rabbit hops, leaving traces of lines.
With each hop, a bug takes flight,
Making the codebase shine bright.
Now the search is smarter, it's no longer blind,
It seeks and finds, leaving no keyword behind. 🎉


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

Copy link
Collaborator Author

@sigmaaa sigmaaa left a comment

Choose a reason for hiding this comment

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

self reviewed

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 31b7055 and 80a094c commits.
Files selected for processing (1)
  • bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationFilter.java (2 hunks)
Additional comments (Suppressed): 2
bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationFilter.java (2)
  • 10-21: The changes in this hunk look good. The function isLeafMatch now checks both the title and name of the KConfigMenuItem which should improve the search functionality as described in the PR summary.

  • 25-36: This hunk also looks good. The function recursiveMatch now checks both the title and name of the KConfigMenuItem and its children, which should further enhance the search functionality.

Copy link
Collaborator

@kolipakakondal kolipakakondal left a comment

Choose a reason for hiding this comment

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

This is really helpful. LGTM

@kolipakakondal
Copy link
Collaborator

Hi @AndriiFilippov Please verify this

@kolipakakondal kolipakakondal added the enhancement New feature or request label Sep 28, 2023
@kolipakakondal kolipakakondal added this to the v2.12.0 milestone Sep 28, 2023
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 05177fd and 1e03b89.
Files selected for processing (1)
  • bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationFilter.java (2 hunks)
Additional comments (Suppressed): 2
bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationFilter.java (2)
  • 10-21: The changes in this hunk are enhancing the search functionality by allowing users to search for Kconfig elements not only by their titles but also by their names. This is a good improvement for usability.

  • 25-36: Similar to the previous hunk, this change enhances the recursive search functionality by checking both the name and title of each KconfigMenuItem. This should improve the depth and accuracy of the search results.

@AndriiFilippov
Copy link
Collaborator

@sigmaaa hi!

Tested:
OS - Windows 10 / MacOS
ESP-IDF: v5.1.1

Able to find under name / update / save / build.
LGTM 👍

@kolipakakondal kolipakakondal modified the milestones: v2.12.0, 2.11.1 Oct 5, 2023
@sigmaaa sigmaaa merged commit 0267c08 into master Oct 5, 2023
7 checks passed
@kolipakakondal kolipakakondal deleted the IEP-1022 branch May 10, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants