b4n requested changes on this pull request.
@@ -1927,15 +1927,23 @@ static void show_goto_popup(GeanyDocument *doc, GPtrArray *tags, gboolean have_b
GdkEventButton *button_event = NULL; TMTag *tmtag; guint i; - + gchar **short_names, **file_names, **p;
`p` is not used anywhere.
@@ -2031,6 +2031,209 @@ gchar **utils_strv_join(gchar **first, gchar **second)
return strv; }
+/* * Returns the common prefix in a list of strings. + * + * The size of the list may be given explicitely, but defaults to @c g_strv_length(strv). + * + * @param strv The list of strings to process. + * @param num The number of strings contained in @a strv. Can be 0 if it's terminated by @c NULL.
GLib APIs generally use `-1` to mean this, and although it has the problem of unsigned here (thus would "bug" on a `G_SIZE_MAX`-long input not `NULL`-terminated) it has the nice property of allowing 0 as a normal value which is often handy because then the `strv` wouldn't be used at all. It's currently private so I don't mind much though.
- @return The common prefix that is part of all strings (maybe empty), or NULL if an empty list
+ * was passed in. + */ +static gchar *utils_strv_find_common_prefix(gchar **strv, gsize num) +{ + gchar *prefix; + + if (!NZV(strv)) + return NULL; + + if (num == 0) + num = g_strv_length(strv); + + prefix = g_strdup(strv[0]); + + for (gint i = 0; prefix[i]; i++)
`guint` or `gsize` here.
- for (gint i = 0; prefix[i]; i++)
+ { + for (gsize j = 1; j < num; j++) + { + gchar *s = strv[j]; + if (s[i] != prefix[i]) + { + /* terminate prefix on first mismatch and return */ + prefix[i] = '\0'; + break; + } + } + if (prefix[i] == '\0') + break; + } + return prefix;
What about avoiding the initial copy and simply do: ```c for (gsize i = 0; strv[0][i]; i++) { for (gsize j = 1; j < num; j++) { if (strv[j][i] != strv[0][i]) { /* return prefix on first mismatch */ return g_strndup(strv[0], i); } } } return g_strdup(strv[0]); ``` ?
+/** Transform file names in a list to be shorter.
+ * + * This function takes a list of file names (porbably with absolute paths), and + * transforms the paths such that they are short but still unique. This is intended + * for dialogs which present the file list to the user, where the base name may result + * in duplicates (showing the full path might be inappropriate). + * + * The algorthm strips the common prefix (e-g. the user's home directory) and + * replaces the longest common substring with an ellipsis ("..."). + * + * @param file_names @array{length=num} The list of strings to process. + * @param num The number of strings contained in @a file_names. Can be 0 if it's terminated by @c NULL. + * @return @transfer{full} A newly-allocated array of transformed paths strings, terminated by + @c NULL. Use @c g_strfreev() to free it. + * + * @since 1.31 (API 232)
OK then
- return lcs;
+} + + +/** Transform file names in a list to be shorter. + * + * This function takes a list of file names (probably with absolute paths), and + * transforms the paths such that they are short but still unique. This is intended + * for dialogs which present the file list to the user, where the base name may result + * in duplicates (showing the full path might be inappropriate). + * + * The algorthm strips the common prefix (e-g. the user's home directory) and + * replaces the longest common substring with an ellipsis ("..."). + * + * @param file_names @array{length=num} The list of strings to process. + * @param num The number of strings contained in @a file_names. Can be 0 if it's terminated by @c NULL.
Same comment for the value meaning "compute it". It's especially problematic here as you annotated `file_names` with `array_length=num`; so this would suggest a caller could do: ```c gsize array_len = 0; gchar **array = g_malloc(sizeof *array * array_len); gchar **short = utils_strv_shorten_file_list(array, array_len); ``` This doesn't seem far-fetched for auto-generated code like Vala, and AFAIK there's nothing telling the GIR consumer it's incorrect. The reason it's a big problem is that [`g_strv_length()` does not allow a `NULL` parameter](https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html#g...). But I could imagine other reasons why it wouldn't be convenient, like for processing only a part of an already-existing strv: the caller would have to carefully special-case processing only 0 elements not to get unexpected behavior. Same, it can be handy that 0 leads to no access of the array pointer at all, because it's a fairly innocent case of not initializing any members, as the caller can expect there will be no access -- and then again, have to special-case 0.
IMO, it'd be a lot easier to use `-1` as the special value like many other functions around. I'd be fine for it to sill be a `gsize` and then asking for a `(gsize) -1` value for the default.
+ /* The return value shall have exactly the same size as the input. If the input is a + * GStrv (last element is NULL), the output will follow suit. */ + if (!num) + num = g_strv_length(file_names); + /* Always include a terminating NULL, enables easy freeing with g_strfreev() */ + names = g_new0(gchar *, num + 1); + + prefix = utils_strv_find_common_prefix(file_names, num); + /* First: determine the common prefix, that will be stripped. + * Don't strip single-letter prefixes, such as '/' */ + prefix_len = 0; + if (NZV(prefix) && prefix[1]) + { + /* Only strip directory components, include trailing '/' */ + start = strrchr(prefix, G_DIR_SEPARATOR);
As mentioned (not clearly, I agree) in a comment, IIUC this doesn't provide full Windows support, because IIUC Windows allows both `/` and `\` as path separators. I guess we'll most likely get `\`-separated paths, but I'm not sure. Also being an API function suggests it's not specific for a particular subset of paths.
The same comment applies thorough the function.
menu = gtk_menu_new();
+ /* If popup would show multiple files presend a smart file list that allows
@elextr seems to indeed have spotted a small typo :)