-
Notifications
You must be signed in to change notification settings - Fork 4
RU-T46 Native PPT Android fix #200
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
Conversation
📝 WalkthroughWalkthroughThe Android application augmentation now extracts the package name from MainApplication.kt's first line and automatically inserts the MediaButtonPackage import statement after the package declaration if not already present, in addition to registering the package. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugins/withMediaButtonModule.js`:
- Around line 484-493: The current package detection uses a line-start regex
that fails if the file has a BOM, blank lines, or comments, causing the import
not to be inserted while still logging success; update the logic that computes
packageMatch/mainApplication.contents: use a multiline-tolerant regex (e.g.,
match first occurrence of /^\s*package\s+([^\s]+)/m) or locate the package
declaration with a search for "package " and capture its end position, then only
perform the replace/insert of importStatement and call
console.log('[withMediaButtonModule] Added MediaButtonPackage import') if the
contents were actually modified (i.e., packageMatch found and
mainApplication.contents changed); keep references to packageMatch, packageName,
importStatement and the replace insertion logic in withMediaButtonModule.
| // Get the package name from the first line of the file | ||
| const packageMatch = mainApplication.contents.match(/^package\s+([^\s]+)/); | ||
| const packageName = packageMatch ? packageMatch[1] : 'com.resgrid.unit'; | ||
|
|
||
| // Add import statement for MediaButtonPackage after the package declaration | ||
| const importStatement = `import ${packageName}.MediaButtonPackage`; | ||
| if (!mainApplication.contents.includes(importStatement)) { | ||
| // Add import after the package declaration line | ||
| mainApplication.contents = mainApplication.contents.replace(/^(package\s+[^\n]+\n)/, `$1${importStatement}\n`); | ||
| console.log('[withMediaButtonModule] Added MediaButtonPackage import'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle non-first-line package declarations and avoid false “import added” logs.
If MainApplication.kt starts with a BOM, blank line, or header comment, ^package won’t match, so the import won’t be inserted even though the log says it was. Consider a multiline-safe match and guard if the package declaration isn’t found.
🛠️ Suggested fix
- const packageMatch = mainApplication.contents.match(/^package\s+([^\s]+)/);
+ const packageMatch = mainApplication.contents.match(/^\s*package\s+([A-Za-z0-9_.]+)\s*$/m);
const packageName = packageMatch ? packageMatch[1] : 'com.resgrid.unit';
// Add import statement for MediaButtonPackage after the package declaration
const importStatement = `import ${packageName}.MediaButtonPackage`;
- if (!mainApplication.contents.includes(importStatement)) {
- // Add import after the package declaration line
- mainApplication.contents = mainApplication.contents.replace(/^(package\s+[^\n]+\n)/, `$1${importStatement}\n`);
- console.log('[withMediaButtonModule] Added MediaButtonPackage import');
- }
+ if (!mainApplication.contents.includes(importStatement)) {
+ if (packageMatch) {
+ // Add import after the package declaration line
+ mainApplication.contents = mainApplication.contents.replace(
+ /^\s*package\s+[^\n]+\n/m,
+ (match) => `${match}${importStatement}\n`
+ );
+ console.log('[withMediaButtonModule] Added MediaButtonPackage import');
+ } else {
+ console.warn('[withMediaButtonModule] Package declaration not found; skipped MediaButtonPackage import');
+ }
+ }🤖 Prompt for AI Agents
In `@plugins/withMediaButtonModule.js` around lines 484 - 493, The current package
detection uses a line-start regex that fails if the file has a BOM, blank lines,
or comments, causing the import not to be inserted while still logging success;
update the logic that computes packageMatch/mainApplication.contents: use a
multiline-tolerant regex (e.g., match first occurrence of
/^\s*package\s+([^\s]+)/m) or locate the package declaration with a search for
"package " and capture its end position, then only perform the replace/insert of
importStatement and call console.log('[withMediaButtonModule] Added
MediaButtonPackage import') if the contents were actually modified (i.e.,
packageMatch found and mainApplication.contents changed); keep references to
packageMatch, packageName, importStatement and the replace insertion logic in
withMediaButtonModule.
|
Approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is approved.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.