After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 742531 - Album sort - ignore punctuation
Album sort - ignore punctuation
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
3.15.x
Other Linux
: Normal major
: ---
Assigned To: Maia
gnome-music-maint
Depends on: 745917
Blocks:
 
 
Reported: 2015-01-07 16:01 UTC by Allan Day
Modified: 2016-07-23 10:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Ignore punctuation for sorting albums and artists (2.10 KB, patch)
2015-09-27 01:05 UTC, Kevin Haller
needs-work Details | Review
query: Ignore punctuation when sorting albums & artists (2.59 KB, patch)
2016-07-14 12:57 UTC, Divyanshu Vishwakarma
none Details | Review
Ignore punctuation when sorting albums & artists (3.72 KB, patch)
2016-07-17 16:24 UTC, Divyanshu Vishwakarma
none Details | Review
query: pep8 and pep257 changes (2.74 KB, patch)
2016-07-17 16:42 UTC, Divyanshu Vishwakarma
none Details | Review
Ignore punctuation when sorting albums & artists (2.74 KB, patch)
2016-07-22 12:42 UTC, Divyanshu Vishwakarma
committed Details | Review
pep8 fixes (2.98 KB, patch)
2016-07-22 12:43 UTC, Divyanshu Vishwakarma
committed Details | Review

Description Allan Day 2015-01-07 16:01:30 UTC
I have an album called "Now Listen!", which is placed at the very beginning of the grid of albums. I would expect the leading " to be ignored, and the album to be sorted according to Now Listen.
Comment 1 Vadim Rutkovsky 2015-01-07 16:08:37 UTC
Right, in query.order_by_statement we should strip punctuation signs when ordering albums
Comment 2 Ankit 2015-02-18 19:42:58 UTC
Hi Vadim Rutkovsky,
I've been trying to do this for quite a number of hours now. I've even learnt SPARQL and xQuery. I hoped that doing something like "ORDER BY fn:replace(...)" would work to replace the punctuation marks for ordering, but this function is not available for Tracker-SPARQL. 

Not even "REPLACE(...)" is working, it throws some error. 

I even tried to BIND but even BIND is working, again error.

How do you suggest I should go on to solve this bug?
Comment 3 Vadim Rutkovsky 2015-03-01 15:05:18 UTC
This seems to be an important issue for 3.16, Maia, would you like to work on that?
Comment 4 Maia 2015-03-02 15:42:52 UTC
Sure thing, I'll work on this today!

(In reply to Vadim Rutkovsky from comment #3)
> This seems to be an important issue for 3.16, Maia, would you like to work
> on that?
Comment 5 Kevin Haller 2015-09-27 01:05:21 UTC
Created attachment 312228 [details] [review]
Ignore punctuation for sorting albums and artists

Uses fn:replace with a regex to replace the punctuation at the start and at the end. I think, that is the easiest way to solve this bug.

Tracker does not support the use of regular expressions for fn:replace, so this patch will not work. I opened an issue and also proposed an implementation for it. https://bugzilla.gnome.org/show_bug.cgi?id=754961
Comment 6 Cédric Bellegarde 2016-05-10 05:19:37 UTC
Bug 754961 fixed, may be time to merge Kevin Haller patch ;)
Comment 7 Marinus Schraal 2016-05-14 11:10:02 UTC
Review of attachment 312228 [details] [review]:

The tracker dep should also be bumped to 1.9 for this, include it in the commit as well.

And follow the commit message guidelines as in https://wiki.gnome.org/Git/CommitMessages .

But looks good to go otherwise, thanks.

::: gnomemusic/query.py
@@ +80,3 @@
+            'Attr' should be given without parentheses, e.g., "attr='?author'"."""
+        return_statement = """ fn:replace(fn:lower-case(%(attribute)s),
+                                          "^[ %(punctuation)s]+|[ %(punctuation)s]+$", "")

Add the space to the punctuation string for clarity sake?

@@ +82,3 @@
+                                          "^[ %(punctuation)s]+|[ %(punctuation)s]+$", "")
+                           """.replace('\n', ' ').strip() % {'attribute': attr,
+                                                             'punctuation': "!\\\"#$%&'()*+,-./:;<=>?@[\\\\]^_`{|}~"}

Make the punctuation filter string global, it's like a constant.
Comment 8 Divyanshu Vishwakarma 2016-07-14 12:57:21 UTC
Created attachment 331491 [details] [review]
query: Ignore punctuation when sorting albums & artists

Makes use of the new regex support in Tracker 1.9
Uses fn:replace with a regex to remove punctuation
at beginning and end.

Bump min required tracker version to 1.9
Comment 9 Marinus Schraal 2016-07-15 09:20:47 UTC
Review of attachment 331491 [details] [review]:

Regarding the git comment: min=minimal & refer to the original patch author at the end (based on work by xx or something).

::: gnomemusic/query.py
@@ +66,2 @@
     @staticmethod
     def order_by_statement(attr):

This should really be _order_by_statement, but that switch needs a separate commit. (pep8 fix)

@@ +68,3 @@
         """Returns a SPARQL ORDER BY statement sorting by the given attribute, ignoring
+            articles as defined in _("the") as well as the punctuation at the start and the end of the string.
+            'Attr' should be given without parentheses, e.g., "attr='?author'"."""

Follow pep8 for line-length of the docstring and pep257 & sphinx for formatting (see README).

@@ +72,3 @@
+                                          "^[ %(punctuation)s ]+|[ %(punctuation)s ]+$", "")
+                           """.replace('\n', ' ').strip() % {'attribute': attr,
+                                                             'punctuation': PUNCTUATION_FILTER}

Space to filter.
pep8 line length.
Using triple quotes is ok (its used inconsistently throughout query.py, but we can settle on triple quotes for that), but then use it consistently in the whole function.

@@ +75,3 @@
         # TRANSLATORS: _("the") should be a space-separated list of all-lowercase articles
         # (such as 'the') that should be ignored when alphabetizing artists/albums. This
         # list should include 'the' regardless of language. If some articles occur more

PEP8 line length, but needs separate commit.
Comment 10 Divyanshu Vishwakarma 2016-07-17 16:24:09 UTC
Created attachment 331654 [details] [review]
Ignore punctuation when sorting albums & artists

Makes use of the new regex support in Tracker 1.9
Uses fn:replace with a regex to remove punctuation
at beginning and end.
Bump minimum required tracker version to 1.9
pep8 fix on order_by_statement
Based on work by Kevin Haller.
Comment 11 Divyanshu Vishwakarma 2016-07-17 16:42:36 UTC
Created attachment 331655 [details] [review]
query: pep8 and pep257 changes

pep8 and pep257 fix on Docstring.
pep8 line length fix on comment.
Based on work by Kevin Haller.
Comment 12 Divyanshu Vishwakarma 2016-07-22 12:42:13 UTC
Created attachment 331989 [details] [review]
Ignore punctuation when sorting albums & artists

Makes use of the new regex support in Tracker 1.9
Uses fn:replace with a regex to remove punctuation
at beginning and end.
Bump minimum required tracker version to 1.9
pep8 and pep257 fix on Docstring.
Based on work by Kevin Haller.
Comment 13 Divyanshu Vishwakarma 2016-07-22 12:43:38 UTC
Created attachment 331990 [details] [review]
pep8 fixes

pep8 line length fix on comment
pep8 fix on order_by_statement
Comment 14 Marinus Schraal 2016-07-23 10:17:08 UTC
This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.

Thanks Rishu for picking this up.