AnySoftKeyboard icon indicating copy to clipboard operation
AnySoftKeyboard copied to clipboard

Set to capitalize when deleting whole line with shift + backspace

Open lubenard opened this issue 5 years ago • 8 comments

When using the Shift + backspace shortcut to delete the whole line, the shift state was not resetting to enabled, meaning you had to reclick on the shift key.

This PR take care of this.

I do not know if my code is the most optimised way to do it. I am maybe missing something, and i do not like calls to ic.getTextBeforeCursor

Maybe there is a another way to do it ?

lubenard avatar Jan 10 '21 15:01 lubenard

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 65.11%. Comparing base (9600a1f) to head (0a43e1a). :warning: Report is 2209 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2626      +/-   ##
============================================
+ Coverage     65.10%   65.11%   +0.01%     
- Complexity     3004     3005       +1     
============================================
  Files           237      237              
  Lines         15863    15870       +7     
  Branches       2087     2088       +1     
============================================
+ Hits          10327    10334       +7     
  Misses         4776     4776              
  Partials        760      760              
Files with missing lines Coverage Δ
...main/java/com/anysoftkeyboard/AnySoftKeyboard.java 60.70% <100.00%> (+0.36%) :arrow_up:
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Jan 10 '21 15:01 codecov[bot]

I can suggest a different approach that does not require getTextBeforeCursor and does not require to keep a count. I think it will work, not sure: When SHIFT+DELETE is pressed (specifically, at com/anysoftkeyboard/AnySoftKeyboard.java:394), you should flip a flag, call it mShouldUpdateCapsAfterShift. Once the shift key is released (maybe at onRelease(primaryCode)?), check for that flag and call updateShiftStateNow().

Now, I see that there is some logic around calling updateShiftStateNow() in onRelease, but I'm not sure what that suppose to do, and if we can re-use (or extend) that logic to also support your case

menny avatar Jan 18 '21 22:01 menny

You might also need to add support for ic.getCursorCapsMode in TestInputConnection, because right now it always returns 0. See com.anysoftkeyboard.TestInputConnection#getCursorCapsMode

menny avatar Jan 18 '21 22:01 menny

Hello ! After further investigations, it seems that modifying getCursorCapsMode will break some tests (About 100).

This is due to the fact that using a "Normal" keyboard, saying mAnySoftKeyboardUnderTest.simulateTextTyping("hello");

would result into Hello and not hello.

Currently, and after many documentation (here), it seems ic.getCursorCapsMode is calling TextUtils.getCapsMode.

My modified version of getCursorCapsMode is currently looking like this:

@Override
    public int getCursorCapsMode(int reqModes) {
        // Number 147457 come from real test using real device.
        // Seems mocked number set reqModes to '1', leading it to fails every time.
        return TextUtils.getCapsMode(mInputText.toString(), mCursorPosition, 147457);
    }

But, it is working in real conditions ! No more ic.getTextBeforeCursor ! What do you think ? Should we modify all of those tests ?

It seems to be the cleanest version

lubenard avatar Jan 23 '21 00:01 lubenard

so, how does your change looks like now? Can you push it here?

menny avatar Feb 12 '21 21:02 menny

Hey ! I was waiting for your answer for this

What do you think ?
Should we modify all of those tests ?

, but i can try to push something tomorrow !

lubenard avatar Feb 12 '21 22:02 lubenard

Yes, please change the implementation of TestInputConnection to use TextUtils.getCapsMode. And fix all the affected tests.

I would also love it if you add a test that ensures that the settings R.string.settings_key_auto_capitalization affects this as well.

menny avatar Feb 16 '21 02:02 menny

Hey ! I changed all tests but some things triggers me. For example, suggestions are lowercase even if the word is having a uppercase letter

For example, suggestions for "He" are : "He", "he'll", "hell", "hello"

and i can't figure out why the other suggestions have a H in lowercase

Second thing:

When the test use simulateGestureProcess, the next characters are Uppercase.

Example,

simulateGestureProcess("hello");
Assert.assertequals("Hello", mAnySoftKeyBoardUnderTest.getCurrentInputConnectionTest()); <----- True

mAnySoftKeyBoardUnderTest.simulateTextTyping("a");
Assert.assertequals("Hello a", mAnySoftKeyBoardUnderTest.getCurrentInputConnectionTest()); <----- False, "Hello A" is actual text.

This seems to act like this only for simulateGestureProcess.

I thought it might be related to policy for getCapsMode ( TextUtils.CAP_MODE_CHARACTERS, TextUtils.CAP_MODE_WORDS, or TextUtils.CAP_MODE_SENTENCES)

But that does not seems related

lubenard avatar Feb 16 '21 09:02 lubenard