Opened 13 years ago

Closed 13 years ago

#1678 closed patch (fixed)

mythvideo sorting is case-sensitive

Reported by: thomas@… Owned by: Anduin Withers
Priority: minor Milestone: 0.20
Component: mythvideo Version: 0.19
Severity: low Keywords:
Cc: Ticket locked: no

Description

Sorting of files (videos) in mythvideo is NOT case-sensitive in "Video Manager". It is case-sensitive in "Browse Videos", Video List" and Video Gallery".

It should be NOT case-sensitive everywhere. AFAIR, so was it in 0.18.

Attachments (6)

mythplugins-mythvideo-i18n_and_case_insensitive_sorting.patch (4.2 KB) - added by sphery <mtdean@…> 13 years ago.
mythplugins-mythvideo-i18n_and_case_insensitive_sorting-2.patch (4.3 KB) - added by sphery <mtdean@…> 13 years ago.
Updated patch--removes no-longer-required qmap.h include
mythplugins-mythvideo-titlesortkey_in_database.patch (15.0 KB) - added by sphery <mtdean@…> 13 years ago.
mythtv-mythvideo-titlesortkey_in_database.patch (2.1 KB) - added by sphery <mtdean@…> 13 years ago.
myththemes-mythvideo-titlesortkey_in_database.patch (2.3 KB) - added by sphery <mtdean@…> 13 years ago.
mythplugins-mythvideo-titlesortkey_in_database-2.patch (14.8 KB) - added by sphery <mtdean@…> 13 years ago.
Updates mythplugins patch (I had left an unwanted lower( ) in guessTitleSortKey( ))

Download all attachments as: .zip

Change History (15)

comment:1 Changed 13 years ago by sphery <mtdean@…>

Type: defectpatch

Attached patch fixes the issue described here by removing the QMap quick-and-easy sort and instead using a new "SortableMetadataList?" class to do the sort. As a side-effect of this change, the patch also:

  • Provides a locale-aware comparison (rather than using Unicode values), so sorting titles with umlauts, etc. will be correct
  • Appends a zero-padded video ID to the title/filename key to act as a "tertiary" sort key (the secondary sort key fails if filenames are identical except for case, even if lower() is called before appending the filename)
  • Does a case-insensitive sort--regardless of the locale's LC_COLLATE preference (US English sorting according to LC_COLLATE is case-sensitive, which seems against the reporter's wishes)

The compareItems() function is rather verbose to allow the committer to more easily change it's behavior. This is the only reason each change was made on a separate line, so feel free to condense the lines as desired. Note, also, that using filename as a secondary sort key is a new addition. Current SVN head is using the video ID as a secondary sort key since [9694].

After the patch, the only "unusual" behavior occurs when there exist at least two identically-titled (including case) videos in different directories and at least one with a similar title, differing only in case. In this situation, the identically-titled videos may be separated from one another by the video(s) with similar titles and different case. The same holds true where the titles differ only by an indefinite article at the beginning. Although it's possible to work around this (sort by case when titles are otherwise identical and include the article in the sort key when it is the only difference), I feel it's enough of an edge case (far out on the fringes of reality) that it's not worth the extra code/processing time. Let me know if you disagree. :)

Changed 13 years ago by sphery <mtdean@…>

comment:2 Changed 13 years ago by anonymous

Owner: changed from Isaac Richards to anonymous
Status: newassigned

comment:3 Changed 13 years ago by Anduin Withers

Owner: changed from anonymous to Anduin Withers
Status: assignednew

comment:4 Changed 13 years ago by gnassas@…

Some observations on this patch and ticket.

First, respecting case during sorting is desirable behaviour. If a change must be made it should be to respect case everywhere.

Second, the sorting order is different because video manager uses sql sorting and the other views use a map. This patch doesn't touch the sql sort so as long as there are two ordering techniques there will be mismatched lists. Presumably that's the original poster's real complaint.

Third, in #1569 I have to do some key manipulation to get folder view to work. The code reorg introduced here makes it complicated to apply that patch. I ask that the earlier ticket be applied first.

I like the idea of the sorting class so I'm going to poach it for #1569. If this ticket were to be deemed still be necessary after that it should focus on harmonizing the scan for videos between video manager and the other views.

