Branch: refs/heads/master Author: Thomas Martitz kugel@rockbox.org Committer: Thomas Martitz kugel@rockbox.org Date: Fri, 20 Sep 2019 21:01:03 UTC Commit: 30a486d62462ad2bf34ac7af48547053f7167441 https://github.com/geany/geany/commit/30a486d62462ad2bf34ac7af48547053f71674...
Log Message: ----------- utils: fix suboptimal elipsis substitution by utils_strv_shorten_file_list()
Because utils_strv_find_lcs() didn't consisider path component boundaries it have have found substrings that are longest in itself but not so ideal when utils_strv_shorten_file_list() applied path boundaries.
utils_strv_find_lcs() now can optionally restrict the substring between delimiters (i.e. dir separators). In that mode it will find the longest substring that is also sorrounded by the delimiters (there may be more delimiters inside the string).
The unit test that demonstrated the deficient is fixed since now the expected substitution takes place.
Modified Paths: -------------- src/utils.c src/utils.h tests/test_utils.c
Modified: src/utils.c 14 lines changed, 12 insertions(+), 2 deletions(-) =================================================================== @@ -2086,7 +2086,7 @@ gchar *utils_strv_find_common_prefix(gchar **strv, gssize strv_len) * @return The common prefix that is part of all strings. */ GEANY_EXPORT_SYMBOL -gchar *utils_strv_find_lcs(gchar **strv, gssize strv_len) +gchar *utils_strv_find_lcs(gchar **strv, gssize strv_len, const gchar *delim) { gchar *first, *_sub, *sub; gsize num; @@ -2112,8 +2112,18 @@ gchar *utils_strv_find_lcs(gchar **strv, gssize strv_len) /* No point in continuing if the remainder is too short */ if (max > chars_left) break; + /* If delimiters are given, we only need to compare substrings which start and + * end with one of them, so skip any non-delim chars at front ... */ + if (NZV(delim) && (strchr(delim, _sub[0]) == NULL)) + continue; for (n_chars = 1; n_chars <= chars_left; n_chars++) { + if (NZV(delim)) + { /* ... and advance to the next delim char at the end, if any */ + if (!_sub[n_chars] || strchr(delim, _sub[n_chars]) == NULL) + continue; + n_chars += 1; + } g_strlcpy(sub, _sub, n_chars+1); found = 1; for (gsize i = 1; i < num; i++) @@ -2199,7 +2209,7 @@ gchar **utils_strv_shorten_file_list(gchar **file_names, gssize file_names_len) }
lcs_len = 0; - substring = (i == num) ? utils_strv_find_lcs(names, num) : NULL; + substring = (i == num) ? utils_strv_find_lcs(names, num, G_DIR_SEPARATOR_S"/") : NULL; if (NZV(substring)) { /* Strip leading component. */
Modified: src/utils.h 2 lines changed, 1 insertions(+), 1 deletions(-) =================================================================== @@ -302,7 +302,7 @@ gchar **utils_strv_join(gchar **first, gchar **second) G_GNUC_WARN_UNUSED_RESULT
gchar *utils_strv_find_common_prefix(gchar **strv, gssize strv_len) G_GNUC_WARN_UNUSED_RESULT;
-gchar *utils_strv_find_lcs(gchar **strv, gssize strv_len) G_GNUC_WARN_UNUSED_RESULT; +gchar *utils_strv_find_lcs(gchar **strv, gssize strv_len, const gchar *delim) G_GNUC_WARN_UNUSED_RESULT;
gchar **utils_strv_shorten_file_list(gchar **file_names, gssize file_names_len);
Modified: tests/test_utils.c 116 lines changed, 85 insertions(+), 31 deletions(-) =================================================================== @@ -106,51 +106,86 @@ static void test_utils_strv_find_common_prefix(void) g_strfreev(data); }
+#define DIR_SEP "\/" void test_utils_strv_find_lcs(void) { gchar **data, *s;
- s = utils_strv_find_lcs(NULL, 0); + s = utils_strv_find_lcs(NULL, 0, ""); g_assert_null(s);
data = utils_strv_new("", NULL); - s = utils_strv_find_lcs(data, -1); + s = utils_strv_find_lcs(data, -1, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, ""); g_free(s); g_strfreev(data);
data = utils_strv_new("1", "2", "3", NULL); - s = utils_strv_find_lcs(data, -1); + s = utils_strv_find_lcs(data, -1, ""); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, ""); + g_free(s); + s = utils_strv_find_lcs(data, -1, DIR_SEP); g_assert_nonnull(s); g_assert_cmpstr(s, ==, ""); g_free(s); g_strfreev(data);
data = utils_strv_new("abc", "amn", "axy", NULL); - s = utils_strv_find_lcs(data, -1); + s = utils_strv_find_lcs(data, -1, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, "a"); g_free(s); + s = utils_strv_find_lcs(data, -1, DIR_SEP); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, ""); + g_free(s); + g_strfreev(data); + + data = utils_strv_new("bca", "mna", "xya", NULL); + s = utils_strv_find_lcs(data, -1, ""); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, "a"); + g_free(s); + s = utils_strv_find_lcs(data, -1, DIR_SEP); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, ""); + g_free(s); g_strfreev(data);
data = utils_strv_new("abc", "", "axy", NULL); - s = utils_strv_find_lcs(data, -1); + s = utils_strv_find_lcs(data, -1, ""); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, ""); + g_free(s); + s = utils_strv_find_lcs(data, -1, DIR_SEP); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, ""); + g_free(s); + g_strfreev(data); + + data = utils_strv_new("a123b", "b123c", "c123d", NULL); + s = utils_strv_find_lcs(data, -1, ""); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, "123"); + g_free(s); + s = utils_strv_find_lcs(data, -1, DIR_SEP); g_assert_nonnull(s); g_assert_cmpstr(s, ==, ""); g_free(s); g_strfreev(data);
data = utils_strv_new("22", "23", "33", NULL); - s = utils_strv_find_lcs(data, 1); + s = utils_strv_find_lcs(data, 1, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, "22"); g_free(s); - s = utils_strv_find_lcs(data, 2); + s = utils_strv_find_lcs(data, 2, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, "2"); g_free(s); - s = utils_strv_find_lcs(data, 3); + s = utils_strv_find_lcs(data, 3, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, ""); g_free(s); @@ -163,48 +198,77 @@ void test_utils_strv_find_lcs(void) "/home/user/src/geany/src/main.c", "/home/user/src/geany-plugins/addons/src/addons.c", NULL); - s = utils_strv_find_lcs(data, 4); + s = utils_strv_find_lcs(data, 4, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, "/home/user/src/geany/src/s"); g_free(s); - s = utils_strv_find_lcs(data, 5); + s = utils_strv_find_lcs(data, 4, DIR_SEP); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, "/home/user/src/geany/src/"); + g_free(s); + s = utils_strv_find_lcs(data, 5, ""); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, "/home/user/src/geany/src/"); + g_free(s); + s = utils_strv_find_lcs(data, 5, DIR_SEP); g_assert_nonnull(s); g_assert_cmpstr(s, ==, "/home/user/src/geany/src/"); g_free(s); - s = utils_strv_find_lcs(data, -1); + s = utils_strv_find_lcs(data, -1, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, "/home/user/src/geany"); g_free(s); + s = utils_strv_find_lcs(data, -1, DIR_SEP); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, "/home/user/src/"); + g_free(s); g_strfreev(data);
data = utils_strv_new("/src/a/app-1.2.3/src/lib/module/source.c", "/src/b/app-2.2.3/src/module/source.c", NULL); - s = utils_strv_find_lcs(data, -1); + s = utils_strv_find_lcs(data, -1, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, "/module/source.c"); g_free(s); + s = utils_strv_find_lcs(data, -1, DIR_SEP); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, "/module/"); + g_free(s); + g_strfreev(data); + + data = utils_strv_new("/src/a/app-1.2.3/src/lib/module\source.c", + "/src/b/app-2.2.3/src/module\source.c", NULL); + s = utils_strv_find_lcs(data, -1, ""); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, "/module\source.c"); + g_free(s); + s = utils_strv_find_lcs(data, -1, DIR_SEP); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, "/module\"); + g_free(s); g_strfreev(data);
data = utils_strv_new("/src/a/app-1.2.3/src/lib/module/", "/src/b/app-2.2.3/src/module/", NULL); - s = utils_strv_find_lcs(data, -1); + s = utils_strv_find_lcs(data, -1, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, ".2.3/src/"); g_free(s); - g_strfreev(data); - - data = utils_strv_new("a123b", "b123c", "c123d", NULL); - s = utils_strv_find_lcs(data, -1); + s = utils_strv_find_lcs(data, -1, DIR_SEP); g_assert_nonnull(s); - g_assert_cmpstr(s, ==, "123"); + g_assert_cmpstr(s, ==, "/module/"); g_free(s); g_strfreev(data);
data = utils_strv_new("/usr/local/bin/geany", "/usr/bin/geany", "/home/user/src/geany/src/geany", NULL); - s = utils_strv_find_lcs(data, -1); + s = utils_strv_find_lcs(data, -1, ""); g_assert_nonnull(s); g_assert_cmpstr(s, ==, "/geany"); g_free(s); + s = utils_strv_find_lcs(data, -1, DIR_SEP); + g_assert_nonnull(s); + g_assert_cmpstr(s, ==, ""); + g_free(s); g_strfreev(data); }
@@ -333,21 +397,11 @@ void test_utils_strv_shorten_file_list(void) g_strfreev(result); g_strfreev(data);
- /* This case shows a weakness. It would be better to elipsize "module". Instead, - * nothing is elipsized as the code determines ".2.3/src/" to be the - * longest common substring. Then it reduces it to directory components, so "src" remains. - * This is shorter than the threshold of 5 chars and consequently not elipsized - * - * Plain utils_strv_find_lcs() actually finds /module/source.c, - * but utils_strv_shorten_file_list() calls it without the file name part, so - * ".2.3/src/" is longer than "/module/". This is illustrated in the test - * test_utils_strv_find_lcs() - */ data = utils_strv_new("/src/a/app-1.2.3/src/lib/module/source.c", "/src/b/app-2.2.3/src/module/source.c", NULL); result = utils_strv_shorten_file_list(data, -1); - expected = utils_strv_new("a/app-1.2.3/src/lib/module/source.c", - "b/app-2.2.3/src/module/source.c", NULL); + expected = utils_strv_new("a/app-1.2.3/src/lib/.../source.c", + "b/app-2.2.3/src/.../source.c", NULL); g_assert_true(strv_eq(result, expected)); g_strfreev(expected); g_strfreev(result);
-------------- This E-Mail was brought to you by github_commit_mail.py (Source: https://github.com/geany/infrastructure).