-
Notifications
You must be signed in to change notification settings - Fork 234
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
Abbreviations #1208
base: main
Are you sure you want to change the base?
Abbreviations #1208
Conversation
…e able to delete entry FIX: Editing abbreviation entries does not create new entries but actually edits them
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.
Thx! This is a ton of work, I really appreciate it, it'll be a great feature to have.
Suggestions below, the main concern being removing individual SQL reads for every single space, and other SQL cleanup.
val allAbbreviations: LiveData<List<Abbreviation>> = abbreviationDao.getAllAbbreviations() | ||
|
||
@WorkerThread | ||
suspend fun insertOrUpdate( |
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.
Did a search, and room now has an @Upsert
: https://developer.android.com/reference/kotlin/androidx/room/Upsert?hl=en
@@ -344,6 +344,34 @@ data class BehaviorUpdate( | |||
val ghostKeysEnabled: Int, | |||
) | |||
|
|||
@Dao |
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.
Might as well put all the abbreviation stuff in one file. So rename the AbbreviationRepository.kt
above to Abbreviation
, and move all this stuff in there.
@Query("SELECT * FROM Abbreviation") | ||
fun getAllAbbreviations(): LiveData<List<Abbreviation>> | ||
|
||
@Query("SELECT * FROM Abbreviation WHERE lower(abbreviation) = lower(:abbr) LIMIT 1") |
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.
hrm... I'm not sure what the best way to handle case would be. I spose this is fine, but we should decide first whether we should allow upper case abbreviations at all, in which case it'd be better to add a table constraint.
@Query("SELECT * FROM Abbreviation WHERE lower(abbreviation) = lower(:abbr) LIMIT 1") | ||
fun getAbbreviation(abbr: String): Abbreviation? | ||
|
||
@Query("SELECT * FROM Abbreviation WHERE lower(abbreviation) = lower(:abbr) LIMIT 1") |
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.
This query is duped, so you might as well make the string a const.
fun getAbbreviation(abbr: String): Abbreviation? | ||
|
||
@Query("SELECT * FROM Abbreviation WHERE lower(abbreviation) = lower(:abbr) LIMIT 1") | ||
suspend fun getAbbreviationAsync(abbr: String): Abbreviation? |
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'd prefer if async was the default, and change getAbbreviation
to getAbbreviationSync
. Seems like you'd never need either of these tho, I'll have to read further down.
EDIT: Actually both of these should be removed. All the abbrevations should only be read once as a list, and passed around as necessary.
|
||
private const val ABBR_TAG = "AbbreviationManager" | ||
|
||
class AbbreviationManager( |
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 don't understand the point of this at all. You should just be passing the AbbreviationViewModel
to any screen that needs it, and reading the abbreviations at the top of that file.
@@ -1107,13 +1139,18 @@ private fun autoCapitalize( | |||
} | |||
} | |||
|
|||
fun autoCapitalizeCheck(ime: IMEService): Boolean = ime.currentInputConnection.getCursorCapsMode(ime.currentInputEditorInfo.inputType) > 0 | |||
fun autoCapitalizeCheck(ime: IMEService): Boolean { |
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.
Why did this need changes.
|
||
/** | ||
* Avoid capitalizing or switching to shifted mode in certain edit boxes | ||
*/ | ||
fun isUriOrEmailOrPasswordField(ime: IMEService): Boolean { | ||
val inputType = ime.currentInputEditorInfo.inputType and (InputType.TYPE_MASK_VARIATION) | ||
val editorInfo = ime.currentInputEditorInfo ?: return false |
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.
Same, why changes here.
1, | ||
) | ||
|
||
if (text == " ") { |
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.
This should be extracted to its own function
if (text == " ") { | ||
val currentText = ime.currentInputConnection.getTextBeforeCursor(1000, 0)?.toString() | ||
if (currentText != null) { | ||
val abbreviationManager = AbbreviationManager(ime.applicationContext) |
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.
Not necessary. Pass in the abbreviations as List<Abbreviation>
to performKeyAction.
i think this is great, however, it might not be desireable to always expand the abbreviation. what about only expanding it when swiping right on spacebar? (right arrow, or left arrow for rtl) it works similarly in browser autocomplete (brave for android) additionally, it might be a good idea to display in the ui somehow 1) that there is an expansion available if you type the trigger word 2) what the expansion is. you could use the android popup messages, but i find them quite unreliable on my device (don't always show up) |
This system allows you to add abbreviation for texts you use often.
For example if you type "gm" + space the texts gets replaced by "good morning".
Abbreviations can be edited from a new settings screen.
20241225_abbreviations.mp4