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-strv-length).
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 :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/1445#pullrequestreview-175739285
