feat: add support for shortdesc, tags and examples in searchbnf.conf#1977
feat: add support for shortdesc, tags and examples in searchbnf.conf#1977Benni0 wants to merge 8 commits into
Conversation
|
Hi @kkedziak-splunk, |
kkedziak-splunk
left a comment
kkedziak-splunk
left a comment
There was a problem hiding this comment.
Review Summary
Overall a clean and well-structured PR that adds shortdesc, tags, and examples support to searchbnf.conf generation. Good test coverage for the new fields.
Issues found:
-
Typo in schema description (
schema.json):"Short description or the custom search command"should be"Short description of the custom search command". -
Template trailing newline (
searchbnf_conf.template): The file is missing a final newline. This can cause issues with some tooling and linters. -
Doc grammar (
custom_search_commands.md): "Each search command can have multiple examples, which are shown displayed in the search assistant" — "shown displayed" is redundant. -
Validator warning message could be improved (
global_config_validator.py): The warning message says"but attributes required for 'searchbnf.conf' is defined which is not required"— now that there are more attributes (shortdesc,tags,examples), consider updating to plural: "are defined which are not required". -
No test for empty
examplesarray edge case: There is no test verifying behavior whenexamplesis an explicitly empty array[]in the config. The existing test covers omitted examples but not empty array.
Good work overall — the schema additions, template changes, and test coverage are solid.
| "shortdesc": { | ||
| "type": "string", | ||
| "description": "Short description or the custom search command, used as shortdesc in searchbnf.conf" | ||
| }, |
There was a problem hiding this comment.
Typo: "Short description or the custom search command" should be "Short description **of** the custom search command".
| example{{ loop.index }} = {{ example["search"] }} | ||
| comment{{ loop.index }} = {{ example["comment"] }} | ||
| {% endfor -%} | ||
| {% endfor -%} No newline at end of file |
There was a problem hiding this comment.
Missing final newline at end of file. This can cause issues with some tools and linters. Consider adding a trailing newline.
| Each search command can have multiple examples, which are shown displayed in the search assistant. The Compact mode, only shows the first example. In the Full mode, the top three examples are displayed. | ||
|
|
||
| For example: | ||
|
|
There was a problem hiding this comment.
Grammar: "which are shown displayed" is redundant. Should be either "which are shown in the search assistant" or "which are displayed in the search assistant".
…s are defined but search assistant is not required
|
Hi @kkedziak-splunk, To avoid unnecessary merge conflicts I want to merge the updates from this pull request in the next pull request (#1979) before fixing the issues there. For this, I'll wait until this pull request is ready to be merged. Is that OK for you? |
|
Hi @kkedziak-splunk, |
2 participants
PR Type
What kind of change does this PR introduce?
Summary
Added support for additional config parameters of searchbnf.conf. This PR allows to specify
shortdesc,tagsandexamplesfor each custom search command. This parameters are used to display the command description in search assistant.Changes
Added the parameters
shortdesc,tagsandexamplesfor custom search command to the schema and modified the generator and the template for searchbnf.conf to support these parameters.User experience
Now the user can specify the parameters
shortdesc,tagsandexamplesin theglobalConfig.jsonwhich provides the ability to enhance the information displayed in the search assistant.Checklist
If an item doesn't apply to your changes, leave it unchecked.
Review
Tests
See the testing doc.
Demo/meeting:
Reviewers are encouraged to request meetings or demos if any part of the change is unclear