[Github-comments] [geany/geany] Improve goto-symbols popup (#1445)

[Colomban Wendling]

b4n commented on this pull request.

small stuff I noticed first time I went over, if it can be useful.
Not a complete review.

> +{
+	gchar *prefix, **ptr;
+
+	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++)
+	{
+		for (gint j = 1; j < num; j++)
+		{
+			gchar *s = strv[j];

`const` would make sense, the string should not be modified

> +
+	if (num == 0)
+		num = g_strv_length(strv);
+
+	/* sub is the working area where substrings from first are copied to */
+	sub = g_malloc(len+1);
+	lcs = g_strdup("");
+	foreach_str(_sub, first)
+	{
+		gsize chars_left = len - (_sub - first);
+		/* No point in continuing if the remainder is too short */
+		if (max > chars_left)
+			break;
+		for (n_chars = 1; n_chars <= chars_left; n_chars++)
+		{
+			/* strlcpy() ftw! */

[`g_strlcpy()`](https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html#g-strlcpy) is there for you if you like.

> +		if (prefix[i] == '\0')
+			break;
+	}
+	return prefix;
+}
+
+/* * Returns the longest common substring 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.
+ *
+ * @return The common prefix that is part of all strings.
+ */
+gchar *utils_strv_find_lcs(gchar **strv, size_t num)

this should be static

> +/** 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)

do we need it in the API right away?  I mean, why not, but if there's no real use (and although it seems nice, it is fairly specific still) I'd rather wait a little.

> +GEANY_API_SYMBOL
+gchar **utils_strv_shorten_file_list(gchar **file_names, size_t num)
+{
+	gint i, j;
+	gchar *prefix, *substring, *name, *sep, **s;
+	TMTag *tmtag;
+	gchar **names;
+	gsize len;
+
+	/* 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_new(gchar *, num + 1);
+	names[num] = 0;

should be `NULL`

-- 
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-61653071
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20180310/b81f52ee/attachment.html>