Feauture: Login screen implementation#551
Feauture: Login screen implementation#551raunaktastic wants to merge 3 commits intoAOSSIE-Org:masterfrom
Conversation
WalkthroughThis PR adds Marathi (mr) localization, updates localization scaffolding (including removing several update-related getters), wires supported locales in main, and introduces username availability guidance in Edit Profile. It also adds platform scaffolding for Linux, macOS, Windows, basic web shell files, iOS tweaks, tests, metadata updates, and a Flutter submodule bump. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Screen as EditProfileScreen (Stateful)
participant Checker as _checkUsernameAvailability(name)
participant UI as UI State
User->>Screen: Type in username
activate Screen
Screen->>Checker: validate(name)
activate Checker
alt available
Checker-->>Screen: _isUsernameAvailable = true
Screen->>UI: show check icon + "Available"
else unavailable
Checker-->>Screen: _isUsernameAvailable = false
Screen->>UI: show close icon + "Unavailable"
end
deactivate Checker
deactivate Screen
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
|
🎉 Welcome @raunaktastic!
We appreciate your contribution! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/views/screens/edit_profile_screen.dart (1)
52-63: Unsaved-changes dialog no longer fires on back navigation.Line 55 still blocks back navigation when there are unsaved changes, but the
onPopInvokedWithResultbody now only early-returns. The previoussaveChangesDialogue(context);call is gone, so users get stuck without feedback. Restore the dialog invocation so the UX matches the original behavior.onPopInvokedWithResult: (didPop, result) async { if (didPop) { return; } - + await saveChangesDialogue(context); },
🧹 Nitpick comments (8)
web/index.html (1)
21-33: Replace placeholder metadata stringsLines 21 and 32 still carry the stock Flutter copy (“A new Flutter project.” and the lower-case window title). That text leaks into search results, install prompts, and browser tabs, so we should swap it for the final Resonate marketing copy before launch.
web/manifest.json (1)
8-8: Update manifest description before shippingLine 8 still advertises “A new Flutter project.” When users install the PWA, that string appears in the install sheet and OS launcher. Let’s replace it with the actual Resonate description so the manifest reflects the product we’re releasing.
linux/runner/my_application.h (1)
4-18: Restore C linkage guardsPlease wrap the declarations in
G_BEGIN_DECLS/G_END_DECLS. Without them this header exports C++ linkage when included from C++, which breaks consumers that expect C ABI (e.g., if another C source includes it) and diverges from the standard Flutter template.#include <gtk/gtk.h> +G_BEGIN_DECLS + G_DECLARE_FINAL_TYPE(MyApplication, my_application, MY, APPLICATION, GtkApplication) MyApplication* my_application_new(); + +G_END_DECLSwindows/runner/utils.cpp (1)
3-9: Be explicit about STL includes used hereThis TU uses
std::vectorandstd::string. Ifutils.hdoesn’t include them, include locally to avoid transitive-include fragility.#include <flutter_windows.h> #include <io.h> #include <stdio.h> #include <windows.h> #include <iostream> +#include <string> +#include <vector>windows/runner/win32_window.h (2)
15-26: Use signed coords/sizes to support off-screen positioningNegative origins (e.g., spanning multi-monitor setups) are valid on Win32. Prefer
intoverunsigned intforPoint/Size.- struct Point { - unsigned int x; - unsigned int y; - Point(unsigned int x, unsigned int y) : x(x), y(y) {} - }; + struct Point { + int x; + int y; + Point(int x, int y) : x(x), y(y) {} + }; - struct Size { - unsigned int width; - unsigned int height; - Size(unsigned int width, unsigned int height) - : width(width), height(height) {} - }; + struct Size { + int width; + int height; + Size(int width, int height) : width(width), height(height) {} + };Note: This change will require corresponding updates in the .cpp (casts to LONG no longer needed).
48-56: Const-correct accessors
GetHandle()andGetClientArea()don’t modify state; mark themconstfor better API guarantees.- HWND GetHandle(); + HWND GetHandle() const; @@ - RECT GetClientArea(); + RECT GetClientArea() const;And in win32_window.cpp, update the definitions accordingly:
-RECT Win32Window::GetClientArea() { +RECT Win32Window::GetClientArea() const { @@ -HWND Win32Window::GetHandle() { +HWND Win32Window::GetHandle() const {windows/runner/win32_window.cpp (2)
152-154: Return a strict bool from Show()
ShowWindowreturns the previous visibility state (int). Normalize to bool for API clarity.-bool Win32Window::Show() { - return ShowWindow(window_handle_, SW_SHOWNORMAL); -} +bool Win32Window::Show() { + return ShowWindow(window_handle_, SW_SHOWNORMAL) != 0; +}
34-38: Prefer rounding when scaling integral sizesInteger truncation during scaling can introduce a 1px shrink. Consider rounding.
-int Scale(int source, double scale_factor) { - return static_cast<int>(source * scale_factor); -} +int Scale(int source, double scale_factor) { + return static_cast<int>(std::lround(source * scale_factor)); +}Note: Requires
#include <cmath>.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (28)
ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-1024x1024@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_1024.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_128.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_16.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_256.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_32.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_512.pngis excluded by!**/*.pngmacos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_64.pngis excluded by!**/*.pngweb/favicon.pngis excluded by!**/*.pngweb/icons/Icon-192.pngis excluded by!**/*.pngweb/icons/Icon-512.pngis excluded by!**/*.pngweb/icons/Icon-maskable-192.pngis excluded by!**/*.pngweb/icons/Icon-maskable-512.pngis excluded by!**/*.pngwindows/runner/resources/app_icon.icois excluded by!**/*.ico
📒 Files selected for processing (63)
.metadata(1 hunks)ios/Runner/Info.plist(1 hunks)ios/RunnerTests/RunnerTests.swift(1 hunks)lib/edit_profile_screen.dart(1 hunks)lib/flutter(1 hunks)lib/l10n/app_localizations.dart(4 hunks)lib/l10n/app_localizations_en.dart(0 hunks)lib/l10n/app_localizations_hi.dart(0 hunks)lib/l10n/app_localizations_mr.dart(1 hunks)lib/l10n/app_mr.arb(1 hunks)lib/main.dart(2 hunks)lib/views/screens/edit_profile_screen.dart(3 hunks)linux/.gitignore(1 hunks)linux/CMakeLists.txt(1 hunks)linux/flutter/CMakeLists.txt(1 hunks)linux/flutter/generated_plugin_registrant.cc(1 hunks)linux/flutter/generated_plugin_registrant.h(1 hunks)linux/flutter/generated_plugins.cmake(1 hunks)linux/runner/CMakeLists.txt(1 hunks)linux/runner/main.cc(1 hunks)linux/runner/my_application.cc(1 hunks)linux/runner/my_application.h(1 hunks)macos/.gitignore(1 hunks)macos/Flutter/Flutter-Debug.xcconfig(1 hunks)macos/Flutter/Flutter-Release.xcconfig(1 hunks)macos/Flutter/GeneratedPluginRegistrant.swift(1 hunks)macos/Runner.xcodeproj/project.pbxproj(1 hunks)macos/Runner.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist(1 hunks)macos/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme(1 hunks)macos/Runner.xcworkspace/contents.xcworkspacedata(1 hunks)macos/Runner.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist(1 hunks)macos/Runner/AppDelegate.swift(1 hunks)macos/Runner/Assets.xcassets/AppIcon.appiconset/Contents.json(1 hunks)macos/Runner/Base.lproj/MainMenu.xib(1 hunks)macos/Runner/Configs/AppInfo.xcconfig(1 hunks)macos/Runner/Configs/Debug.xcconfig(1 hunks)macos/Runner/Configs/Release.xcconfig(1 hunks)macos/Runner/Configs/Warnings.xcconfig(1 hunks)macos/Runner/DebugProfile.entitlements(1 hunks)macos/Runner/Info.plist(1 hunks)macos/Runner/MainFlutterWindow.swift(1 hunks)macos/Runner/Release.entitlements(1 hunks)macos/RunnerTests/RunnerTests.swift(1 hunks)test/widget_test.dart(1 hunks)web/index.html(1 hunks)web/manifest.json(1 hunks)windows/.gitignore(1 hunks)windows/CMakeLists.txt(1 hunks)windows/flutter/CMakeLists.txt(1 hunks)windows/flutter/generated_plugin_registrant.cc(1 hunks)windows/flutter/generated_plugin_registrant.h(1 hunks)windows/flutter/generated_plugins.cmake(1 hunks)windows/runner/CMakeLists.txt(1 hunks)windows/runner/Runner.rc(1 hunks)windows/runner/flutter_window.cpp(1 hunks)windows/runner/flutter_window.h(1 hunks)windows/runner/main.cpp(1 hunks)windows/runner/resource.h(1 hunks)windows/runner/runner.exe.manifest(1 hunks)windows/runner/utils.cpp(1 hunks)windows/runner/utils.h(1 hunks)windows/runner/win32_window.cpp(1 hunks)windows/runner/win32_window.h(1 hunks)
💤 Files with no reviewable changes (2)
- lib/l10n/app_localizations_en.dart
- lib/l10n/app_localizations_hi.dart
🧰 Additional context used
🧬 Code graph analysis (14)
linux/flutter/generated_plugin_registrant.h (1)
linux/flutter/generated_plugin_registrant.cc (2)
fl_register_plugins(20-51)fl_register_plugins(20-20)
windows/flutter/generated_plugin_registrant.h (1)
windows/flutter/generated_plugin_registrant.cc (2)
RegisterPlugins(23-50)RegisterPlugins(23-23)
ios/RunnerTests/RunnerTests.swift (1)
macos/RunnerTests/RunnerTests.swift (1)
testExample(7-10)
windows/runner/flutter_window.h (3)
windows/runner/flutter_window.cpp (8)
FlutterWindow(7-8)FlutterWindow(10-10)OnCreate(12-40)OnCreate(12-12)OnDestroy(42-48)OnDestroy(42-42)MessageHandler(50-71)MessageHandler(51-53)windows/runner/win32_window.cpp (8)
Win32Window(114-116)Win32Window(118-121)OnCreate(266-269)OnCreate(266-266)OnDestroy(271-273)OnDestroy(271-271)MessageHandler(176-222)MessageHandler(177-180)windows/runner/main.cpp (1)
window(27-27)
windows/runner/utils.h (1)
windows/runner/utils.cpp (6)
CreateAndAttachConsole(10-22)CreateAndAttachConsole(10-10)Utf8FromUtf16(44-65)Utf8FromUtf16(44-44)GetCommandLineArguments(24-42)GetCommandLineArguments(24-24)
windows/runner/flutter_window.cpp (3)
windows/runner/flutter_window.h (1)
FlutterWindow(12-31)windows/runner/win32_window.cpp (10)
OnCreate(266-269)OnCreate(266-266)GetClientArea(252-256)GetClientArea(252-252)SetChildContent(241-250)SetChildContent(241-241)OnDestroy(271-273)OnDestroy(271-271)MessageHandler(176-222)MessageHandler(177-180)windows/flutter/generated_plugin_registrant.cc (2)
RegisterPlugins(23-50)RegisterPlugins(23-23)
linux/runner/main.cc (1)
linux/runner/my_application.cc (2)
my_application_new(133-144)my_application_new(133-133)
macos/Runner/MainFlutterWindow.swift (1)
macos/Flutter/GeneratedPluginRegistrant.swift (1)
RegisterGeneratedPlugins(32-56)
macos/RunnerTests/RunnerTests.swift (1)
ios/RunnerTests/RunnerTests.swift (1)
testExample(7-10)
windows/runner/main.cpp (1)
windows/runner/utils.cpp (4)
CreateAndAttachConsole(10-22)CreateAndAttachConsole(10-10)GetCommandLineArguments(24-42)GetCommandLineArguments(24-24)
linux/runner/my_application.h (1)
linux/runner/my_application.cc (2)
my_application_new(133-144)my_application_new(133-133)
linux/runner/my_application.cc (1)
linux/flutter/generated_plugin_registrant.cc (2)
fl_register_plugins(20-51)fl_register_plugins(20-20)
windows/runner/win32_window.h (1)
windows/runner/win32_window.cpp (2)
Win32Window(114-116)Win32Window(118-121)
windows/runner/win32_window.cpp (1)
windows/runner/flutter_window.cpp (6)
OnCreate(12-40)OnCreate(12-12)MessageHandler(50-71)MessageHandler(51-53)OnDestroy(42-48)OnDestroy(42-42)
🪛 RuboCop (1.80.2)
lib/l10n/app_mr.arb
[warning] 611-611: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 612-612: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 613-613: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 614-614: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 615-615: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 616-616: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 617-617: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 618-618: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 619-619: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 620-620: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 621-621: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 622-622: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 623-623: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 624-624: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 625-625: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 626-626: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 627-627: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 628-628: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 629-629: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 630-630: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
🔇 Additional comments (13)
macos/Runner/Configs/AppInfo.xcconfig (1)
8-14: macOS bundle metadata looks consistentThe product name, bundle identifier, and copyright values align with platform expectations and match the Resonate naming. No changes required.
macos/Runner/Assets.xcassets/AppIcon.appiconset/Contents.json (1)
3-62: AppIcon catalog covers the required macOS sizesAll expected mac idiom entries (16–512 with their @2x counterparts) are present and mapped to consistent filenames, so the asset catalog should import cleanly into Xcode. Nicely done.
macos/Runner.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist (1)
1-7: Workspace check metadata OKCapturing the computed 32-bit warning state here is harmless and avoids Xcode from regenerating the file.
macos/.gitignore (1)
1-7: Ignore rules cover Flutter/macOS artifactsThese patterns keep Pods, Flutter ephemeral outputs, and user-specific Xcode data out of version control.
macos/Runner.xcworkspace/contents.xcworkspacedata (1)
1-7: Workspace reference looks goodPointing the workspace at
Runner.xcodeprojis the expected setup for the macOS host project.ios/Runner/Info.plist (1)
7-12: iOS now advertises Marathi localeAdding
mrtoCFBundleLocalizationsaligns iOS metadata with the new Marathi localization assets.macos/Runner.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist (1)
1-7: Workspace check plist is fineIncluding this shared data plist prevents Xcode from reintroducing the 32-bit warning prompt.
linux/runner/main.cc (1)
3-5: LGTM!The new Linux entry point cleanly instantiates and runs
MyApplication, and theg_autoptrusage takes care of cleanup. No further action needed.macos/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme (1)
5-51: Scheme wiring looks correct.Build/Test/Launch actions all reference the Runner target and its macOS test bundle with expected LLDB defaults, so the new scheme should integrate cleanly into the project workflows.
macos/Runner/AppDelegate.swift (1)
4-12: App delegate setup aligns with Flutter defaults.Returning true from the termination and secure state overrides matches the standard Flutter macOS template, so lifecycle behavior remains predictable.
linux/flutter/generated_plugin_registrant.cc (1)
20-50: Plugin registrant matches the declared dependencies.Each plugin is registered through the expected
fl_plugin_registry_get_registrar_for_pluginflow, so Linux builds should load the new desktop plugins without additional wiring.windows/CMakeLists.txt (1)
40-108: CMake scaffolding mirrors Flutter’s template.The
APPLY_STANDARD_SETTINGShelper, install targets, and native-asset wiring align with recent Flutter desktop projects, so the Windows build pipeline should function as intended.linux/flutter/CMakeLists.txt (1)
24-88: Flutter tool integration looks good.PkgConfig setup, the exported interface library, and the
flutter_assemblebackend hook follow the upstream Linux template, keeping Flutter-generated artifacts in sync.
| String friendRequestAcceptedTo(String username) { | ||
| return 'तुम्ही आता \$$username चे मित्र आहात.'; | ||
| } | ||
|
|
||
| @override | ||
| String get friendRequestDeclined => 'मित्र विनंती नाकारली'; | ||
|
|
||
| @override | ||
| String friendRequestDeclinedTo(String username) { | ||
| return 'तुम्ही \$$username कडून आलेली मित्र विनंती नाकारली आहे.'; | ||
| } | ||
|
|
||
| @override | ||
| String get accept => 'स्वीकारा'; | ||
|
|
||
| @override | ||
| String get callDeclined => 'कॉल नाकारला'; | ||
|
|
||
| @override | ||
| String callDeclinedTo(String username) { | ||
| return 'वापरकर्ता \$$username ने कॉल नाकारला.'; | ||
| } |
There was a problem hiding this comment.
Fix interpolation escaping in Marathi friend-request strings
The \$$username occurrences render a literal $ before the username (e.g., “$Aarav”) because \$ escapes the dollar sign. We only want the interpolated name, so please drop the backslash in all three methods.
- return 'तुम्ही आता \$$username चे मित्र आहात.';
+ return 'तुम्ही आता $username चे मित्र आहात.';
...
- return 'तुम्ही \$$username कडून आलेली मित्र विनंती नाकारली आहे.';
+ return 'तुम्ही $username कडून आलेली मित्र विनंती नाकारली आहे.';
...
- return 'वापरकर्ता \$$username ने कॉल नाकारला.';
+ return 'वापरकर्ता $username ने कॉल नाकारला.';📝 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.
| String friendRequestAcceptedTo(String username) { | |
| return 'तुम्ही आता \$$username चे मित्र आहात.'; | |
| } | |
| @override | |
| String get friendRequestDeclined => 'मित्र विनंती नाकारली'; | |
| @override | |
| String friendRequestDeclinedTo(String username) { | |
| return 'तुम्ही \$$username कडून आलेली मित्र विनंती नाकारली आहे.'; | |
| } | |
| @override | |
| String get accept => 'स्वीकारा'; | |
| @override | |
| String get callDeclined => 'कॉल नाकारला'; | |
| @override | |
| String callDeclinedTo(String username) { | |
| return 'वापरकर्ता \$$username ने कॉल नाकारला.'; | |
| } | |
| String friendRequestAcceptedTo(String username) { | |
| return 'तुम्ही आता $username चे मित्र आहात.'; | |
| } | |
| @override | |
| String get friendRequestDeclined => 'मित्र विनंती नाकारली'; | |
| @override | |
| String friendRequestDeclinedTo(String username) { | |
| return 'तुम्ही $username कडून आलेली मित्र विनंती नाकारली आहे.'; | |
| } | |
| @override | |
| String get accept => 'स्वीकारा'; | |
| @override | |
| String get callDeclined => 'कॉल नाकारला'; | |
| @override | |
| String callDeclinedTo(String username) { | |
| return 'वापरकर्ता $username ने कॉल नाकारला.'; | |
| } |
🤖 Prompt for AI Agents
In lib/l10n/app_localizations_mr.dart around lines 1176 to 1197 the Marathi
strings use escaped dollar signs (\$username) which render a literal "$" before
the name; remove the backslash so the templates use $username for proper Dart
string interpolation in all three methods (friendRequestAcceptedTo,
friendRequestDeclinedTo, callDeclinedTo) and keep the surrounding text
unchanged.
| "friendRequestAcceptedTo": "तुम्ही आता ${username} चे मित्र आहात.", | ||
| "@friendRequestAcceptedTo": { | ||
| "description": "जेव्हा मित्र विनंती स्वीकारली जाते तेव्हा संदेश", | ||
| "placeholders": { | ||
| "username": { | ||
| "type": "String", | ||
| "example": "john_doe" | ||
| } | ||
| } | ||
| }, | ||
| "friendRequestDeclined": "मित्र विनंती नाकारली", | ||
| "friendRequestDeclinedTo": "तुम्ही ${username} कडून आलेली मित्र विनंती नाकारली आहे.", | ||
| "@friendRequestDeclinedTo": { | ||
| "description": "जेव्हा मित्र विनंती नाकारली जाते तेव्हा संदेश", | ||
| "placeholders": { | ||
| "username": { | ||
| "type": "String", | ||
| "example": "john_doe" | ||
| } | ||
| } | ||
| }, | ||
| "accept": "स्वीकारा", | ||
| "callDeclined": "कॉल नाकारला", | ||
| "callDeclinedTo": "वापरकर्ता ${username} ने कॉल नाकारला.", | ||
| "@callDeclinedTo": { | ||
| "description": "जेव्हा कॉल नाकारला जातो तेव्हा संदेश", |
There was a problem hiding this comment.
Fix placeholder interpolation for username strings.
These messages use ${username}, which the ICU/ARB tooling will treat as a literal string. This breaks flutter gen-l10n: the placeholder defined in @friendRequestAcceptedTo/@friendRequestDeclinedTo/@callDeclinedTo is never actually referenced, so code generation fails. Replace the ${...} syntax with {username} to hook up the runtime substitution.
- "friendRequestAcceptedTo": "तुम्ही आता ${username} चे मित्र आहात.",
+ "friendRequestAcceptedTo": "तुम्ही आता {username} चे मित्र आहात.",
@@
- "friendRequestDeclinedTo": "तुम्ही ${username} कडून आलेली मित्र विनंती नाकारली आहे.",
+ "friendRequestDeclinedTo": "तुम्ही {username} कडून आलेली मित्र विनंती नाकारली आहे.",
@@
- "callDeclinedTo": "वापरकर्ता ${username} ने कॉल नाकारला.",
+ "callDeclinedTo": "वापरकर्ता {username} ने कॉल नाकारला.",📝 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.
| "friendRequestAcceptedTo": "तुम्ही आता ${username} चे मित्र आहात.", | |
| "@friendRequestAcceptedTo": { | |
| "description": "जेव्हा मित्र विनंती स्वीकारली जाते तेव्हा संदेश", | |
| "placeholders": { | |
| "username": { | |
| "type": "String", | |
| "example": "john_doe" | |
| } | |
| } | |
| }, | |
| "friendRequestDeclined": "मित्र विनंती नाकारली", | |
| "friendRequestDeclinedTo": "तुम्ही ${username} कडून आलेली मित्र विनंती नाकारली आहे.", | |
| "@friendRequestDeclinedTo": { | |
| "description": "जेव्हा मित्र विनंती नाकारली जाते तेव्हा संदेश", | |
| "placeholders": { | |
| "username": { | |
| "type": "String", | |
| "example": "john_doe" | |
| } | |
| } | |
| }, | |
| "accept": "स्वीकारा", | |
| "callDeclined": "कॉल नाकारला", | |
| "callDeclinedTo": "वापरकर्ता ${username} ने कॉल नाकारला.", | |
| "@callDeclinedTo": { | |
| "description": "जेव्हा कॉल नाकारला जातो तेव्हा संदेश", | |
| "friendRequestAcceptedTo": "तुम्ही आता {username} चे मित्र आहात.", | |
| "@friendRequestAcceptedTo": { | |
| "description": "जेव्हा मित्र विनंती स्वीकारली जाते तेव्हा संदेश", | |
| "placeholders": { | |
| "username": { | |
| "type": "String", | |
| "example": "john_doe" | |
| } | |
| } | |
| }, | |
| "friendRequestDeclined": "मित्र विनंती नाकारली", | |
| "friendRequestDeclinedTo": "तुम्ही {username} कडून आलेली मित्र विनंती नाकारली आहे.", | |
| "@friendRequestDeclinedTo": { | |
| "description": "जेव्हा मित्र विनंती नाकारली जाते तेव्हा संदेश", | |
| "placeholders": { | |
| "username": { | |
| "type": "String", | |
| "example": "john_doe" | |
| } | |
| } | |
| }, | |
| "accept": "स्वीकारा", | |
| "callDeclined": "कॉल नाकारला", | |
| "callDeclinedTo": "वापरकर्ता {username} ने कॉल नाकारला.", | |
| "@callDeclinedTo": { | |
| "description": "जेव्हा कॉल नाकारला जातो तेव्हा संदेश", |
🤖 Prompt for AI Agents
In lib/l10n/app_mr.arb around lines 578 to 603 the message strings use
JavaScript-style interpolation `${username}` which is not recognized by ICU/ARB
and prevents flutter gen-l10n from connecting the defined placeholders; replace
all occurrences of `${username}` in the affected message values
(friendRequestAcceptedTo, friendRequestDeclinedTo, callDeclinedTo) with ICU
placeholder syntax `{username}` so the existing @... placeholder metadata is
correctly referenced and code generation succeeds.
| "checkForUpdates": "अपडेट तपासा", | ||
| "updateNow": "आता अपडेट करा", | ||
| "updateLater": "नंतर", | ||
| "updateSuccessful": "अपडेट यशस्वी", | ||
| "updateSuccessfulMessage": "Resonate यशस्वीरित्या अपडेट झाले आहे!", | ||
| "updateCancelled": "अपडेट रद्द केले", | ||
| "updateCancelledMessage": "वापरकर्त्याने अपडेट रद्द केले", | ||
| "updateFailed": "अपडेट अयशस्वी", | ||
| "updateFailedMessage": "अपडेट करण्यात अयशस्वी. कृपया Play Store वरून हाताने अपडेट करा.", | ||
| "updateError": "अपडेट त्रुटी", | ||
| "updateErrorMessage": "अपडेट करताना त्रुटी आली. कृपया पुन्हा प्रयत्न करा.", | ||
| "platformNotSupported": "प्लॅटफॉर्म समर्थित नाही", | ||
| "platformNotSupportedMessage": "अपडेट तपासणी फक्त Android डिव्हाइसेसवर उपलब्ध आहे", | ||
| "updateCheckFailed": "अपडेट तपासणी अयशस्वी", | ||
| "updateCheckFailedMessage": "अपडेट तपासता आले नाही. कृपया नंतर प्रयत्न करा.", | ||
| "upToDateTitle": "तुमचे अॅप अद्ययावत आहे!", | ||
| "upToDateMessage": "तुम्ही Resonate ची नवीनतम आवृत्ती वापरत आहात", | ||
| "updateAvailableTitle": "नवीन अपडेट उपलब्ध!", | ||
| "updateAvailableMessage": "Resonate ची नवीन आवृत्ती Play Store वर उपलब्ध आहे", | ||
| "updateFeaturesImprovement": "नवीन वैशिष्ट्ये आणि सुधारणा मिळवा!" |
There was a problem hiding this comment.
Remove the duplicated update-status keys.
This block redefines the same keys that already exist a few lines above. Because JSON keeps only the last value, the earlier translations (with their metadata) are silently overridden, which can break placeholder validation down the line and makes the ARB harder to maintain. Drop the duplicate entries.
- "checkForUpdates": "अपडेट तपासा",
- "updateNow": "आता अपडेट करा",
- "updateLater": "नंतर",
- "updateSuccessful": "अपडेट यशस्वी",
- "updateSuccessfulMessage": "Resonate यशस्वीरित्या अपडेट झाले आहे!",
- "updateCancelled": "अपडेट रद्द केले",
- "updateCancelledMessage": "वापरकर्त्याने अपडेट रद्द केले",
- "updateFailed": "अपडेट अयशस्वी",
- "updateFailedMessage": "अपडेट करण्यात अयशस्वी. कृपया Play Store वरून हाताने अपडेट करा.",
- "updateError": "अपडेट त्रुटी",
- "updateErrorMessage": "अपडेट करताना त्रुटी आली. कृपया पुन्हा प्रयत्न करा.",
- "platformNotSupported": "प्लॅटफॉर्म समर्थित नाही",
- "platformNotSupportedMessage": "अपडेट तपासणी फक्त Android डिव्हाइसेसवर उपलब्ध आहे",
- "updateCheckFailed": "अपडेट तपासणी अयशस्वी",
- "updateCheckFailedMessage": "अपडेट तपासता आले नाही. कृपया नंतर प्रयत्न करा.",
- "upToDateTitle": "तुमचे अॅप अद्ययावत आहे!",
- "upToDateMessage": "तुम्ही Resonate ची नवीनतम आवृत्ती वापरत आहात",
- "updateAvailableTitle": "नवीन अपडेट उपलब्ध!",
- "updateAvailableMessage": "Resonate ची नवीन आवृत्ती Play Store वर उपलब्ध आहे",
- "updateFeaturesImprovement": "नवीन वैशिष्ट्ये आणि सुधारणा मिळवा!"📝 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.
| "checkForUpdates": "अपडेट तपासा", | |
| "updateNow": "आता अपडेट करा", | |
| "updateLater": "नंतर", | |
| "updateSuccessful": "अपडेट यशस्वी", | |
| "updateSuccessfulMessage": "Resonate यशस्वीरित्या अपडेट झाले आहे!", | |
| "updateCancelled": "अपडेट रद्द केले", | |
| "updateCancelledMessage": "वापरकर्त्याने अपडेट रद्द केले", | |
| "updateFailed": "अपडेट अयशस्वी", | |
| "updateFailedMessage": "अपडेट करण्यात अयशस्वी. कृपया Play Store वरून हाताने अपडेट करा.", | |
| "updateError": "अपडेट त्रुटी", | |
| "updateErrorMessage": "अपडेट करताना त्रुटी आली. कृपया पुन्हा प्रयत्न करा.", | |
| "platformNotSupported": "प्लॅटफॉर्म समर्थित नाही", | |
| "platformNotSupportedMessage": "अपडेट तपासणी फक्त Android डिव्हाइसेसवर उपलब्ध आहे", | |
| "updateCheckFailed": "अपडेट तपासणी अयशस्वी", | |
| "updateCheckFailedMessage": "अपडेट तपासता आले नाही. कृपया नंतर प्रयत्न करा.", | |
| "upToDateTitle": "तुमचे अॅप अद्ययावत आहे!", | |
| "upToDateMessage": "तुम्ही Resonate ची नवीनतम आवृत्ती वापरत आहात", | |
| "updateAvailableTitle": "नवीन अपडेट उपलब्ध!", | |
| "updateAvailableMessage": "Resonate ची नवीन आवृत्ती Play Store वर उपलब्ध आहे", | |
| "updateFeaturesImprovement": "नवीन वैशिष्ट्ये आणि सुधारणा मिळवा!" |
🧰 Tools
🪛 RuboCop (1.80.2)
[warning] 611-611: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 612-612: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 613-613: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 614-614: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 615-615: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 616-616: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 617-617: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 618-618: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 619-619: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 620-620: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 621-621: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 622-622: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 623-623: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 624-624: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 625-625: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 626-626: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 627-627: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 628-628: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 629-629: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 630-630: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
🤖 Prompt for AI Agents
In lib/l10n/app_mr.arb around lines 611 to 630 there are duplicated
update-status keys that override earlier entries and lose their metadata; remove
these duplicate keys (e.g., checkForUpdates, updateNow, updateLater,
updateSuccessful, updateSuccessfulMessage, updateCancelled,
updateCancelledMessage, updateFailed, updateFailedMessage, updateError,
updateErrorMessage, platformNotSupported, platformNotSupportedMessage,
updateCheckFailed, updateCheckFailedMessage, upToDateTitle, upToDateMessage,
updateAvailableTitle, updateAvailableMessage, updateFeaturesImprovement) from
this block so only the original definitions (with their metadata) remain earlier
in the file, ensuring no translations or placeholders are accidentally dropped.
| import 'package:flutter/material.dart'; | ||
| void main() { | ||
| runApp(MyApp()); | ||
| } | ||
| class MyApp extends StatelessWidget { | ||
| @override | ||
| Widget build(BuildContext context) { | ||
| return MaterialApp( | ||
| home: Scaffold( | ||
| appBar: AppBar( | ||
| title: Text('My First App'), | ||
| ), | ||
| body: Center( | ||
| child: Text('Hello World!'), | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove the duplicate bootstrap main/MyApp stub.
Lines 1-17 reintroduce a bare main()/MyApp pair that collides with the real async main() and the GetX-powered MyApp declared later in the same file, so the analyzer will fail with “Error: The name 'main' is already defined” (and the class definition conflicts too). Please drop this template block so only the actual application entry point remains.
Apply this diff to resolve the conflict:
-import 'package:flutter/material.dart';
-void main() {
- runApp(MyApp());
-}
-class MyApp extends StatelessWidget {
- @override
- Widget build(BuildContext context) {
- return MaterialApp(
- home: Scaffold(
- appBar: AppBar(
- title: Text('My First App'),
- ),
- body: Center(
- child: Text('Hello World!'),
- ),
- ),
- );
- }
-}📝 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.
| import 'package:flutter/material.dart'; | |
| void main() { | |
| runApp(MyApp()); | |
| } | |
| class MyApp extends StatelessWidget { | |
| @override | |
| Widget build(BuildContext context) { | |
| return MaterialApp( | |
| home: Scaffold( | |
| appBar: AppBar( | |
| title: Text('My First App'), | |
| ), | |
| body: Center( | |
| child: Text('Hello World!'), | |
| ), | |
| ), | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/main.dart around lines 1 to 19, there is a duplicate bootstrap
main()/MyApp stub that conflicts with the real async main() and GetX-powered
MyApp defined later; remove the entire initial stub (the import, the simple
main(), and the MyApp StatelessWidget) so only the actual application entry
point and GetX MyApp remain in the file.
| locale: locale('mr'), | ||
| localizationsDelegates: [ | ||
| AppLocalizations.delegate, | ||
| GlobalMaterialLocalizations.delegate, | ||
| GlobalWidgetsLocalizations.delegate, | ||
| GlobalCupertinoLocalizations.delegate, | ||
| ], | ||
| supportedLocales: [Locale('en'), Locale('hi')], | ||
| supportedLocales: [Locale('en'), Locale('hi') Locale('mr')], | ||
| debugShowCheckedModeBanner: false, |
There was a problem hiding this comment.
Fix locale wiring syntax/compile errors.
locale: locale('mr') calls an undefined function (should be the Locale constructor), and supportedLocales is missing a comma before the Marathi locale, both of which currently fail to compile. Update these lines to use const Locale('mr') and add the missing comma.
Apply this diff to compile cleanly:
- locale: locale('mr'),
+ locale: const Locale('mr'),
@@
- supportedLocales: [Locale('en'), Locale('hi') Locale('mr')],
+ supportedLocales: const [Locale('en'), Locale('hi'), Locale('mr')],📝 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.
| locale: locale('mr'), | |
| localizationsDelegates: [ | |
| AppLocalizations.delegate, | |
| GlobalMaterialLocalizations.delegate, | |
| GlobalWidgetsLocalizations.delegate, | |
| GlobalCupertinoLocalizations.delegate, | |
| ], | |
| supportedLocales: [Locale('en'), Locale('hi')], | |
| supportedLocales: [Locale('en'), Locale('hi') Locale('mr')], | |
| debugShowCheckedModeBanner: false, | |
| locale: const Locale('mr'), | |
| localizationsDelegates: [ | |
| AppLocalizations.delegate, | |
| GlobalMaterialLocalizations.delegate, | |
| GlobalWidgetsLocalizations.delegate, | |
| GlobalCupertinoLocalizations.delegate, | |
| ], | |
| supportedLocales: const [Locale('en'), Locale('hi'), Locale('mr')], | |
| debugShowCheckedModeBanner: false, |
🤖 Prompt for AI Agents
In lib/main.dart around lines 91 to 99, the code uses an undefined function and
has a missing comma which prevents compilation: change the call from
locale('mr') to the Locale constructor (use const Locale('mr') for immutability)
and add the missing comma between Locale('hi') and Locale('mr') in
supportedLocales so the list is valid; ensure supportedLocales entries use
Locale (optionally const) and the final trailing comma/formatting is correct.
| // Initialize COM, so that it is available for use in the library and/or | ||
| // plugins. | ||
| ::CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED); | ||
|
|
||
| flutter::DartProject project(L"data"); | ||
|
|
||
| std::vector<std::string> command_line_arguments = | ||
| GetCommandLineArguments(); | ||
|
|
||
| project.set_dart_entrypoint_arguments(std::move(command_line_arguments)); | ||
|
|
||
| FlutterWindow window(project); | ||
| Win32Window::Point origin(10, 10); | ||
| Win32Window::Size size(1280, 720); | ||
| if (!window.Create(L"resonate", origin, size)) { | ||
| return EXIT_FAILURE; | ||
| } | ||
| window.SetQuitOnClose(true); |
There was a problem hiding this comment.
Balance COM initialization on every exit path
We call CoInitializeEx but bail out via return EXIT_FAILURE; if window.Create(...) fails, leaving COM initialized. That leaks the apartment setup for the lifetime of the process and violates the required init/uninit pairing. Please track whether COM was initialized successfully and invoke CoUninitialize() before each exit, including the failure branch. This also guards us from calling CoUninitialize() when CoInitializeEx fails (e.g., RPC_E_CHANGED_MODE).
- ::CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED);
+ const HRESULT coinit_result =
+ ::CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED);
+ const bool com_initialized = SUCCEEDED(coinit_result);
+
+ if (FAILED(coinit_result) && coinit_result != RPC_E_CHANGED_MODE) {
+ return EXIT_FAILURE;
+ }
...
if (!window.Create(L"resonate", origin, size)) {
- return EXIT_FAILURE;
+ if (com_initialized) {
+ ::CoUninitialize();
+ }
+ return EXIT_FAILURE;
}
...
- ::CoUninitialize();
+ if (com_initialized) {
+ ::CoUninitialize();
+ }🤖 Prompt for AI Agents
In windows/runner/main.cpp around lines 16-33, CoInitializeEx is called but not
paired with CoUninitialize on the failure path; capture and check the HRESULT
returned by CoInitializeEx (or set a bool flag when initialization succeeds) and
ensure CoUninitialize() is called before every exit path (both the early
EXIT_FAILURE after window.Create(...) fails and the normal process exit), and
only call CoUninitialize() if initialization succeeded (to avoid unbalanced
uninit on failures like RPC_E_CHANGED_MODE).
| if (freopen_s(&unused, "CONOUT$", "w", stdout)) { | ||
| _dup2(_fileno(stdout), 1); | ||
| } | ||
| if (freopen_s(&unused, "CONOUT$", "w", stderr)) { | ||
| _dup2(_fileno(stdout), 2); | ||
| } |
There was a problem hiding this comment.
stderr redirection bug; duplicate wrong FD
_dup2(_fileno(stdout), 2) duplicates stdout into FD 2. It should use stderr. Also, make the freopen_s checks explicit (!= 0) for clarity.
- if (freopen_s(&unused, "CONOUT$", "w", stdout)) {
- _dup2(_fileno(stdout), 1);
- }
- if (freopen_s(&unused, "CONOUT$", "w", stderr)) {
- _dup2(_fileno(stdout), 2);
- }
+ if (freopen_s(&unused, "CONOUT$", "w", stdout) != 0) {
+ _dup2(_fileno(stdout), 1);
+ }
+ if (freopen_s(&unused, "CONOUT$", "w", stderr) != 0) {
+ _dup2(_fileno(stderr), 2);
+ }📝 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 (freopen_s(&unused, "CONOUT$", "w", stdout)) { | |
| _dup2(_fileno(stdout), 1); | |
| } | |
| if (freopen_s(&unused, "CONOUT$", "w", stderr)) { | |
| _dup2(_fileno(stdout), 2); | |
| } | |
| if (freopen_s(&unused, "CONOUT$", "w", stdout) != 0) { | |
| _dup2(_fileno(stdout), 1); | |
| } | |
| if (freopen_s(&unused, "CONOUT$", "w", stderr) != 0) { | |
| _dup2(_fileno(stderr), 2); | |
| } |
🤖 Prompt for AI Agents
In windows/runner/utils.cpp around lines 13 to 18, the stderr redirection
duplicates the wrong file descriptor and uses implicit truthiness for freopen_s
results; change the freopen_s conditionals to explicit comparisons against 0
(e.g. freopen_s(...) != 0) and when redirecting stderr call
_dup2(_fileno(stderr), 2) instead of _dup2(_fileno(stdout), 2) so FD 2 correctly
points to stderr.
| std::string Utf8FromUtf16(const wchar_t* utf16_string) { | ||
| if (utf16_string == nullptr) { | ||
| return std::string(); | ||
| } | ||
| unsigned int target_length = ::WideCharToMultiByte( | ||
| CP_UTF8, WC_ERR_INVALID_CHARS, utf16_string, | ||
| -1, nullptr, 0, nullptr, nullptr) | ||
| -1; // remove the trailing null character | ||
| int input_length = (int)wcslen(utf16_string); | ||
| std::string utf8_string; | ||
| if (target_length == 0 || target_length > utf8_string.max_size()) { | ||
| return utf8_string; | ||
| } | ||
| utf8_string.resize(target_length); | ||
| int converted_length = ::WideCharToMultiByte( | ||
| CP_UTF8, WC_ERR_INVALID_CHARS, utf16_string, | ||
| input_length, utf8_string.data(), target_length, nullptr, nullptr); | ||
| if (converted_length == 0) { | ||
| return std::string(); | ||
| } | ||
| return utf8_string; | ||
| } |
There was a problem hiding this comment.
Utf8FromUtf16 can underflow and mishandle edge cases
WideCharToMultiByte(...)-1 stored in unsigned int can underflow on error; the two-pass logic also mixes “include NUL” and “exclude NUL”. Use a safer two-pass flow that sizes with -1, then trims the trailing NUL.
-std::string Utf8FromUtf16(const wchar_t* utf16_string) {
- if (utf16_string == nullptr) {
- return std::string();
- }
- unsigned int target_length = ::WideCharToMultiByte(
- CP_UTF8, WC_ERR_INVALID_CHARS, utf16_string,
- -1, nullptr, 0, nullptr, nullptr)
- -1; // remove the trailing null character
- int input_length = (int)wcslen(utf16_string);
- std::string utf8_string;
- if (target_length == 0 || target_length > utf8_string.max_size()) {
- return utf8_string;
- }
- utf8_string.resize(target_length);
- int converted_length = ::WideCharToMultiByte(
- CP_UTF8, WC_ERR_INVALID_CHARS, utf16_string,
- input_length, utf8_string.data(), target_length, nullptr, nullptr);
- if (converted_length == 0) {
- return std::string();
- }
- return utf8_string;
-}
+std::string Utf8FromUtf16(const wchar_t* utf16_string) {
+ if (!utf16_string) return {};
+ // Query required bytes including trailing NUL.
+ int required = ::WideCharToMultiByte(
+ CP_UTF8, WC_ERR_INVALID_CHARS, utf16_string, -1, nullptr, 0, nullptr, nullptr);
+ if (required <= 1) return {}; // 0=error, 1=only NUL
+ std::string utf8(required - 1, '\0'); // exclude trailing NUL
+ int written = ::WideCharToMultiByte(
+ CP_UTF8, WC_ERR_INVALID_CHARS, utf16_string, -1, utf8.data(), required, nullptr, nullptr);
+ if (written <= 1) return {};
+ utf8.resize(written - 1);
+ return utf8;
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In windows/runner/utils.cpp around lines 44-65, the current Utf8FromUtf16
implementation subtracts 1 from WideCharToMultiByte’s return into an unsigned
int (causing underflow on error) and mixes calls that include/exclude the
terminating NUL; change to a safe two-pass pattern: call WideCharToMultiByte
with cbMultiByte = 0 and cchWideChar = -1 to obtain the required size (check for
0 and handle error), store that size in a signed/size_t variable without
subtracting, allocate a std::string of size (required_size - 1) or reserve
required_size then call WideCharToMultiByte again with cchWideChar = -1 and
buffer size = required_size to write the NUL-terminated bytes, then trim the
trailing NUL before returning; ensure lengths and error checks use appropriate
signed/unsigned types and return empty string on failure.
| case WM_DESTROY: | ||
| window_handle_ = nullptr; | ||
| Destroy(); | ||
| if (quit_on_close_) { | ||
| PostQuitMessage(0); | ||
| } | ||
| return 0; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Avoid re-entrant Destroy() and double-calling OnDestroy()
Calling Destroy() inside WM_DESTROY can lead to OnDestroy() running twice. Handle cleanup directly in this branch and let Destroy() only trigger the window destruction path.
- case WM_DESTROY:
- window_handle_ = nullptr;
- Destroy();
- if (quit_on_close_) {
- PostQuitMessage(0);
- }
- return 0;
+ case WM_DESTROY:
+ // Perform per-instance cleanup once.
+ OnDestroy();
+ window_handle_ = nullptr;
+ if (quit_on_close_) {
+ PostQuitMessage(0);
+ }
+ return 0;🏁 Script executed:
#!/bin/bash
# Verify no subclass relies on double OnDestroy invocations.
rg -n -C2 -g 'windows/**' $'\bclass\s+\w+\s*:\s*public\s+Win32Window\b|\bOnDestroy\s*\('Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find all classes inheriting from Win32Window
rg -nP 'class\s+\w+\s*:\s*public\s+Win32Window' -g 'windows/**'
# Locate any OnDestroy method definitions
rg -nP 'void\s+OnDestroy\s*\(' -g 'windows/**'
# Search for any calls to OnDestroy
rg -n 'OnDestroy\s*\(' -g 'windows/**'Length of output: 582
🏁 Script executed:
#!/bin/bash
# Show WM_DESTROY handling in win32_window.cpp
rg -nC5 'case WM_DESTROY' -g 'windows/runner/win32_window.cpp'
# Show Destroy() implementation in win32_window.cpp
rg -nC5 'void Win32Window::Destroy' -g 'windows/runner/win32_window.cpp'Length of output: 1350
Centralize cleanup in WM_DESTROY and remove OnDestroy from Destroy()
To prevent double OnDestroy() calls when manually destroying the window and on WM_DESTROY, move the cleanup into the WM_DESTROY handler and make Destroy() only tear down the HWND.
--- windows/runner/win32_window.cpp
void Win32Window::Destroy() {
- OnDestroy();
if (window_handle_) {
DestroyWindow(window_handle_);
window_handle_ = nullptr;
}
}--- windows/runner/win32_window.cpp
case WM_DESTROY:
- window_handle_ = nullptr;
- Destroy();
+ // Perform cleanup exactly once on WM_DESTROY
+ OnDestroy();
+ window_handle_ = nullptr;
if (quit_on_close_) {
PostQuitMessage(0);
}
return 0;This guarantees OnDestroy() is invoked only once per destruction path and preserves FlutterWindow’s override without unexpected re-entry.
📝 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.
| case WM_DESTROY: | |
| window_handle_ = nullptr; | |
| Destroy(); | |
| if (quit_on_close_) { | |
| PostQuitMessage(0); | |
| } | |
| return 0; | |
| // windows/runner/win32_window.cpp | |
| // … elsewhere in WndProc … | |
| case WM_DESTROY: | |
| // Perform cleanup exactly once on WM_DESTROY | |
| OnDestroy(); | |
| window_handle_ = nullptr; | |
| if (quit_on_close_) { | |
| PostQuitMessage(0); | |
| } | |
| return 0; | |
| // … rest of WndProc … | |
| // ------------------------------------------------------------------ | |
| void Win32Window::Destroy() { | |
| // Only tear down the HWND here; cleanup happens in WM_DESTROY | |
| if (window_handle_) { | |
| DestroyWindow(window_handle_); | |
| window_handle_ = nullptr; | |
| } | |
| } |
| void Win32Window::Destroy() { | ||
| OnDestroy(); | ||
|
|
||
| if (window_handle_) { | ||
| DestroyWindow(window_handle_); | ||
| window_handle_ = nullptr; | ||
| } | ||
| if (g_active_window_count == 0) { | ||
| WindowClassRegistrar::GetInstance()->UnregisterWindowClass(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make Destroy() idempotent and single-responsibility
Destroy() should only destroy the HWND; OnDestroy() should be called from WM_DESTROY to avoid re-entrancy.
-void Win32Window::Destroy() {
- OnDestroy();
-
- if (window_handle_) {
- DestroyWindow(window_handle_);
- window_handle_ = nullptr;
- }
- if (g_active_window_count == 0) {
- WindowClassRegistrar::GetInstance()->UnregisterWindowClass();
- }
-}
+void Win32Window::Destroy() {
+ if (window_handle_) {
+ DestroyWindow(window_handle_);
+ window_handle_ = nullptr;
+ }
+ if (g_active_window_count == 0) {
+ WindowClassRegistrar::GetInstance()->UnregisterWindowClass();
+ }
+}📝 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.
| void Win32Window::Destroy() { | |
| OnDestroy(); | |
| if (window_handle_) { | |
| DestroyWindow(window_handle_); | |
| window_handle_ = nullptr; | |
| } | |
| if (g_active_window_count == 0) { | |
| WindowClassRegistrar::GetInstance()->UnregisterWindowClass(); | |
| } | |
| } | |
| void Win32Window::Destroy() { | |
| if (window_handle_) { | |
| DestroyWindow(window_handle_); | |
| window_handle_ = nullptr; | |
| } | |
| if (g_active_window_count == 0) { | |
| WindowClassRegistrar::GetInstance()->UnregisterWindowClass(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In windows/runner/win32_window.cpp around lines 224 to 234, remove the call to
OnDestroy() from Destroy() and make Destroy() idempotent by returning
immediately if window_handle_ is nullptr; leave only the HWND teardown (call
DestroyWindow(window_handle_) and set window_handle_ = nullptr) and keep the
existing WindowClassRegistrar::GetInstance()->UnregisterWindowClass() check
as-is, and ensure OnDestroy() is invoked from the WM_DESTROY case in the window
procedure instead to avoid re-entrancy.
|
Hey @raunaktastic as mentioned before please do not delete things from the PR Template like this, the template contains important checklists and information about the PR. Also, please raise your PRs to the dev branch. Please share a video of the functionality implemented. |
|
Hey @raunaktastic , lots of issues with this PR, please raise to dev branch, please fill out the PR template properly (copy from templates folder and fill out again), please remove changes unrelated to the issue being solved, and automated tests are failing, please fix that. |
|
Closing as stale |
|
✅ PR Closed - Thank You, @raunaktastic!
We appreciate your effort and look forward to more contributions from you! 🤝 |
2 participants
“Hi @M4dhav , this PR includes the login screen and layout fixes. Please review and let me know if any changes are needed.”
Summary by CodeRabbit
New Features
Chores
Tests