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

text to be committed is the middle suggestion #672

Closed
wants to merge 2 commits into from

Conversation

codokie
Copy link
Contributor

@codokie codokie commented Apr 10, 2024

Fixes #658
After testing it for a while there does not seem to be a need for a separate setting to toggle this functionality.
The change has effect only if autocorrect is enabled and it seems very intuitive that the middle suggestion should be the text that will be committed should the space key be pressed.

Nevermind, I may be able to see why some individuals may not be crazy about it. So I've added a setting (off by default) called "Center text to commit".
It is under Suggestions settings and can be activated even if autocorrect is disabled, although it makes more sense to do so when it autocorrect is on.

@codokie codokie changed the title (autocorrect) text to be committed is the middle suggestion text to be committed is the middle suggestion Apr 11, 2024
@Helium314
Copy link
Owner

Technically it's fine, only some minor naming things.

// make sure typed word is shown, so user is able to override incoming autocorrection
// If there is an incoming autocorrection, make sure typed word is shown, so user is able to override it.
// Otherwise, if the relevant setting is enabled, show the typed word in the middle.
val typedIndex = if (hasAutoCorrection) 2 else 1
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typedIndex is a confusing name, please be more explicit (e.g. indexOfTypedWord)

Comment on lines +127 to +130
<!-- Option to show the text to be committed as the middle middle suggestion -->
<string name="center_suggestion_text_to_commit">Center text to commit</string>
<!-- Description for the center text to commit setting -->
<string name="center_suggestion_text_to_commit_summary">If enabled, the middle suggestion will show the text to be committed</string>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to avoid to avoid "commit" in the name and description (no one used it in the related issue).
Also it's somewhat confusing, as the context of where the text is centered is not clear.

I'm not good at finding short and clear titles and descriptions, not sure how much I can help here...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked in the issue, maybe someone has a good idea. If not we can still change it later (though it would mean unnecessary work for translators)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "middle suggestion always used"? Then for the description something like "If enabled, the middle suggestion will display the word selected for completion, whether the typed text was correct or not"? Add a bit more detail for what its doing. Obviously just some ideas

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Always use middle suggestion" sounds good to me, at least I think this is hardest do mis-interpret.
And as summary something like "On pressing space or punctuation, the middle suggestion will be used". Or maybe "typed" or "entered"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used or entered seem better, given no typing is taking place. Entered leaves less ambiguity, but used might be more clear. Your summary is much clearer than mine in fewer words lol

@Volterxien
Copy link
Contributor

I'm not able to finish this PR right? I was gonna go and add the changes we discussed, but it seems only the original contributor is able to do that?

@micahmo
Copy link

micahmo commented Apr 23, 2024

You could always clone the fork, use their branch, and make a new PR. 😉 They'd still get credit for their commits.

@Volterxien
Copy link
Contributor

Is that rude lol? I don't really know what the best practices are for contributing

@micahmo
Copy link

micahmo commented Apr 23, 2024

I don't think so, if the original PR stalls, anyone is welcome to pick it up. Again, the OP will still get credit for their commits as a contributor to this repo. 😊

@Volterxien
Copy link
Contributor

By clone, did you mean fork? I don't have write permission into the original fork, so I would think I would need to create my own fork and push to that? Sorry for all the questions, appreciate your time.

@micahmo
Copy link

micahmo commented Apr 23, 2024

Yeah sorry, not exactly the right use of terminology. I would fork this repo, but then add codokie's repo as a remote. Then you can check out the branch from the remote and either commit directly to it or make a new branch from it. Then push it to your fork and PR here. Let me know if that helps or not. I can write up some Git commands if that would help more. 😊

@codokie codokie closed this Apr 23, 2024
@codokie codokie deleted the middle-suggestions branch April 26, 2024 14:48
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.

Always show word that will be typed as middle word in suggestions row
4 participants