-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Add Cyrillic keyboard layout support for flutter_tools terminal commands #177855
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
Add Cyrillic keyboard layout support for flutter_tools terminal commands #177855
Conversation
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.
Code Review
This pull request introduces support for Cyrillic keyboard layouts in the flutter_tools terminal, which is a valuable enhancement for developers using these layouts. The implementation correctly maps Cyrillic characters to their Latin equivalents, and the new functionality is well-tested. My feedback includes suggestions to refactor the growing switch statement into a more maintainable Map and to adopt a data-driven approach for the new tests to reduce code duplication.
0b80293 to
5116ca1
Compare
|
Thanks for the contribution @777genius! Do you happen to know if there's already an issue filed for this? If so, could you link it here? Otherwise, we should file an issue describing the problem so that we can continue to track work in this area. This approach does work, but I'm sure if you're encountering this issue for Cyrillic keyboard layouts, other users are encountering if with other layouts as well. This means that we should probably come up with a more generalizable solution that allows for us to add mappings for other languages as well without having to introduce case statements for each individual mapping. I think this can be done by creating a |
|
Hi @777genius, is this something you're still interested in pursuing? |
|
Thank you for the feedback, @bkonyi! You're absolutely right about making this more generalizable. I found several related issues that confirm this is a broader problem affecting multiple keyboard layouts: #27021 - Flutter run not handling keypresses with non-English layouts (Cyrillic, closed in 2020 but the terminal command issue persists) I agree that using a Map<String, String> approach is much cleaner and more maintainable. I'll refactor the implementation to use your suggested approach:
/// Maps non-Latin keyboard layout characters to their Latin equivalents
/// based on physical key positions.
///
/// This allows users with non-Latin keyboard layouts to use terminal commands
/// without switching layouts. Contributors can add mappings for additional
/// layouts (Arabic, Hebrew, Greek, etc.) by adding entries to this map.
const Map<String, String> _keyboardLayoutMappings = {
// Cyrillic (Russian ЙЦУКЕН) layout -> QWERTY Latin
'к': 'r', 'К': 'R', // hot reload / hot restart
'й': 'q', 'Й': 'Q', // quit
'ц': 'w', 'Ц': 'W', // dump widget tree
'р': 'h', 'Р': 'H', // help
'ы': 's', 'Ы': 'S', // screenshot / semantics
'в': 'd', 'В': 'D', // detach
'а': 'f', 'А': 'F', // dump focus tree
'п': 'g', 'П': 'G', // run source generators
'ш': 'i', 'Ш': 'I', // widget inspector / invert images
'д': 'l', 'Д': 'L', // dump layer tree
'щ': 'o', 'Щ': 'O', // toggle platform
'з': 'p', 'З': 'P', // debug paint / performance overlay
'т': 'n', 'Т': 'N', // (reserved for future use)
'е': 't', 'Е': 'T', // dump render tree
'г': 'u', 'Г': 'U', // dump semantics (inverse hit test)
'м': 'v', 'М': 'V', // open DevTools
// TODO: Add mappings for other non-Latin layouts as needed
};
/// Maps a keyboard character to its Latin equivalent if a mapping exists.
/// Returns the original character if no mapping is found.
String _mapKeyToLatin(String key) {
return _keyboardLayoutMappings[key] ?? key;
}
Future<bool> _commonTerminalInputHandler(String character) async {
// Map non-Latin characters to Latin equivalents based on physical key position
character = _mapKeyToLatin(character);
_logger.printStatus('');
switch (character) {
case 'a':
return residentRunner.debugToggleProfileWidgetBuilds();
case 'r':
// ... existing hot reload logic
// ... rest of the switch statement remains unchanged
}
}This approach has several benefits:
How do you like the solution? |
|
That sounds good to me! |
5116ca1 to
af39447
Compare
…mmands Implements a keyboard layout mapping layer that allows users with non-Latin layouts to use flutter_tools terminal commands without switching layouts. The implementation uses a Map-based approach for character translation, mapping characters based on physical key positions rather than linguistic equivalents. This design makes it easy for contributors to add support for additional layouts (Arabic, Hebrew, Greek, etc.). Includes comprehensive test coverage following Flutter's testing patterns, with tests for basic functionality, error handling (fatal/non-fatal), and unsupported operations. Fixes: flutter#27021, flutter#100456, flutter#116658
af39447 to
5eec195
Compare
|
@bkonyi Please take a look with your professional eye. Updated the implementation to use the Map-based approach as suggested. Changes include:
The implementation is now much more maintainable and makes it easy to add support for other keyboard layouts (Arabic, Hebrew, Greek, etc.) in the future. Ready for review! |
|
Sorry for the delay @777genius. I'll review this today! |
bkonyi
left a 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.
Just a few comments, but looks good overall!
| static const _keyboardLayoutMappings = <String, String>{ | ||
| // Cyrillic (Russian ЙЦУКЕН) layout → QWERTY Latin | ||
| // Maps based on physical key positions for terminal commands | ||
| 'к': 'r', 'К': 'R', // hot reload / hot restart |
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.
Can we add a comment here warning that these keys are not QWERTY Latin, even if they look identical? That way future contributors know that р is not p 😅
| switch (character) { | ||
| case 'a': | ||
| case 'ф': // 'a' in Cyrillic (QWERTY 'a' → ЙЦУКЕН 'ф') |
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.
Are these cases needed anymore since _mapKeyToLatin(...) does the conversion?
| .file(globals.artifacts!.getHostArtifact(HostArtifact.flutterWebLibrariesJson)) | ||
| .uri | ||
| .toString(), | ||
| librariesSpec: |
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.
I think you may have accidentally formatted these files with an older version of the Dart formatter. Can you upgrade your Flutter installation to the latest stable and format this again?
- Add warning comment about visually similar Cyrillic/Latin characters - Remove redundant Cyrillic cases from switch (already handled by _mapKeyToLatin) - Update docstring to reference _keyboardLayoutMappings - Reformat with latest dart formatter
|
@bkonyi Thanks for the top-level review! I've addressed all your comments:
Ready for another look! |
|
Thanks for addressing the comments @777genius! This LGTM! @chingjun would you mind giving a second review? |
|
Also, can we add a test, or an assert, that we never add regular ASCII characters as keys in the |
- Format terminal_handler_test.dart - Expose keyboardLayoutMappings with @VisibleForTesting - Add test to ensure mapping keys have code points > 127 (prevents accidental addition of Latin characters as keys)
bb6195c to
13df6fb
Compare
|
@chingjun Thank you for the review and helpful suggestions! CI is green. |
Manual roll Flutter from d81baab to 57c3f8b (38 revisions) Manual roll requested by stuartmorgan@google.com flutter/flutter@d81baab...57c3f8b 2025-12-17 bruno.leroux@gmail.com Add FloatingActionButtonTheme (flutter/flutter#179736) 2025-12-17 engine-flutter-autoroll@skia.org Roll Skia from b1569739f431 to fb576bd6827a (1 revision) (flutter/flutter#179989) 2025-12-17 bruno.leroux@gmail.com Update more comments related to theme normalization (flutter/flutter#179682) 2025-12-17 engine-flutter-autoroll@skia.org Roll Skia from cce9b86bda7d to b1569739f431 (1 revision) (flutter/flutter#179979) 2025-12-17 jobguldemeester@gmail.com Adds property passthrough for CheckboxListTile, SwitchListTile and RadioListTile (flutter/flutter#178098) 2025-12-17 engine-flutter-autoroll@skia.org Roll Dart SDK from ca949b915846 to 3793f3d2d0c4 (2 revisions) (flutter/flutter#179973) 2025-12-17 engine-flutter-autoroll@skia.org Roll Skia from 318400199beb to cce9b86bda7d (1 revision) (flutter/flutter#179976) 2025-12-17 ahmedsameha1@gmail.com Make sure that a CupertinoTextFormFieldRow doesn't crash in 0x0 envir… (flutter/flutter#179932) 2025-12-17 ahmedsameha1@gmail.com Make sure that a CupertinoTabView doesn't crash in 0x0 environment (flutter/flutter#179845) 2025-12-17 ahmedsameha1@gmail.com Make sure that a CupertinoTextField doesn't crash in 0x0 environment (flutter/flutter#179865) 2025-12-17 ahmedsameha1@gmail.com Make sure that a CupertinoSwitch doesn't crash in 0x0 environment (flutter/flutter#179748) 2025-12-17 116356835+AbdeMohlbi@users.noreply.github.com Update `BuildContext` docs to make it easier to understand (flutter/flutter#178616) 2025-12-17 41930132+hellohuanlin@users.noreply.github.com [ios][pv] quick fix to enable and re-enable web view's gesture recognizer (flutter/flutter#179908) 2025-12-17 biggs0125@gmail.com Deduplicate wasm dry run entries in analytics. (flutter/flutter#179970) 2025-12-17 engine-flutter-autoroll@skia.org Roll Skia from 99899cbb415b to 318400199beb (1 revision) (flutter/flutter#179969) 2025-12-17 engine-flutter-autoroll@skia.org Roll Skia from 2ac4a8709bc9 to 99899cbb415b (1 revision) (flutter/flutter#179968) 2025-12-17 engine-flutter-autoroll@skia.org Roll Dart SDK from 95a92bc705d3 to ca949b915846 (6 revisions) (flutter/flutter#179967) 2025-12-17 biggs0125@gmail.com Add package info to wasm dry run events. (flutter/flutter#179826) 2025-12-17 matt.boetger@gmail.com Platform View Hide/Show Integration test (flutter/flutter#179902) 2025-12-16 engine-flutter-autoroll@skia.org Roll Skia from 61162d72343f to 2ac4a8709bc9 (14 revisions) (flutter/flutter#179961) 2025-12-16 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 433KtmJvbMyaDMJvD... to fAoyBAT99XxwPE5hL... (flutter/flutter#179960) 2025-12-16 bkonyi@google.com [ Tool ] Fix update-packages not accounting for path dependencies (flutter/flutter#179951) 2025-12-16 45459898+RamonFarizel@users.noreply.github.com ListTile fix MinIntrinsicHeight calculation (flutter/flutter#179515) 2025-12-16 108678139+manu-sncf@users.noreply.github.com Fix pinned header in NestedScrollView (flutter/flutter#179210) 2025-12-16 bruno.leroux@gmail.com Update some comments related to theme normalization (flutter/flutter#179624) 2025-12-16 iliyazelenkog@gmail.com Add Cyrillic keyboard layout support for flutter_tools terminal commands (flutter/flutter#177855) 2025-12-16 lauren@selfisekai.rocks Minor fixes for libstdc++ 15 (flutter/flutter#178601) 2025-12-16 34465683+rkishan516@users.noreply.github.com Feat: Add top gap for cupertino sheet (flutter/flutter#171348) 2025-12-16 116356835+AbdeMohlbi@users.noreply.github.com Align `Build.API_LEVELS` usage in `FlutterImageDecoder.java` with existing usage (flutter/flutter#179868) 2025-12-16 41930132+hellohuanlin@users.noreply.github.com Revert "[ios][pv] accept/reject gesture based on hitTest (with new wi… (flutter/flutter#179895) 2025-12-16 codefu@google.com fix: line endings for flutter/dart/flutter-dev (flutter/flutter#179912) 2025-12-16 22373191+Hari-07@users.noreply.github.com Platform view blur clipping - Rounded Rect (iOS) (flutter/flutter#177551) 2025-12-16 engine-flutter-autoroll@skia.org Roll Dart SDK from 20d114f951db to 95a92bc705d3 (1 revision) (flutter/flutter#179909) 2025-12-16 34871572+gmackall@users.noreply.github.com [Reland] Unmodified android sdk bundle (flutter/flutter#179920) 2025-12-16 engine-flutter-autoroll@skia.org Roll Skia from 6903a4e65c3f to 61162d72343f (2 revisions) (flutter/flutter#179915) 2025-12-16 engine-flutter-autoroll@skia.org Roll Packages from 2cd921c to 57725eb (1 revision) (flutter/flutter#179942) 2025-12-16 45490440+shindonghwi@users.noreply.github.com Filter out FrameEvents/updateAcquireFence log spam from adb logcat (flutter/flutter#179884) 2025-12-16 30870216+gaaclarke@users.noreply.github.com `flutter update-packages --force-upgrade --update-hashes` (flutter/flutter#179950) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC bmparr@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. ...
Description
This PR adds support for Cyrillic (Russian ЙЦУКЕН) keyboard layout in the flutter_tools terminal handler, allowing users to use hot reload, hot restart, and other terminal commands without switching keyboard layouts.
Motivation
Developers working with Cyrillic keyboard layouts currently must switch to a Latin layout every time they want to use terminal commands like hot reload (r/R) or help (h). This disrupts the development workflow and reduces productivity.
Changes
The implementation maps Cyrillic characters to their corresponding Latin equivalents based on physical key positions on QWERTY/ЙЦУКЕН layouts:
Testing
Impact
This change improves the developer experience for users working with Cyrillic keyboard layouts by eliminating the need for constant layout switching during development. The implementation follows Flutter code style guidelines and does not affect existing functionality.