diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b6c701c8..0ceccf41b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ #### What's New - Reworked music hashing to be even more reliable (Will wipe playback state) +#### What's Improved +- Sorting now takes accented characters into account + #### What's Fixed - Fixed issue where the scroll popup would not display correctly in landscape mode [#230] - Fixed issue where the playback progress would continue in the notification even if diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt index 4e64c15ad..c3c0a6a6a 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/AlbumListFragment.kt @@ -64,10 +64,10 @@ class AlbumListFragment : HomeListFragment() { // Change how we display the popup depending on the mode. return when (homeModel.getSortForDisplay(DisplayMode.SHOW_ALBUMS).mode) { // By Name -> Use Name - is Sort.Mode.ByName -> album.sortName?.run { first().uppercase() } + is Sort.Mode.ByName -> album.collationKey?.run { sourceString.first().uppercase() } // By Artist -> Use Artist Name - is Sort.Mode.ByArtist -> album.artist.sortName?.run { first().uppercase() } + is Sort.Mode.ByArtist -> album.artist.collationKey?.run { sourceString.first().uppercase() } // Year -> Use Full Year is Sort.Mode.ByYear -> album.date?.resolveYear(requireContext()) diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt index 7077c3bf4..b7cef8180 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/ArtistListFragment.kt @@ -59,7 +59,7 @@ class ArtistListFragment : HomeListFragment() { // Change how we display the popup depending on the mode. return when (homeModel.getSortForDisplay(DisplayMode.SHOW_ARTISTS).mode) { // By Name -> Use Name - is Sort.Mode.ByName -> artist.sortName?.run { first().uppercase() } + is Sort.Mode.ByName -> artist.collationKey?.run { sourceString.first().uppercase() } // Duration -> Use formatted duration is Sort.Mode.ByDuration -> artist.durationMs.formatDurationMs(false) diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt index 2e123197a..73d1d0c04 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/GenreListFragment.kt @@ -59,7 +59,7 @@ class GenreListFragment : HomeListFragment() { // Change how we display the popup depending on the mode. return when (homeModel.getSortForDisplay(DisplayMode.SHOW_GENRES).mode) { // By Name -> Use Name - is Sort.Mode.ByName -> genre.sortName?.run { first().uppercase() } + is Sort.Mode.ByName -> genre.collationKey?.run { sourceString.first().uppercase() } // Duration -> Use formatted duration is Sort.Mode.ByDuration -> genre.durationMs.formatDurationMs(false) diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt index 418c6e1c9..845592583 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt @@ -74,13 +74,13 @@ class SongListFragment : HomeListFragment() { // based off the names of the parent objects and not the child objects. return when (homeModel.getSortForDisplay(DisplayMode.SHOW_SONGS).mode) { // Name -> Use name - is Sort.Mode.ByName -> song.sortName?.run { first().uppercase() } + is Sort.Mode.ByName -> song.collationKey?.run { sourceString.first().uppercase() } // Artist -> Use Artist Name - is Sort.Mode.ByArtist -> song.album.artist.sortName?.run { first().uppercase() } + is Sort.Mode.ByArtist -> song.album.artist.collationKey?.run { sourceString.first().uppercase() } // Album -> Use Album Name - is Sort.Mode.ByAlbum -> song.album.sortName?.run { first().uppercase() } + is Sort.Mode.ByAlbum -> song.album.collationKey?.run { sourceString.first().uppercase() } // Year -> Use Full Year is Sort.Mode.ByYear -> song.album.date?.resolveYear(requireContext()) diff --git a/app/src/main/java/org/oxycblt/auxio/music/Music.kt b/app/src/main/java/org/oxycblt/auxio/music/Music.kt index e87b4a373..79c7918ce 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Music.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Music.kt @@ -32,6 +32,8 @@ import org.oxycblt.auxio.util.inRangeOrNull import org.oxycblt.auxio.util.nonZeroOrNull import org.oxycblt.auxio.util.unlikelyToBeNull import java.security.MessageDigest +import java.text.CollationKey +import java.text.Collator import java.util.UUID import kotlin.math.max import kotlin.math.min @@ -50,27 +52,29 @@ sealed class Music : Item { abstract val rawSortName: String? /** - * The name of this item used for sorting.This should not be used outside of sorting and - * fast-scrolling. - */ - val sortName: String? - get() = - rawSortName - ?: rawName?.run { - when { - length > 5 && startsWith("the ", ignoreCase = true) -> substring(4) - length > 4 && startsWith("an ", ignoreCase = true) -> substring(3) - length > 3 && startsWith("a ", ignoreCase = true) -> substring(2) - else -> this - } - } - - /** - * Resolve a name from it's raw form to a form suitable to be shown in a ui. Ex. "unknown" would - * become Unknown Artist, (124) would become its proper genre name, etc. + * Resolve a name from it's raw form to a form suitable to be shown in a UI. + * Null values will be resolved into their string form with this function. */ abstract fun resolveName(context: Context): String + /** + * A key used by the sorting system that takes into account the sort tags of this item, + * any (english) articles that prefix the names, and collation rules. Lazily generated + * since generating a collation key is non-trivial. + */ + val collationKey: CollationKey? by lazy { + val sortName = (rawSortName ?: rawName)?.run { + when { + length > 5 && startsWith("the ", ignoreCase = true) -> substring(4) + length > 4 && startsWith("an ", ignoreCase = true) -> substring(3) + length > 3 && startsWith("a ", ignoreCase = true) -> substring(2) + else -> this + } + } + + COLLATOR.getCollationKey(sortName) + } + // Equality is based on UIDs, as some items (Especially artists) can have identical // properties (Name) yet non-identical UIDs due to MusicBrainz tags @@ -130,6 +134,12 @@ sealed class Music : Item { } } } + + companion object { + private val COLLATOR = Collator.getInstance().apply { + strength = Collator.PRIMARY + } + } } /** @@ -208,6 +218,8 @@ class Song constructor(raw: Raw) : Music() { * back to the album artist tag (i.e parent artist name). Null if name is unknown. */ val individualArtistRawName: String? + // Note: This is a getter since it relies on a parent value that will not be initialized + // yet on creation. get() = artistName ?: album.artist.rawName /** @@ -263,8 +275,8 @@ class Song constructor(raw: Raw) : Music() { // by now. uid = UID.hashed(this::class) { - update(rawName.lowercase()) - update(_rawAlbum.name.lowercase()) + update(rawName) + update(_rawAlbum.name) update(_rawAlbum.date) update(artistName) @@ -579,26 +591,20 @@ class Date private constructor(private val tokens: List) : Comparable } } - val year: Int - get() = tokens[0] + val year = tokens[0] /** Resolve the year field in a way suitable for the UI. */ fun resolveYear(context: Context) = context.getString(R.string.fmt_number, year) - private val month: Int? - get() = tokens.getOrNull(1) + private val month = tokens.getOrNull(1) - private val day: Int? - get() = tokens.getOrNull(2) + private val day = tokens.getOrNull(2) - private val hour: Int? - get() = tokens.getOrNull(3) + private val hour = tokens.getOrNull(3) - private val minute: Int? - get() = tokens.getOrNull(4) + private val minute = tokens.getOrNull(4) - private val second: Int? - get() = tokens.getOrNull(5) + private val second = tokens.getOrNull(5) override fun hashCode() = tokens.hashCode() diff --git a/app/src/main/java/org/oxycblt/auxio/music/extractor/CacheLayer.kt b/app/src/main/java/org/oxycblt/auxio/music/extractor/CacheLayer.kt index ecd45f71c..5e4efde73 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/extractor/CacheLayer.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/extractor/CacheLayer.kt @@ -31,5 +31,3 @@ class CacheLayer { fun maybePopulateCachedRaw(raw: Song.Raw) = false } - -// TODO: Make raw naming consistent (always rawSong(s), not raw) diff --git a/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt b/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt index b64cd5bcd..0a03487c5 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt @@ -256,9 +256,9 @@ class Indexer { val songs = mutableSetOf() val rawSongs = mutableListOf() - metadataLayer.parse { raw -> - songs.add(Song(raw)) - rawSongs.add(raw) + metadataLayer.parse { rawSong -> + songs.add(Song(rawSong)) + rawSongs.add(rawSong) // Check if we got cancelled after every song addition. yield() diff --git a/app/src/main/java/org/oxycblt/auxio/music/system/IndexerNotifications.kt b/app/src/main/java/org/oxycblt/auxio/music/system/IndexerNotifications.kt index 71a1530eb..368eb4547 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/system/IndexerNotifications.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/system/IndexerNotifications.kt @@ -30,7 +30,7 @@ import org.oxycblt.auxio.util.newMainPendingIntent /** The notification responsible for showing the indexer state. */ class IndexingNotification(private val context: Context) : ServiceNotification(context, INDEXER_CHANNEL) { - private var lastUpdateTime: Int = -1 + private var lastUpdateTime = -1L init { setSmallIcon(R.drawable.ic_indexer_24) @@ -51,16 +51,19 @@ class IndexingNotification(private val context: Context) : when (indexing) { is Indexer.Indexing.Indeterminate -> { logD("Updating state to $indexing") + lastUpdateTime = -1 setContentText(context.getString(R.string.lng_indexing)) setProgress(0, 0, true) return true } is Indexer.Indexing.Songs -> { val now = SystemClock.elapsedRealtime() - if (lastUpdateTime != -1 && (now - lastUpdateTime) < 1500) { + if (lastUpdateTime > -1 && (now - lastUpdateTime) < 1500) { return false } + lastUpdateTime = SystemClock.elapsedRealtime() + // Only update the notification every two seconds to prevent rate-limiting. logD("Updating state to $indexing") setContentText( diff --git a/app/src/main/java/org/oxycblt/auxio/search/SearchViewModel.kt b/app/src/main/java/org/oxycblt/auxio/search/SearchViewModel.kt index 3b8b2f3c5..34a5e3ee4 100644 --- a/app/src/main/java/org/oxycblt/auxio/search/SearchViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/search/SearchViewModel.kt @@ -157,44 +157,21 @@ class SearchViewModel(application: Application) : private fun List.filterGenresBy(value: String) = baseFilterBy(value) { false } - private inline fun List.baseFilterBy(value: String, additional: (T) -> Boolean) = + private inline fun List.baseFilterBy(value: String, fallback: (T) -> Boolean) = filter { // The basic comparison is first by the *normalized* name, as that allows a - // non-unicode search to match with some unicode characters. If that fails, - // filter impls have fallback values, primarily around sort tags or file names. + // non-unicode search to match with some unicode characters. In an ideal world, we + // would just want to leverage CollationKey, but that is not designed for a contains + // algorithm. If that fails, filter impls have fallback values, primarily around + // sort tags or file names. it.resolveNameNormalized(application).contains(value, ignoreCase = true) || - additional(it) + fallback(it) } .ifEmpty { null } private fun Music.resolveNameNormalized(context: Context): String { - // This method normalizes strings so that songs with accented characters will show - // up in search even if the actual character was not inputted. - // https://stackoverflow.com/a/32030586/14143986 - - // Normalize with NFKD [Meaning that symbols with identical meanings will be turned into - // their letter variants]. val norm = Normalizer.normalize(resolveName(context), Normalizer.Form.NFKD) - - // Normalizer doesn't exactly finish the job though. We have to rebuild all the codepoints - // in the string and remove the hidden characters that were added by Normalizer. - var idx = 0 - val sb = StringBuilder() - - while (idx < norm.length) { - val cp = norm.codePointAt(idx) - idx += Character.charCount(cp) - - when (Character.getType(cp)) { - // Character.NON_SPACING_MARK and Character.COMBINING_SPACING_MARK were added - // by normalizer - 6, - 8 -> continue - else -> sb.appendCodePoint(cp) - } - } - - return sb.toString() + return NORMALIZATION_SANITIZE_REGEX.replace(norm, "") } override fun onLibraryChanged(library: MusicStore.Library?) { @@ -208,4 +185,8 @@ class SearchViewModel(application: Application) : super.onCleared() musicStore.removeCallback(this) } + + companion object { + private val NORMALIZATION_SANITIZE_REGEX = Regex("\\p{InCombiningDiacriticalMarks}+") + } } diff --git a/app/src/main/java/org/oxycblt/auxio/ui/Sort.kt b/app/src/main/java/org/oxycblt/auxio/ui/Sort.kt index 107a6f0ad..7047bc01c 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/Sort.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/Sort.kt @@ -369,16 +369,13 @@ data class Sort(val mode: Mode, val isAscending: Boolean) { } private class BasicComparator private constructor() : Comparator { - // TODO: Perhaps I should leverage collator? - override fun compare(a: T, b: T): Int { - val aSortName = a.sortName - val bSortName = b.sortName + val aKey = a.collationKey + val bKey = b.collationKey return when { - aSortName != null && bSortName != null -> - aSortName.compareTo(bSortName, ignoreCase = true) - aSortName == null && bSortName != null -> -1 // a < b - aSortName == null && bSortName == null -> 0 // a = b + aKey != null && bKey != null -> aKey.compareTo(bKey) + aKey == null && bKey != null -> -1 // a < b + aKey == null && bKey == null -> 0 // a = b else -> 1 // a < b } }