work on some more todos

app/src/main/java/helium314/keyboard/keyboard/internal/keyboard_parser/KeyboardParser.kt

@@ -267,7 +267,7 @@ class KeyboardParser(private val params: KeyboardParams, private val context: Co
             val numberRowCopy = numberRow.toMutableList()
             numberRowCopy.forEachIndexed { index, keyData -> keyData.popup.symbol = baseKeys[0].getOrNull(index)?.label }
             baseKeys[0] = numberRowCopy
-        } else if (!params.mId.mNumberRowEnabled && params.mId.isAlphabetKeyboard && hasNumbersOnTopRow()) {
+        } else if (!params.mId.mNumberRowEnabled && params.mId.isAlphabetKeyboard && !hasBuiltInNumbers()) {
             if (baseKeys[0].any { it.popup.main != null || !it.popup.relevant.isNullOrEmpty() } // first row of baseKeys has any layout popup key
                 && params.mPopupKeyLabelSources.let {
                     val layout = it.indexOf(POPUP_KEYS_LAYOUT)
@@ -321,9 +321,11 @@ class KeyboardParser(private val params: KeyboardParams, private val context: Co
         return row
     }
 
-    // some layouts have different number layout, and there we don't want the numbers on the top row
+    // some layouts have numbers hardcoded in the main layout (pcqwerty as keys, and others as popups)
-    // todo: this should be derived from main layout and popup / hint order settings
+    private fun hasBuiltInNumbers() = params.mId.mSubtype.mainLayoutName == "pcqwerty"
-    private fun hasNumbersOnTopRow() = params.mId.mSubtype.mainLayoutName !in listOf("pcqwerty", "lao", "thai", "korean_sebeolsik_390", "korean_sebeolsik_final")
+            || (Settings.getInstance().current.mPopupKeyTypes.contains(POPUP_KEYS_LAYOUT)
+                && params.mId.mSubtype.mainLayoutName in listOf("lao", "thai", "korean_sebeolsik_390", "korean_sebeolsik_final")
+            )
 
     companion object {
         private const val TAG = "KeyboardParser"

app/src/main/java/helium314/keyboard/latin/DictionaryFactory.kt

@@ -27,7 +27,7 @@ import java.util.Locale
  * @return an initialized instance of DictionaryCollection
  */
 fun createMainDictionary(context: Context, locale: Locale): DictionaryCollection {
-    val cacheDir = DictionaryInfoUtils.getCacheDirectoryForLocale(locale, context)
+    val cacheDir = DictionaryInfoUtils.getAndCreateCacheDirectoryForLocale(locale, context)
     val dictList = LinkedList<Dictionary>()
     // get cached dict files
     val (userDicts, extractedDicts) = DictionaryInfoUtils.getCachedDictsForLocale(locale, context)

app/src/main/java/helium314/keyboard/latin/settings/LanguageSettingsDialog.kt

@@ -392,7 +392,7 @@ class LanguageSettingsDialog(
 private fun getUserAndInternalDictionaries(context: Context, locale: Locale): Pair<List<File>, Boolean> {
     val userDicts = mutableListOf<File>()
     var hasInternalDict = false
-    val userLocaleDir = File(DictionaryInfoUtils.getCacheDirectoryForLocale(locale, context))
+    val userLocaleDir = File(DictionaryInfoUtils.getAndCreateCacheDirectoryForLocale(locale, context))
     if (userLocaleDir.exists() && userLocaleDir.isDirectory) {
         userLocaleDir.listFiles()?.forEach {
             if (it.name.endsWith(USER_DICTIONARY_SUFFIX))

app/src/main/java/helium314/keyboard/latin/utils/DictionaryInfoUtils.java

@@ -115,9 +115,8 @@ public class DictionaryInfoUtils {
     /**
      * Find out the cache directory associated with a specific locale.
      */
-    public static String getCacheDirectoryForLocale(final Locale locale, final Context context) {
+    public static String getAndCreateCacheDirectoryForLocale(final Locale locale, final Context context) {
-        final String relativeDirectoryName = replaceFileNameDangerousCharacters(locale.toLanguageTag());
+        final String absoluteDirectoryName = getCacheDirectoryForLocale(locale, context);
-        final String absoluteDirectoryName = getWordListCacheDirectory(context) + File.separator + relativeDirectoryName;
         final File directory = new File(absoluteDirectoryName);
         if (!directory.exists()) {
             if (!directory.mkdirs()) {
@@ -127,8 +126,13 @@ public class DictionaryInfoUtils {
         return absoluteDirectoryName;
     }
 
+    public static String getCacheDirectoryForLocale(final Locale locale, final Context context) {
+        final String relativeDirectoryName = replaceFileNameDangerousCharacters(locale.toLanguageTag());
+        return getWordListCacheDirectory(context) + File.separator + relativeDirectoryName;
+    }
+
     public static File[] getCachedDictsForLocale(final Locale locale, final Context context) {
-        final File cachedDir = new File(getCacheDirectoryForLocale(locale, context));
+        final File cachedDir = new File(getAndCreateCacheDirectoryForLocale(locale, context));
         if (!cachedDir.isDirectory())
             return new File[]{};
         return cachedDir.listFiles();

app/src/main/java/helium314/keyboard/latin/utils/NewDictionaryAdder.kt

@@ -107,7 +107,7 @@ class NewDictionaryAdder(private val context: Context, private val onAdded: ((Bo
 
     private fun addDictAndAskToReplace(header: DictionaryHeader, mainLocale: Locale) {
         val dictionaryType = header.mIdString.substringBefore(":")
-        val cacheDir = DictionaryInfoUtils.getCacheDirectoryForLocale(mainLocale, context)
+        val cacheDir = DictionaryInfoUtils.getAndCreateCacheDirectoryForLocale(mainLocale, context)
         val dictFile = File(cacheDir, dictionaryType + "_" + USER_DICTIONARY_SUFFIX)
 
         fun moveDict(replaced: Boolean) {

app/src/main/java/helium314/keyboard/latin/utils/SubtypeUtils.kt

@@ -177,9 +177,10 @@ data class SettingsSubtype(val locale: Locale, val extraValues: String) {
                 it == ExtraValue.ASCII_CAPABLE
                         || it == ExtraValue.EMOJI_CAPABLE
                         || it == ExtraValue.IS_ADDITIONAL_SUBTYPE
-                // todo: this is in "old" additional subtypes, but where was it set?
+                // todo: UNTRANSLATABLE_STRING_IN_SUBTYPE_NAME is in "old" additional subtypes, but where was it set?
-                //  must have been by app in 2.3, but not any more?
+                //  must have been done by 2.3, can't find it being done explicitly
                 //  anyway, a. we can easily create it again, and b. it may contain "bad" characters messing up the extra value
+                //  see also todo at createAdditionalSubtype
                 // removing UNTRANSLATABLE_STRING_IN_SUBTYPE_NAME changes the name of some layouts,
                 // e.g. from "English (United States)" to "English (US)"
                         || it.startsWith(ExtraValue.UNTRANSLATABLE_STRING_IN_SUBTYPE_NAME)

app/src/main/java/helium314/keyboard/latin/utils/SubtypeUtilsAdditional.kt

@@ -23,7 +23,6 @@ object SubtypeUtilsAdditional {
     // todo: extra value does not contain UNTRANSLATABLE_STRING_IN_SUBTYPE_NAME for custom layout
     //  it did contain that key in 2.3, but where was it set? anyway, need to be careful with separators if we want to use it
     //  see also todo in SettingsSubtype
-    // todo: the name always contains the layout, but we may just use the original one
     fun createAdditionalSubtype(locale: Locale, extraValue: String, isAsciiCapable: Boolean,
                                         isEmojiCapable: Boolean): InputMethodSubtype {
         val mainLayoutName = LayoutType.getMainLayoutFromExtraValue(extraValue) ?: "qwerty"
@@ -53,7 +52,7 @@ object SubtypeUtilsAdditional {
     fun createEmojiCapableAdditionalSubtype(locale: Locale, mainLayoutName: String, asciiCapable: Boolean) =
         createAdditionalSubtype(locale, "${ExtraValue.KEYBOARD_LAYOUT_SET}=MAIN${Separators.KV}$mainLayoutName", asciiCapable, true)
 
-    // todo: consider using SettingsSubtype
+    // todo: consider using SettingsSubtype (nah, this can be removed after removing old settings)
     fun addAdditionalSubtype(prefs: SharedPreferences, subtype: InputMethodSubtype) {
         val oldAdditionalSubtypesString = prefs.getString(Settings.PREF_ADDITIONAL_SUBTYPES, Defaults.PREF_ADDITIONAL_SUBTYPES)!!
         val additionalSubtypes = createAdditionalSubtypes(oldAdditionalSubtypesString).toMutableSet()

app/src/main/java/helium314/keyboard/settings/dialogs/LayoutEditDialog.kt

@@ -91,8 +91,8 @@ fun LayoutEditDialog(
                     trailingIcon = { if (!nameValid) Icon(painterResource(R.drawable.ic_close), null) },
                 )
         },
-        checkTextValid = {
+        checkTextValid = { text ->
-            val valid = LayoutUtilsCustom.checkLayout(it, ctx)
+            val valid = LayoutUtilsCustom.checkLayout(text, ctx)
             errorJob?.cancel()
             if (!valid) {
                 errorJob = scope.launch {
@@ -105,13 +105,13 @@ fun LayoutEditDialog(
             }
             valid && nameValid // don't allow saving with invalid name, but inform user about issues with layout content
         },
-        // todo: this looks weird when the text field is not covered by the keyboard (long dialog)
+        // this looks weird when the text field is not covered by the keyboard (long dialog)
-        //  but better than not seeing the bottom part of the field...
+        // but better than not seeing the bottom part of the field...
         modifier = Modifier.padding(bottom = with(LocalDensity.current)
             { (bottomInsets / 2 + 36).toDp() }), // why is the /2 necessary?
         reducePadding = true,
     )
 }
 
-// the job is here to make sure old jobs are canceled
+// the job is here (outside the composable to make sure old jobs are canceled
 private var errorJob: Job? = null

app/src/main/java/helium314/keyboard/settings/dialogs/NewDictionaryDialog.kt

@@ -51,7 +51,7 @@ fun NewDictionaryDialog(
         val locales = SubtypeSettings.getAvailableSubtypeLocales()
             .filter { it.script() == dictLocale.script() || it.script() == mainLocale?.script() }
             .sortedWith(comparer)
-        val cacheDir = DictionaryInfoUtils.getCacheDirectoryForLocale(locale, ctx)
+        val cacheDir = DictionaryInfoUtils.getAndCreateCacheDirectoryForLocale(locale, ctx)
         val dictFile = File(cacheDir, header.mIdString.substringBefore(":") + "_" + USER_DICTIONARY_SUFFIX)
         val type = header.mIdString.substringBefore(":")
         val info = header.info(ctx.resources.configuration.locale())

app/src/main/java/helium314/keyboard/settings/dialogs/TextInputDialog.kt

@@ -66,7 +66,6 @@ fun TextInputDialog(
         modifier = modifier,
         title = title,
         text = {
-            // todo: this sometimes looks weird on start (like non-outlined field where the label is in the text field)
             OutlinedTextField(
                 value = value,
                 onValueChange = { value = it },

app/src/main/java/helium314/keyboard/settings/screens/DictionaryScreen.kt

@@ -44,7 +44,8 @@ fun DictionaryScreen(
 ) {
     val ctx = LocalContext.current
     val enabledLanguages = SubtypeSettings.getEnabledSubtypes(ctx.prefs(), true).map { it.locale().language }
-    val comparer = compareBy<Locale>({ it.language !in enabledLanguages }, { it.displayName }) // todo: could also prefer if there is a user-added dict
+    val cachedDictFolders = DictionaryInfoUtils.getCachedDirectoryList(ctx).orEmpty().map { it.name }
+    val comparer = compareBy<Locale>({ it.language !in enabledLanguages }, { it.toLanguageTag() !in cachedDictFolders}, { it.displayName })
     val dictionaryLocales = getDictionaryLocales(ctx).sortedWith(comparer).toMutableList()
     dictionaryLocales.add(0, Locale(SubtypeLocaleUtils.NO_LANGUAGE))
     var selectedLocale: Locale? by remember { mutableStateOf(null) }
@@ -55,26 +56,26 @@ fun DictionaryScreen(
         title = { Text(stringResource(R.string.dictionary_settings_category)) },
         filteredItems = { term ->
             if (term.isBlank()) dictionaryLocales
-            else dictionaryLocales.filter {
+            else dictionaryLocales.filter { loc ->
-                    it.language != SubtypeLocaleUtils.NO_LANGUAGE &&
+                    loc.language != SubtypeLocaleUtils.NO_LANGUAGE
-                    it.localizedDisplayName(ctx).replace("(", "")
+                            && loc.localizedDisplayName(ctx).replace("(", "")
-                        .splitOnWhitespace().any { it.startsWith(term, true) }
+                                .splitOnWhitespace().any { it.startsWith(term, true) }
                 }
         },
-        itemContent = {
+        itemContent = { locale ->
-            if (it.language == SubtypeLocaleUtils.NO_LANGUAGE) {
+            if (locale.language == SubtypeLocaleUtils.NO_LANGUAGE) {
                 Text(stringResource(R.string.add_new_dictionary_title), Modifier.clickable { showAddDictDialog = true })
             } else {
                 Column(
-                    Modifier.clickable { selectedLocale = it }
+                    Modifier.clickable { selectedLocale = locale }
                         .padding(vertical = 6.dp, horizontal = 16.dp)
                         .fillMaxWidth()
                 ) {
-                    val (dicts, hasInternal) = getUserAndInternalDictionaries(ctx, it)
+                    val (dicts, hasInternal) = getUserAndInternalDictionaries(ctx, locale)
                     val types = dicts.mapTo(mutableListOf()) { it.name.substringBefore("_${USER_DICTIONARY_SUFFIX}") }
                     if (hasInternal && !types.contains(Dictionary.TYPE_MAIN))
                         types.add(0, stringResource(R.string.internal_dictionary_summary))
-                    Text(it.localizedDisplayName(ctx))
+                    Text(locale.localizedDisplayName(ctx))
                     Text(
                         types.joinToString(", "),
                         style = MaterialTheme.typography.bodyMedium,