[vlc-devel] [PATCH] directory: reconcile numerical and alphabetical sorting
Jean-Baptiste Kempf
jb at videolan.org
Wed Nov 30 22:52:10 CET 2016
I very much like the idea, if it behaves like I expect it.
But, shouldn't we have tests to check the different cases for sorting?
BR,
On Wed, 30 Nov 2016, at 22:43, Rémi Denis-Courmont wrote:
> Also mark the --directory-sort option obsolete.
> ---
> include/vlc_access.h | 4 +-
> modules/access/fs.c | 1 +
> modules/demux/playlist/playlist.c | 12 ------
> src/input/access.c | 83
> ++++++++++++++++++---------------------
> 4 files changed, 41 insertions(+), 59 deletions(-)
>
> diff --git a/include/vlc_access.h b/include/vlc_access.h
> index dc91e47..0531035 100644
> --- a/include/vlc_access.h
> +++ b/include/vlc_access.h
> @@ -102,7 +102,6 @@ struct access_fsdir
> int i_sub_autodetect_fuzzy;
> bool b_show_hiddenfiles;
> char *psz_ignored_exts;
> - char *psz_sort;
> };
>
> /**
> @@ -117,8 +116,7 @@ VLC_API void access_fsdir_init(struct access_fsdir
> *p_fsdir,
> /**
> * Finish adding items to the node
> *
> - * \param b_success if true, items of the node will be sorted according
> - * "directory-sort" option.
> + * \param b_success if true, items of the node will be sorted.
> */
> VLC_API void access_fsdir_finish(struct access_fsdir *p_fsdir, bool
> b_success);
>
> diff --git a/modules/access/fs.c b/modules/access/fs.c
> index edce520..fc516b4 100644
> --- a/modules/access/fs.c
> +++ b/modules/access/fs.c
> @@ -52,4 +52,5 @@ vlc_module_begin ()
>
> add_bool("list-special-files", false, N_("List special files"),
> N_("Include devices and pipes when listing directories"),
> true)
> + add_obsolete_string("directory-sort") /* since 3.0.0 */
> vlc_module_end ()
> diff --git a/modules/demux/playlist/playlist.c
> b/modules/demux/playlist/playlist.c
> index 58f89ba..c8a74e9 100644
> --- a/modules/demux/playlist/playlist.c
> +++ b/modules/demux/playlist/playlist.c
> @@ -62,16 +62,6 @@ static const char *const psz_recursive_list_text[] = {
> "collapse: subdirectories appear but are expanded on first
> play.\n" \
> "expand: all subdirectories are expanded.\n" )
>
> -static const char *const psz_sort_list[] = { "collate", "version",
> "none" };
> -static const char *const psz_sort_list_text[] = {
> - N_("Sort alphabetically according to the current language's
> collation rules."),
> - N_("Sort items in a natural order (for example: 1.ogg 2.ogg 10.ogg).
> This method does not take the current language's collation rules into
> account."),
> - N_("Do not sort the items.") };
> -
> -#define SORT_TEXT N_("Directory sort order")
> -#define SORT_LONGTEXT N_( \
> - "Define the sort algorithm used when adding items from a directory."
> )
> -
> #define IGNORE_TEXT N_("Ignored extensions")
> #define IGNORE_LONGTEXT N_( \
> "Files with these extensions will not be added to playlist when
> " \
> @@ -182,8 +172,6 @@ vlc_module_begin ()
> change_string_list( psz_recursive_list,
> psz_recursive_list_text )
> add_string( "ignore-filetypes",
> "m3u,db,nfo,ini,jpg,jpeg,ljpg,gif,png,pgm,pgmyuv,pbm,pam,tga,bmp,pnm,xpm,xcf,pcx,tif,tiff,lbm,sfv,txt,sub,idx,srt,cue,ssa",
> IGNORE_TEXT, IGNORE_LONGTEXT, false )
> - add_string( "directory-sort", "collate", SORT_TEXT,
> SORT_LONGTEXT, false )
> - change_string_list( psz_sort_list, psz_sort_list_text )
> add_bool( "show-hiddenfiles", false,
> SHOW_HIDDENFILES_TEXT, SHOW_HIDDENFILES_LONGTEXT,
> false )
> vlc_module_end ()
> diff --git a/src/input/access.c b/src/input/access.c
> index 6c48431..ed3e40f 100644
> --- a/src/input/access.c
> +++ b/src/input/access.c
> @@ -29,6 +29,9 @@
> #include <stdlib.h>
> #include <string.h>
> #include <ctype.h>
> +#ifdef HAVE_STRCOLL
> +# define strcoll strcasecmp
> +#endif
>
> #include <vlc_common.h>
> #include <vlc_url.h>
> @@ -332,9 +335,7 @@ static int compar_type(input_item_t *p1, input_item_t
> *p2)
> return 0;
> }
>
> -/* Some code duplication between comparison functions.
> - * GNU qsort_r() would be needed to solve this. */
> -static int compar_collate(const void *a, const void *b)
> +static int compar_filename(const void *a, const void *b)
> {
> input_item_node_t *const *na = a, *const *nb = b;
> input_item_t *ia = (*na)->p_item, *ib = (*nb)->p_item;
> @@ -343,28 +344,40 @@ static int compar_collate(const void *a, const void
> *b)
> if (i_ret != 0)
> return i_ret;
>
> -#ifdef HAVE_STRCOLL
> - /* The program's LOCAL defines if case is ignored */
> - return strcoll(ia->psz_name, ib->psz_name);
> -#else
> - return strcasecmp(ia->psz_name, ib->psz_name);
> -#endif
> + size_t i;
> + char c;
> +
> + /* Attempt to guess if the sorting algorithm should be alphabetic
> + * (i.e. collation) or numeric:
> + * - If the first mismatching characters are not both digits,
> + * then collation is the only option.
> + * - If one of the first mismatching characters is 0 and the other
> is also
> + * a digit, the comparands are probably left-padded numerical
> values.
> + * It does not matter which algorithm is used: the zero will be
> smaller
> + * than non-zero either way.
> + * - Otherwise, the comparands are numerical values, and might not
> be
> + * aligned (i.e. not same order of magnitude). If so, collation
> would
> + * fail. So numerical comparison is performed. */
> + for (i = 0; (c = ia->psz_name[i]) == ib->psz_name[i]; i++)
> + if (c == '\0')
> + return 0; /* strings are exactly identical */
> +
> + if ((unsigned)(c - '0') > 9)
> + return strcoll(ia->psz_name, ib->psz_name);
> +
> + unsigned long long ua = strtoull(ia->psz_name + i, NULL, 10);
> + unsigned long long ub = strtoull(ib->psz_name + i, NULL, 10);
> +
> + /* The number may be identical in two cases:
> + * - leading zero (e.g. "012" and "12")
> + * - overflow on both sides (#ULLONG_MAX) */
> + if (ua == ub)
> + return strcoll(ia->psz_name, ib->psz_name);
> +
> + return (ua > ub) ? +1 : -1;
> }
>
> -static int compar_version(const void *a, const void *b)
> -{
> - input_item_node_t *const *na = a, *const *nb = b;
> - input_item_t *ia = (*na)->p_item, *ib = (*nb)->p_item;
> -
> - int i_ret = compar_type(ia, ib);
> - if (i_ret != 0)
> - return i_ret;
> -
> - return strverscmp(ia->psz_name, ib->psz_name);
> -}
> -
> -static void fsdir_sort_sub(input_item_node_t *p_node,
> - int (*compar)(const void *, const void *))
> +static void fsdir_sort(input_item_node_t *p_node)
> {
> if (p_node->i_children <= 0)
> return;
> @@ -376,7 +389,7 @@ static void fsdir_sort_sub(input_item_node_t *p_node,
>
> /* Sort current node */
> qsort(p_node->pp_children, p_node->i_children,
> - sizeof(input_item_node_t *), compar);
> + sizeof(input_item_node_t *), compar_filename);
>
> /* Unlock all children */
> for (int i = 0; i < p_node->i_children; i++)
> @@ -384,23 +397,7 @@ static void fsdir_sort_sub(input_item_node_t
> *p_node,
>
> /* Sort all children */
> for (int i = 0; i < p_node->i_children; i++)
> - fsdir_sort_sub(p_node->pp_children[i], compar);
> -}
> -
> -static void fsdir_sort(struct access_fsdir *p_fsdir)
> -{
> - int (*pf_compar)(const void *, const void *) = NULL;
> -
> - if (p_fsdir->psz_sort != NULL)
> - {
> - if (!strcasecmp(p_fsdir->psz_sort, "version"))
> - pf_compar = compar_version;
> - else if(strcasecmp(p_fsdir->psz_sort, "none"))
> - pf_compar = compar_collate;
> -
> - if (pf_compar != NULL)
> - fsdir_sort_sub(p_fsdir->p_node, pf_compar);
> - }
> + fsdir_sort(p_node->pp_children[i]);
> }
>
> /**
> @@ -636,7 +633,6 @@ void access_fsdir_init(struct access_fsdir *p_fsdir,
> p_fsdir->p_node = p_node;
> p_fsdir->b_show_hiddenfiles = var_InheritBool(p_access,
> "show-hiddenfiles");
> p_fsdir->psz_ignored_exts = var_InheritString(p_access,
> "ignore-filetypes");
> - p_fsdir->psz_sort = var_InheritString(p_access, "directory-sort");
> bool b_autodetect = var_InheritBool(p_access,
> "sub-autodetect-file");
> p_fsdir->i_sub_autodetect_fuzzy = !b_autodetect ? 0 :
> var_InheritInteger(p_access, "sub-autodetect-fuzzy");
> @@ -648,10 +644,9 @@ void access_fsdir_finish(struct access_fsdir
> *p_fsdir, bool b_success)
> if (b_success)
> {
> fsdir_attach_slaves(p_fsdir);
> - fsdir_sort(p_fsdir);
> + fsdir_sort(p_fsdir->p_node);
> }
> free(p_fsdir->psz_ignored_exts);
> - free(p_fsdir->psz_sort);
>
> /* Remove unmatched slaves */
> for (unsigned int i = 0; i < p_fsdir->i_slaves; i++)
> --
> 2.10.2
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
--
Jean-Baptiste Kempf - President
+33 672 704 734
More information about the vlc-devel
mailing list