Skip to content
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 phantom space after characters usually followed by space #564

Merged
merged 3 commits into from
Mar 21, 2022

Conversation

Helium314
Copy link
Contributor

@Helium314 Helium314 commented Mar 11, 2022

When tapping a suggestion, a phantom space is added, and this phantom space is kept if a "character usually followed by space" (e.g. punctuation) is entered, so that on starting the next word the space is inserted.

When writing a word without tapping a suggestion, no phantom space is added and thus the user needs to add the space after punctuation (and other characters) manually, which I and others (#252) find rather annoying and inconsistent (why is it added when tapping suggestion, but not when typing the word manually).

With this PR, a phantom space is added when entering a separator that is "usually followed by space", if the user "was composing a word". The latter restriction is necessary to avoid auto-inserting spaces when trying to enter decimal numbers.

A problem that remains is that when ending a sentence on a number, there will not be a phantom space. This is the same behavior as on Gboard, but still not good.
[Edit: I was wrong, Gboard does not insert a space when not using suggestions, same as current OpenBoard]

I tried inserting

        if (SpaceState.NONE == inputTransaction.getMSpaceState() && settingsValues.isUsuallyFollowedBySpace(mConnection.getCodePointBeforeCursor()) && Character.isLetter(codePoint)) {
            insertAutomaticSpaceIfOptionsAndTextAllow(settingsValues);
        }

in line 839 (when entering a non-separator, e.g. a letter) instead of the proposed change.
Then spaces are added even if sentence ends on a number, but the first word in the new sentence isn't capitalised. I think this would be much worse than the flaw of my proposed change...
[Edit: and then it becomes impossible to remove an unwanted inserted space without moving the cursor]

Fixes #252
Fixes #563

@MajeurAndroid
Copy link
Member

I just tested this, it looks like it does the trick. I found a case where two spaces where added after a suggestion commit but I cannot figure out the exact conditions to reproduce this.
For that reason, and mostly because I really don't think the needed changes are that simple, we need to treat this as an experimental feature. Thus, it needs to have its setting in "Settings/Advanced/Experimental", and would be disabled by default. Do you feel comfortable implementing this?

Also, I investigated a bit more and I also ended up in thinking handleSeparatorEvent() is the right place to do the check and set mSpaceState to PHANTOM. Can you add a bit of commentary in the if statement to give a bit of context ?

Again thanks for your great contributions :)

@Helium314
Copy link
Contributor Author

Thanks for testing!
I'll play around a bit more to see whether I can reproduce/fix the double space problem, and add explaining comments.

Adding the setting should be ok, I'll ask for help if I have some trouble with this.

@Helium314
Copy link
Contributor Author

Helium314 commented Mar 16, 2022

I noticed another issue with this change: when trying to enter URLs, there is a space added. There already is some check that avoids this (RichInputConnection.textBeforeCursorLooksLikeURL), but it doesn't recognize e.g. github.com as URL.
This already happens now if you select a suggestion, but is made much worse by this PR because there is no convenient way to avoid the unwanted space.
My idea so far is something like not adding a space if the current field has InputType TYPE_TEXT_VARIATION_URI or TYPE_TEXT_VARIATION_WEB_EMAIL_ADDRESS (for non-web email address type it seems to work already). But I'm not sure about this, as in browsers the URL field typically also serves as search bar... maybe we can convert phantom space to space only if the input field already contains a space character (meaning it can't be an URL)? Ideas are welcome!

I also saw you linked #563 to this issue, but I don't think this will fix it.
I'm not even sure it makes sense to (implictly) expect @ to be preceded by space...
Adding @ to the list of word separators (see SpacingAndPunctuation) might help, but will likely stop the keyboard from remembering entire mail addresses (splitting it up into 2 parts).
Adding @ to list of word connectors doesn't really fit the name, but it may help. And detecting @ in the word should allow to avoid inserting a space after the .
[Edit: word connector works nicely, but is also used in some more places via isWordCodePoint. Still I think it should be fine...
One more way: simply add an exclusion for @ either in insertAutomaticSpaceIfOptionsAndTextAllow or in handleNonSeparatorEvent]

Anyway, I'm beginning to see more and more why you "really don't think the needed changes are that simple".

@Helium314
Copy link
Contributor Author

So to summarize some auto-space issues that currently exist only when entering a word via a suggestion, and that will occur more frequently with this PR, but IMO should be fixed anyway:

  • space inserted after . when entering github.com in TYPE_TEXT_VARIATION_URI
    • solution: don't insert space after . if there was no space in the entire input field (only for TYPE_TEXT_VARIATION_URI), and maybe do the same for :, as this is also sometimes used in URIs
  • spaces are inserted in TYPE_TEXT_VARIATION_WEB_EMAIL_ADDRESS, but not TYPE_TEXT_VARIATION_EMAIL_ADDRESS. I found no information on the actual difference of these fields, except that the first one explicitly refers to being used in a web form, and think they should be treated the same
    • solution: add TYPE_TEXT_VARIATION_WEB_EMAIL_ADDRESS to SUPPRESSING_AUTO_SPACES_FIELD_VARIATION in inputTypeUtils, because email addresses do not contain spaces
  • space inserted after . in email addresses when entered in normal text fields
    • solution: don't insert space if the current word contains @. This will probably break in case the issue below is to be solved by adding @ to the list of word separators.
  • space inserted before @. This is bad for email addresses and some URLs when entered in normal text fields, but probably useful when mentioning users, e.g. on github. I'm not sure whether something should be done, or maybe it could be put into a setting... (the current PR doesn't change behavior here)

I tested the solutions and they appear to work. Should I add them to this PR, or open another one?

@Helium314
Copy link
Contributor Author

I added the experimental setting. Is the description ok?
Technically it applies to more than just punctuation, but there is only limited space available.

One small change I did is that I moved the inserting of space to the else-branch of the check above, because at this place the check for mSpaceState == SpaceState.NONE is not necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blank spaces before special characters Feature request: Comma should add space
2 participants