Add rad2deg and deg2rad to default calculator plugin. #3754
Conversation
📝 WalkthroughWalkthroughSupport for two new mathematical functions, Changes
Suggested labels
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (1)
49-51: Mathematical formulas are correct, but consider improving code consistency.The conversion formulas are mathematically accurate:
rad2deg:rad * (180.0 / Math.PI)correctly converts radians to degreesdeg2rad:x * (Math.PI / 180.0)correctly converts degrees to radiansHowever, there's an inconsistency in the function definition approach -
rad2degis defined as a separate variable whiledeg2radis defined inline.Consider standardizing the function definitions for better consistency:
-Func<double, double> rad2deg = (rad) => rad * (180.0 / Math.PI); -MagesEngine.SetFunction("rad2deg", rad2deg); -MagesEngine.SetFunction("deg2rad", (double x) => x * (Math.PI / 180.0)); +MagesEngine.SetFunction("rad2deg", (double rad) => rad * (180.0 / Math.PI)); +MagesEngine.SetFunction("deg2rad", (double deg) => deg * (Math.PI / 180.0));This approach:
- Makes both functions consistent in style
- Improves parameter naming (
deginstead ofxfor degrees)- Reduces code verbosity
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs(2 hunks)
🔇 Additional comments (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (1)
19-19: LGTM! Regex pattern correctly updated for new functions.The addition of
rad2deg|deg2rad|to the regex pattern is necessary and correctly placed to allow expressions containing these function names to pass validation.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs(10 hunks)Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml
🧰 Additional context used
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs
[warning] 282-282:
actionkeywordview is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Report (PR)
🔇 Additional comments (5)
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs (1)
282-284: Major inconsistency: Changes don't match PR descriptionThe PR objectives describe adding
rad2deganddeg2radfunctions to the calculator plugin, but these changes are for the Explorer plugin's rename feature. This appears to be a mismatch between the PR description and the actual implementation.Technical review: Implementation follows existing pattern
The addition of the rename action keyword model follows the established pattern and integrates properly with the existing settings UI structure.
Likely an incorrect or invalid review comment.
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (1)
164-164: Rename action keyword matching implemented correctlyThe addition to the
ActionKeywordMatchmethod follows the established pattern and correctly checks both the enabled flag and keyword match.Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (3)
64-65: Rename action keyword properties added correctlyThe new
RenameActionKeywordandRenameActionKeywordEnabledproperties follow the established naming conventions and have reasonable default values.
187-188: Enum extended properly for rename functionalityThe
RenameActionKeywordaddition to the internal enum maintains consistency with the existing action keyword structure.
191-259: Switch statement updates are comprehensive and consistentAll four switch statement methods (
GetActionKeyword,SetActionKeyword,GetActionKeywordEnabled,SetActionKeywordEnabled) have been updated to handle the newRenameActionKeywordcase with proper error handling for out-of-range values.
| if (ActionKeywordMatch(query, Settings.ActionKeyword.RenameActionKeyword)) | ||
| { | ||
| return new List<Result>() | ||
| { | ||
| new Result(){ | ||
| Title = "Done", | ||
| AutoCompleteText = "test" | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
Replace placeholder implementation with actual rename functionality
The rename action keyword handling returns hardcoded test values ("Done" title, "test" autocomplete text) which appears to be placeholder code. This should be replaced with proper rename functionality before merging.
- if (ActionKeywordMatch(query, Settings.ActionKeyword.RenameActionKeyword))
- {
- return new List<Result>()
- {
- new Result(){
- Title = "Done",
- AutoCompleteText = "test"
- }
- };
- }
+ if (ActionKeywordMatch(query, Settings.ActionKeyword.RenameActionKeyword))
+ {
+ // TODO: Implement actual rename functionality
+ // This should handle file/folder renaming logic
+ return new List<Result>();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (ActionKeywordMatch(query, Settings.ActionKeyword.RenameActionKeyword)) | |
| { | |
| return new List<Result>() | |
| { | |
| new Result(){ | |
| Title = "Done", | |
| AutoCompleteText = "test" | |
| } | |
| }; | |
| } | |
| if (ActionKeywordMatch(query, Settings.ActionKeyword.RenameActionKeyword)) | |
| { | |
| // TODO: Implement actual rename functionality | |
| // This should handle file/folder renaming logic | |
| return new List<Result>(); | |
| } |
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs between lines
50 and 59, replace the placeholder return of a hardcoded Result with actual
rename functionality. Implement the logic to handle the rename action keyword
properly by generating appropriate Result objects that perform or initiate the
rename operation instead of returning static test values.
…ow.Launcher into dev" This reverts commit 8d8388b, reversing changes made to 60e8226.
This reverts commit 60e8226.
|
Sorry, I forgot how pull requests worked and accidentally committed other changes to the same branch, so I had to revert them. |
This PR adds two functions to the default calculator plugin: rad2deg which converts from radians to degrees, and deg2rad which converts from degrees to radians. I believe calculators that can do the trignometric functions are expected to be able to convert between degrees and radians.
Examples: