Languages are not sorted alphabetically in Inkscape preferences

Bug #600267 reported by Mirek2
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Fix Released
Low
Fernando Lucchesi

Bug Description

Languages in the language selection menu should be sorted alphabetically. Although the English version seems to display them in alphabetical order, German (de) and Greek (el) appear before English. It would then appear that the languages were sorted by short name (de, el, etc.), but this is not so: Spanish (es), for example, is listed under 'S', and there are various other instances.
In other language versions, the menu item order appears even more random and is harder to browse.

Tags: preferences ui
su_v (suv-lp)
tags: added: preferences ui
removed: international language options
Revision history for this message
jazzynico (jazzynico) wrote :

Confirmed.
This is a static list in ui/dialog/inkscape-preferences.cpp. Parts of this list are sorted by language code, some others sorted by English name.
Sorting by language code looks more consistent.

Changed in inkscape:
assignee: nobody → JazzyNico (jazzynico)
importance: Undecided → Low
milestone: none → 0.49
status: New → Confirmed
Revision history for this message
Fernando Lucchesi (fernandolbastos) wrote :

Hi,
I'm willing to send a patch to fix this bug(as a practice for gsoc), but I'm not sure what I should do.
Should I sort the list "by hand" according to the language code, or is it better to write some sorting routine, perhaps based on the translated name?

And why are the language names localized, instead of using their own version of the name(like the language selection of Ubuntu live cd)?
Thanks.

Revision history for this message
jazzynico (jazzynico) wrote :

Hi Fernando,

> Should I sort the list "by hand" according to the language code
> or is it better to write some sorting routine, perhaps based on the translated name?

It would be better to use the translated name. Ideally, it would be even better if we could detect and list the installed languages (sorted), but it would require some more work (and possibly different code for Linux/OSX and Windows...).

Revision history for this message
Fernando Lucchesi (fernandolbastos) wrote :

I think this patch solves it.

It sorts the two vectors according to the translated string, but also makes the system default the first option on the list. It seems to work for the languages that I tried(en, pt_BR, es, etc).

I used a "textbook" insertion sort to do the job, perhaps there is a more cpp-ish way of doing it.

I don't know if the coding style is acceptable, I tried to follow it but perhaps I missed something.
(I personally would prefer all the conditions of the "while" on a single line, but I didn't grok the limit yet)

As for detecting the installed languages, I don't know if it would be worth all the work only to limit the user's choices(I actually find it fun to open random locales, but that's just me =).

Revision history for this message
Jaspervdg (jaspervdg) wrote :

I would recommend going with std::sort from "algorithm" (or possibly a glib equivalent if it exists), much less chance of some subtle bug and more readable code (normally speed would of course also be a consideration, but given that you're sorting languages that's probably no problem anyway, insertion sort might even be faster in most cases).

You are sorting pairs of things (some code and the language name?), so it might take you slightly more effort to use std::sort than normal (they really should have made a standard tool for this in STL, it happens too often), but there are a couple of ways to deal with that. If you want an almost out of the box solution you could have a look at boost's zip_iterator, it would need a custom comparison, but you'll probably want that anyway.

Revision history for this message
Jaspervdg (jaspervdg) wrote :

BTW, I should have written "pairs of things from two different arrays" or something like that (sorting pairs, as in std::pair, is of course no problem).

jazzynico (jazzynico)
Changed in inkscape:
assignee: JazzyNico (jazzynico) → Fernando Lucchesi Bastos Jurema (fernandolbastos)
status: Confirmed → In Progress
Revision history for this message
Fernando Lucchesi (fernandolbastos) wrote :

Interesting, I didn't know boost, it seems to be quite useful.

I agree we usually shouldn't use custom sort routines, but it seems to be one of the best solutions to this specific problem(as the vectors are small and there's no standard way of sorting two separate vectors at once).

I've searched the web to see if anyone had already done anything like that using boost. However, there seems to be no easy way to use boost iterator with std::sort:
http://boost.2283326.n4.nabble.com/iterator-std-sort-on-zip-iterator-td2659322.html

There is a way, but IMHO it wouldn't really increase readability, and has even more chances of having bugs:
http://www.stanford.edu/~dgleich/notebook/2006/03/sorting_two_arrays_simultaneou.html

Another option would be to, instead of using boost, make a custom vector class(of pairs, maybe), only so that std::sort could correctly "view" the two vectors as a single thing. This class would have pointers to the original vectors(instead of a copy of its contents), and we would overload its access operator([]) to use directly the values on the original vectors. I'm not sure if it would work "as is", but I'll play with this idea a little.

(We will also need a custom compare routine, but the other solutions also need it).

Revision history for this message
Jaspervdg (jaspervdg) wrote :

Okay... Interesting material. Lets put this in the "not your problem to solve" box for now :) Especially since the sort does appear to be correct.

One final question: Any particular reason to allocate the Glib::ustrings using new? (Instead of on the stack.) If so, could you make sure they get "deleted"? If there is no particular reason to allocate them on the heap, then I would recommend allocating them on the stack (no pointer, no new). In either case, as far as I am concerned the patch could be committed asap (I could probably do it this weekend).

Revision history for this message
Fernando Lucchesi (fernandolbastos) wrote :

>If there is no particular reason to allocate them on the heap, then I would recommend allocating them on the stack (no pointer, no new).

Sure, now I changed it to use the stack.

Revision history for this message
Felipe "Juca" Sanches (felipe-sanches) wrote :

Fix committed to trunk with minor changes to use Glib::ustring instead of pointers to Glib::ustring.

http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/10219

Changed in inkscape:
status: In Progress → Fix Committed
Bryce Harrington (bryce)
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.