feat:close application intent #11
Conversation
WalkthroughThe pull request introduces several enhancements to the application launcher functionality. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApplicationLauncherSkill
participant IntentRecognizer
participant ProcessManager
User->>IntentRecognizer: "Close Firefox"
IntentRecognizer->>ApplicationLauncherSkill: Recognize intent "close"
ApplicationLauncherSkill->>ProcessManager: close_app("Firefox")
ProcessManager->>ProcessManager: Find running processes
ProcessManager-->>ApplicationLauncherSkill: Process found
ApplicationLauncherSkill-->>User: "Firefox has been closed."
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (6)
requirements.txt (2)
3-3: Approve the addition of ovos-utils and suggest version constraint improvement.The addition of
ovos-utils>=0.3.5aligns with the PR objectives. To prevent potential compatibility issues with future major versions, consider specifying both minimum and maximum version constraints.Consider updating the line to:
ovos-utils>=0.3.5,<1.0.0This ensures compatibility with versions from 0.3.5 up to, but not including, 1.0.0.
4-4: Suggest adding version constraints for psutil.While this line is unchanged, it's generally a good practice to specify version constraints for all dependencies to ensure reproducibility and prevent potential compatibility issues.
Consider updating the line to include version constraints. For example:
psutil>=5.8.0,<6.0.0Replace
5.8.0with the minimum version that includes the features you need, and adjust the upper bound accordingly.locale/en-us/close.intent (1)
1-5: Overall: Well-structured intent file with good variety.The
close.intentfile provides a good range of phrases for closing applications, catering to both casual and technical users. The use of the {application} placeholder allows for flexibility across different applications.Suggestions for potential improvements:
- Consider adding phrases for specific types of applications (e.g., "close browser", "exit text editor").
- You might want to include phrases for closing multiple applications at once (e.g., "close all applications").
- Consider adding phrases that include politeness markers (e.g., "please close {application}") to cater to users who prefer more formal interactions.
Would you like me to propose additional intent phrases based on these suggestions?
translations/en-us/intents.json (1)
7-13: LGTM: "close.intent" is well-defined with comprehensive phrases.The new "close.intent" section is correctly structured with appropriate phrases for closing an application. The {application} placeholder is used consistently across all phrases, and the verbs chosen (close, terminate, kill, exit, quit) cover a wide range of user expressions.
Consider adding "stop {application}" as an additional phrase, as some users might use this verb to express the intent of closing an application.
README.md (1)
17-17: Fix list style inconsistency.The new list item uses an asterisk (*) while the existing items use dashes (-). To maintain consistency and address the static analysis warning, please update the list style.
Apply this change:
-* "Close Firefox" +- "Close Firefox"🧰 Tools
🪛 Markdownlint
17-17: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
__init__.py (1)
49-49: Address the TODO: Expand parentheses handling in intent samplesThere's a TODO comment indicating that parentheses should be expanded and not require one per line. If you need assistance implementing this feature, I can help.
Would you like assistance in enhancing the intent sample parsing to handle parentheses within the same line?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- README.md (1 hunks)
- init.py (4 hunks)
- locale/en-us/close.intent (1 hunks)
- requirements.txt (1 hunks)
- translations/en-us/intents.json (1 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md
17-17: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
🪛 Ruff
__init__.py
50-50: Ambiguous variable name:
l(E741)
70-70: Ambiguous variable name:
l(E741)
🔇 Additional comments (14)
locale/en-us/close.intent (5)
1-1: LGTM: Clear and common phrase for closing applications.The phrase "close {application}" is a straightforward and widely used command for closing applications. It's intuitive and likely to be used by users.
2-2: Approved: Technical alternative for closing applications.The phrase "terminate {application}" provides a more technical alternative for closing applications. This is beneficial for users who prefer or are more familiar with technical terminology.
4-4: LGTM: User-friendly phrase for closing applications.The phrase "exit {application}" is a clear, user-friendly term commonly used for closing applications. It's widely understood and aligns well with the intent.
5-5: LGTM: Another clear phrase for closing applications.The phrase "quit {application}" is another clear and commonly used term for closing applications. It provides a good alternative to "exit" and enhances the variety of recognized intents.
3-3: Approved with suggestion: Consider clarifying the "kill" command behavior.The phrase "kill {application}" is a valid technical term, often used in command-line interfaces. However, it might be perceived as aggressive by some users and could potentially lead to forceful termination of applications.
Consider clarifying how this command will be handled in the application. Will it perform a forceful termination or a graceful exit? You may want to add a comment in the code or documentation explaining the behavior.
To verify the implementation, you can run the following script:
✅ Verification successful
Verified: Clarification on the "kill" command behavior is implemented in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any implementation details or comments about the "kill" command # Search for any mentions of "kill" in Python files rg --type python "def.*kill" -C 5 rg --type python "# .*kill" -C 2Length of output: 132
Script:
#!/bin/bash # Description: Check for any implementation details or comments about the "kill" command # Search for any mentions of "kill" in Python files rg "def.*kill" --glob "*.py" -C 5 rg "# .*kill" --glob "*.py" -C 2Length of output: 385
translations/en-us/intents.json (2)
2-6: LGTM: "launch.intent" is well-defined and consistent.The "launch.intent" section is correctly structured with appropriate phrases for launching an application. The {application} placeholder is used consistently across all phrases.
1-14: LGTM: Overall structure and consistency are excellent.The file is well-structured with consistent formatting. Both intents ("launch.intent" and "close.intent") follow the same pattern, making it easy to read and maintain. The JSON structure is valid, and the file location (translations/en-us/intents.json) correctly indicates its purpose for English (US) translations.
README.md (1)
17-17: LGTM! New example added successfully.The addition of the "Close Firefox" example is consistent with the PR objectives and enhances the documentation by showcasing the new close application functionality.
🧰 Tools
🪛 Markdownlint
17-17: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
__init__.py (6)
2-3: Imports are appropriate for the new functionalityThe added imports
subprocessandtimeare necessary for process management and timing functionality introduced in this PR.
7-8: Adding required imports for process management and language matchingThe imports of
psutilandclosest_matchare appropriate for the new methodsclose_appand improved language handling.
11-11: Importing parsing utilities for matching applicationsThe addition of
match_oneandfuzzy_matchfromovos_utils.parseis suitable for the application's matching logic.
39-39: Including new intents 'close' and 'launch'Adding
closeandlaunchto the list of intents enhances the skill's capability to handle more voice commands.
148-150: Verify the language match scoring logicIn the condition
if score >= 10:, a higher score may indicate a poorer match, depending on the implementation ofclosest_match. Ensure that this threshold correctly determines when a language is unsupported.
170-174: Test cases demonstrate functionality across languagesThe added test cases in the
__main__block effectively demonstrate the skill's ability to handle commands in different languages.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
ovos-utilsto the requirements.