fix: changed many rules in browser-to-appname map to use case-insensitive regexes #561
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 09dece0 in 1 minute and 1 seconds
More details
- Looked at
94lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0additional comments because they didn't meet confidence threshold of50%.
Workflow ID: wflow_S5ewNwVlugTgGMsh
You can customize Ellipsis with review rules, user-specific overrides, quiet mode, and more. See docs.
⌛ 13 days left in your free trial, upgrade for $20/seat/month or contact us.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #561 +/- ##
=======================================
Coverage 25.60% 25.60%
=======================================
Files 27 27
Lines 1613 1613
Branches 281 273 -8
=======================================
Hits 413 413
- Misses 1141 1174 +33
+ Partials 59 26 -33 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 6490244 in 35 seconds
More details
- Looked at
14lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. src/queries.ts:242:
- Assessed confidence :
33% - Comment:
The regex pattern for LibreWolf has been updated to include '-default' as an optional suffix. Ensure that this change aligns with the application naming conventions across different platforms and versions as intended. - Reasoning:
The regex pattern for LibreWolf in thebrowser_appnamesmap has been updated to include an optional '-default' suffix. This change is intended to handle variations in the application name, as indicated by the comment and linked Discord discussion. The regex pattern seems correctly formed to match 'LibreWolf', 'LibreWolf-Portable', and 'LibreWolf-default' with optional '.exe' suffix, and it is case-insensitive.
Workflow ID: wflow_QjfcICgtmYRZ6AN4
You can customize Ellipsis with review rules, user-specific overrides, quiet mode, and more. See docs.
⌛ 13 days left in your free trial, upgrade for $20/seat/month or contact us.
|
@ellipsis-dev Is this free of regressions? |
|
Based on my review of the changes, I don't see any obvious issues that could introduce regressions. The regex patterns appear to be correct, and there are no comments in the code that indicate potential issues. However, without being able to run the unit tests or the application itself, I can't definitively say that the changes are free of regressions. I would recommend running the unit tests and doing some manual testing to ensure that the changes are indeed free of regressions. Responding to this comment by @ErikBjare. For more information about Ellipsis, check the documentation. |
src/queries.ts
Outdated
| // Brave (should this be merged with the brave entry?) | ||
| 'Brave-browser', |
There was a problem hiding this comment.
Just curious — Is there some reason for not moving these lines to the Brave Browser category?
There was a problem hiding this comment.
I don't remember...
Found ActivityWatch/activitywatch#461
Looks like we should merge: #283
But something stalled it.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on f53f8b8 in 1 minute and 4 seconds
More details
- Looked at
23lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. src/queries.ts:242:
- Assessed confidence :
100% - Grade:
0% - Comment:
The PR description mentions updates forLibreWolfusing regex, but the diff does not reflect any changes to theLibreWolfentry. Please ensure that the intended changes are included in the PR. - Reasoning:
The PR description mentions that thebrowser_appnamesmap has been updated to use regex for case-insensitive matching, including a new variant forLibreWolf. However, the diff provided does not show any changes related toLibreWolf. This could be an oversight or an error in the PR description. It's important to ensure that the changes mentioned in the PR description are reflected in the code changes.
Workflow ID: wflow_rESCZdsC2Un83gkA
You can customize Ellipsis with review rules, user-specific overrides, quiet mode, and more. See docs.
⌛ 13 days left in your free trial, upgrade for $20/seat/month or contact us.
|
Looks like I can't pass regexes? Makes sense if the mapping goes into the query... |
|
This will never work. |
Replaces #283
Summary:
Enhanced the
browser_appnamesmapping in/src/queries.tswith improved regex patterns for flexible, case-insensitive browser identification, including updates forLibreWolf,Brave, and other browsers.Key points:
browser_appnamesin/src/queries.tsto use regex for flexible, case-insensitive matching.LibreWolfwith new variantlibrewolf-default.Braveand other browsers.Generated with ❤️ by ellipsis.dev