[Geany-devel] ui_menu_add_document_items_sorted() and document_sort_by_display_name()

[Nick Treleaven]

On Sun, 10 Apr 2011 17:03:41 +0000
eht16 at users.sourceforge.net wrote:

> Revision: 5705
>           http://geany.svn.sourceforge.net/geany/?rev=5705&view=rev
> Author:   eht16
> Date:     2011-04-10 17:03:41 +0000 (Sun, 10 Apr 2011)
> 
> Log Message:
> -----------
> Add ui_menu_add_document_items_sorted() and document_sort_by_display_name() to the plugin API.

Some comments on these:

> +/** GCompareFunc-like sort function to sort documents, e.g. in a GPtrArray by
> + *  their display names.
> + *  Display names means the base name of the document's file.
> + *
> + * @param a @c document a.
> + * @param b @c document b.
> + * @return negative value if a < b; zero if a = b; positive value if a > b.
> + *
> + *  @since 0.21
> + */
> +gint document_sort_by_display_name(gconstpointer a, gconstpointer b)

I think this should be renamed document_cmp_by_display_name
(or document_compare_by_display_name). The function doesn't actually
sort anything.

> +/** Adds a list of document items to @a menu.
> + *
> + * @a sort_func might be NULL to not sort the documents in the menu. In this case,
> + * the order of the document tabs is used.
> + *
> + * See document_sort_by_display_name() for an example sort function.
> + *
> + * @param menu Menu.
> + * @param active Which document to highlight, or @c NULL.
> + * @param callback is used for each menu item's @c "activate" signal and will be passed
> + * the corresponding document pointer as @c user_data.
> + * @param sort_func is used to sort the list. Might be @c NULL to not sort the list.
> + * @warning You should check @c doc->is_valid in the callback.
> + * @since 0.21 */
> +void ui_menu_add_document_items_sorted(GtkMenu *menu, GeanyDocument *active,

I think it would be better if when sort_func is NULL the items are
sorted by display name, because in that case the function is the same as
ui_menu_add_document_items, so no one would pass NULL.

I think these 2 changes could actually be done without breaking the
ABI (even though the second one changes behaviour, it shouldn't affect
plugins badly).

> +	for (i = 0; i < GEANY(sorted_documents)->len; i++)

The GEANY() macro does not do anything in core code BTW.

Regards,
Nick