comment:5 Changed 13 years ago by thomas@…

I repeat: My intention when opening this ticket was, that it is everywhere NOT case-sensitive. AFAIR, that's the way it was in 0.18.

Changed 13 years ago by sphery <mtdean@…>

Updated patch--removes no-longer-required qmap.h include

comment:6 Changed 13 years ago by sphery <mtdean@…>

Ignore the patch mythplugins-mythvideo-i18n_and_case_insensitive_sorting-2.patch--QMap is still used in videolist.{h,cpp}, so the include should remain. The original patch ( mythplugins-mythvideo-i18n_and_case_insensitive_sorting.patch ) should be used.

comment:7 Changed 13 years ago by sphery <mtdean@…>

Updated patches (*-mythvideo-titlesortkey_in_database.patch). As described at http://www.gossamer-threads.com/lists/mythtv/dev/199541#199541 , this patch modifies MythVideo to use a user-specifiable title-sort key that's stored in the database (per JDS's suggestion in the comment he added above the existing sort code). To support the new title-sort key, the patch makes the following changes:

  • Updates the DB schema and adds a column, titlesortkey, to the videometadata table of the database (the column is 256 characters--2x the size of the title column--to allow the user to specify secondary or tertiary sort keys as desired (within reason))
  • Creates a default "guess" value for titlesortkey (using the current value of title with initial indefinite articles removed)
  • Removes all in-application sorting code (now that sorting is done by the database)
  • Adds an editable text box for "Title-Sort Key" to the "Edit Video Metadata" screen
  • Updates all themes to support the new titlesortkey (the only changes in myththemes-mythvideo-titlesortkey_in_database.patch mythtv-mythvideo-titlesortkey_in_database.patch are theme updates)
  • Automatically guesses the titlesortkey when a new video is found during scan, when metadata is reset, when an IMDB movie data search is performed, or when metadata is written to the database with no value for titlesortkey
  • Changes all usage of the title sort (kOrderByTitle) to use the titlesortkey (this includes the Video Manager, so sorting will be consistent in all places within MythVideo)

Although the "guess" is not lower-case, since MySQL performs case-insensitive sorts, the patch fixes the issue in this ticket by moving the sorting back to MySQL

The SQL UPDATE in dbcheck.cpp should work for all supported versions of MySQL. It trims the initial indefinite articles from titles to create the titlesortkey using the translated value of the "indefinite articles regex" to ensure the initial titlesortkey are correct--even for non-English speakers.

The "guessed" value of titlesortkey does not include any secondary (or tertiary) sort keys. Therefore, a user who has multiple videos differing only in initial articles and/or case may choose to append additional key information to the titlesortkey to specify an order for these like-named videos. If the "guessed" value is used, the like-named videos will appear in the order they were added to the database (from oldest to newest), which is a reasonable default.

Users wanting "custom" sorting may either edit the "Title-Sort Key" via the GUI or may use SQL to update the titlesortkey column for multiple videos with a single command.

Note that the titlesortkey is only used when the user sets the view to be sorted by title (kOrderByTitle).

Changed 13 years ago by sphery <mtdean@…>

Changed 13 years ago by sphery <mtdean@…>

Changed 13 years ago by sphery <mtdean@…>

Changed 13 years ago by sphery <mtdean@…>

Updates mythplugins patch (I had left an unwanted lower( ) in guessTitleSortKey( ))

comment:8 Changed 13 years ago by Anduin Withers

Milestone: 0.20

comment:9 Changed 13 years ago by Anduin Withers

Resolution: fixed
Status: newclosed

(In [10852]) Closes #2197, #1647, #1746, #679, #1569, #1678, #2040, #2159, #2167, #2170

Various small cleanups and speed improvements.

  • Folders in metadata only view
  • Option to ignore case in title comparisons (though a locale may override this)
  • Multiple prefixes should be consistent now
  • All metadata fields are available to the theme in all views
  • Minor redraw issue
  • Consistent metadata representation across views (where appropriate)

Fixes fully (to my knowledge) all referenced tickets.

Note: See TracTickets for help on using tickets.