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 694974 - Don't special case the last position in the remote search order
Don't special case the last position in the remote search order
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks: 694975
 
 
Reported: 2013-03-02 01:27 UTC by Giovanni Campagna
Modified: 2013-04-02 19:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
RemoteSearch: don't sort the last explicitly sorted provider last (1.54 KB, patch)
2013-03-02 01:29 UTC, Giovanni Campagna
committed Details | Review

Description Giovanni Campagna 2013-03-02 01:27:23 UTC
This is a follow up from https://bugzilla.gnome.org/show_bug.cgi?id=687491#c33 after that got closed.

To me, special casing the last position is very unexpected, and if I didn't find that comment I would simple assume it was a very weird bug in the sorting function.
Additionally, it causes misbehavior in the search panel, when dealing with a mix of sorted and unsorted items (which randomly become first or last in the list).
I believe it is a lot simpler, logically and implementation-wise, to have explicitly sorted elements first, and then everything else in alphabetical order.
Comment 1 Giovanni Campagna 2013-03-02 01:29:04 UTC
Created attachment 237770 [details] [review]
RemoteSearch: don't sort the last explicitly sorted provider last

It's a confusing semantic, and keeping it causes bugs in the control
center panels.
Comment 2 Giovanni Campagna 2013-03-10 18:03:34 UTC
Any comments on this?
I'd like to solve the related control center bug before 3.8
Comment 3 Jasper St. Pierre (not reading bugmail) 2013-03-10 18:18:27 UTC
What's the control-center bug? And you'd have to change the gsettings schema description as well.
Comment 4 Giovanni Campagna 2013-03-10 18:19:58 UTC
I forgot to link it, it's bug 694975.
Comment 5 Cosimo Cecchi 2013-04-02 18:09:33 UTC
Review of attachment 237770 [details] [review]:

The patch looks good to me; removing the special casing makes a lot of sense code-wise and semantically, but doesn't completely adhere to the suggested design.
I think it's okay regardless, as long as we pay attention to keeping the default list in GSettings updated when new core applications gain a search provider.

Like Jasper said, the gsettings-desktop-schemas key description should be updated as well; other than that, let's do this — if this really proves to be a problem in the future, we could introduce another different key to store one single value to go last if set. Up to the Shell and gnome-control-center maintainers to decide if they want it for 3.8 or only for master.
Comment 6 Florian Müllner 2013-04-02 18:13:57 UTC
(In reply to comment #5)
> Up to the Shell and gnome-control-center maintainers to decide if they 
> want it for 3.8 or only for master.

For gnome-shell, master *is* 3.8. I'm fine with targeting 3.8.1.
Comment 7 Giovanni Campagna 2013-04-02 19:51:11 UTC
Attachment 237770 [details] pushed as 78272e5 - RemoteSearch: don't sort the last explicitly sorted provider last