-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
feat(view): Enable mouse cursor movement in shell readline #4775
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -53,6 +53,8 @@ public final class TerminalView extends View { | |||
| /** Our terminal emulator whose session is {@link #mTermSession}. */ | ||||
| public TerminalEmulator mEmulator; | ||||
|
|
||||
| private boolean mIsMouseCursorMovementEnabled = false; | ||||
|
|
||||
| public TerminalRenderer mRenderer; | ||||
|
|
||||
| public TerminalViewClient mClient; | ||||
|
|
@@ -295,6 +297,9 @@ public boolean attachSession(TerminalSession session) { | |||
| mEmulator = null; | ||||
| mCombiningAccent = 0; | ||||
|
|
||||
| android.content.SharedPreferences prefs = androidx.preference.PreferenceManager.getDefaultSharedPreferences(getContext()); | ||||
| mIsMouseCursorMovementEnabled = prefs.getBoolean("mouse_cursor_movement_enabled", false); | ||||
|
|
||||
|
Comment on lines
+300
to
+302
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Termux does not use androidx preferences. Probably it would be better to use existing |
||||
| updateSize(); | ||||
|
|
||||
| // Wait with enabling the scrollbar until we have a terminal to get scroll position from. | ||||
|
|
@@ -611,11 +616,11 @@ public boolean onTouchEvent(MotionEvent event) { | |||
| updateFloatingToolbarVisibility(event); | ||||
| mGestureRecognizer.onTouchEvent(event); | ||||
| return true; | ||||
| } else if (event.isFromSource(InputDevice.SOURCE_MOUSE)) { | ||||
| if (event.isButtonPressed(MotionEvent.BUTTON_SECONDARY)) { | ||||
| } else if (event.isFromSource(InputDevice.SOURCE_MOUSE) || event.isFromSource(InputDevice.SOURCE_TOUCHSCREEN)) { | ||||
| if (event.isFromSource(InputDevice.SOURCE_MOUSE) && event.isButtonPressed(MotionEvent.BUTTON_SECONDARY)) { | ||||
| if (action == MotionEvent.ACTION_DOWN) showContextMenu(); | ||||
| return true; | ||||
| } else if (event.isButtonPressed(MotionEvent.BUTTON_TERTIARY)) { | ||||
| } else if (event.isFromSource(InputDevice.SOURCE_MOUSE) && event.isButtonPressed(MotionEvent.BUTTON_TERTIARY)) { | ||||
| ClipboardManager clipboardManager = (ClipboardManager) getContext().getSystemService(Context.CLIPBOARD_SERVICE); | ||||
| ClipData clipData = clipboardManager.getPrimaryClip(); | ||||
| if (clipData != null) { | ||||
|
|
@@ -636,6 +641,47 @@ public boolean onTouchEvent(MotionEvent event) { | |||
| break; | ||||
| } | ||||
| } | ||||
| else { // Mouse tracking is OFF, handle for the shell | ||||
| if (mIsMouseCursorMovementEnabled && event.getAction() == MotionEvent.ACTION_DOWN) { | ||||
| // Get target row and column from the click | ||||
| int[] targetCoords = getColumnAndRow(event, false); | ||||
| int targetColumn = targetCoords[0]; | ||||
| int targetRow = targetCoords[1]; | ||||
| // Boundary Check | ||||
| try { | ||||
| String clickedChar = mEmulator.getScreen().getSelectedText(targetColumn, targetRow, targetColumn + 1, targetRow).toString(); | ||||
| if (clickedChar == null || clickedChar.trim().isEmpty()) { | ||||
| return true; | ||||
| } | ||||
| } catch (Exception e) { | ||||
| return true; | ||||
| } | ||||
|
|
||||
| // Get cursor's current visual position | ||||
| int currentRow = mEmulator.getCursorRow(); | ||||
| int currentColumn = mEmulator.getCursorCol(); | ||||
|
|
||||
| // Get the width of the terminal in characters | ||||
| int terminalWidth = mEmulator.mColumns; | ||||
|
|
||||
| // Calculate the 1D index for both the click position and the cursor position. | ||||
| int targetIndex = (targetRow * terminalWidth) + targetColumn; | ||||
| int currentIndex = (currentRow * terminalWidth) + currentColumn; | ||||
|
|
||||
| int diff = targetIndex - currentIndex; | ||||
|
|
||||
| // Send Left or Right commands | ||||
| if (diff > 0) { // Need to move right | ||||
| for (int i = 0; i < diff; i++) { | ||||
| handleKeyCode(android.view.KeyEvent.KEYCODE_DPAD_RIGHT, 0); | ||||
| } | ||||
| } else if (diff < 0) { // Need to move left | ||||
| for (int i = 0; i < -diff; i++) { | ||||
| handleKeyCode(android.view.KeyEvent.KEYCODE_DPAD_LEFT, 0); | ||||
| } | ||||
| } | ||||
|
Comment on lines
+674
to
+682
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like a bad idea to me. It will not work fine in the case if text editing widget is not as wide as screen (i.e. widget has borders or other decorations).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have tested this PR on my device Samsung Galaxy S7 SM-G930U. This PR is working, however, as should be expected, (and as also applies to the previous, similar PR): this mode does work in regular shells like but it does not work very well (but does a little bit) in curses or curses-like programs like that is why I asked for it to be disabled by default, and only enabled by a setting (which I can confirm, in the current version of this PR, the setting is not enabled by default) |
||||
| } | ||||
| } | ||||
| } | ||||
|
|
||||
| mGestureRecognizer.onTouchEvent(event); | ||||
|
|
@@ -1495,4 +1541,9 @@ public void updateFloatingToolbarVisibility(MotionEvent event) { | |||
| } | ||||
| } | ||||
|
|
||||
| public void reloadPreferences() { | ||||
| android.content.SharedPreferences prefs = androidx.preference.PreferenceManager.getDefaultSharedPreferences(getContext()); | ||||
| mIsMouseCursorMovementEnabled = prefs.getBoolean("mouse_cursor_movement_enabled", false); | ||||
| } | ||||
|
|
||||
|
Comment on lines
+1544
to
+1548
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I said before, it will be better to use termux.properties for this.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, however, it might be relevant to mention that currently this preexisting setting:
and this preexisting setting:
both do very similar (but subtly different) things, and there appears to currently be no way to configure the first setting purely from therefore, if either setting is modified from the default, software keyboard will not work, and both must be set to the default before software keyboard works again. this might be a different topic from what this PR is about, but this summary explains why I didn't mention
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The setting in PR should go in properties, but in my local changes, both props and prefs have been rewritten, so would be moot to work on it currently. The first setting for soft keyboard enabled is in settings because if it was in properties file and soft keyboard gets disabled, you wouldn't be able to enable it again without an external keyboard to type into the terminal to open the properties file, hence its in a UI. The later hide property only hides soft keyboard at startup and tapping screen brings it up again. Eventually all settings will be moved to properties or yaml file and shown in a UI, but that's in distant future.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In my testing, both settings caused the software keyboard to remain closed and simply tapping the screen did not make it appear on my device if either setting was changed from its default. The subtle difference between them I mentioned is something else that is very subtle and harder to explain. However, the problem of the "
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My rewritten solution doesn't make any changes for keyboard. Hide on startup only hides keyboard and should show on tap, while disable setting, completely disables it until toggled with keyboard key or setting. You likely need to restart termux app after making changes for them to properly take effect. Soft keyboard enabled should be enabled, and hide set to What does it take for keyboard to appear again with hide?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are correct, when I test again I am unable to reproduce the problem I saw before on any of my devices, so I must have been wrong. I don't know what happened because I thought that I set everything to default and restarted Termux using the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Welcome and thanks for the confirmation for saving me work, there was a chance there could have been a bug on some devices. Android does not provide any API to check if keyboard is open or not, so termux code for it is very hacky, which could have caused problems, but it was well tested before I merged it and I haven't received any other reports. I tend not to touch that code. |
||||
| } | ||||


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.
Thanks for this catch though.