Awesome, thanks!
--
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#issuecomment-444275376
Yep, thanks :)
--
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#issuecomment-444274695
Merged #1445 into master.
--
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#event-2005586108
@b4n are we good to go?
--
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#issuecomment-444258483
@kugel- See https://github.com/kugel-/geany/pull/6
--
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#issuecomment-443498042
I rebased and imply removed the API bump that was causing the conflict. I hope
that's what you wanted?
--
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#issuecomment-443462293
@kugel- pushed 1 commit.
c599a1e fix inverted logic in assertion
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/geany/geany/pull/1445/files/dbafede20cd3622867b2ec4649b16ee5947f6083..c599a1e7eab887ff2a792fd15eddb8a51d4570ed
@kugel- just in case you didn't see my review here like on the other PR.
--
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#issuecomment-443449989
b4n commented on this pull request.
> + * @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)
+ */
+GEANY_API_SYMBOL
+gchar **utils_strv_shorten_file_list(gchar
```
(geany:25173): Geany-CRITICAL **: utils_strv_shorten_file_list: assertion 'num
!= 0 && file_names == NULL' failed
Erreur de segmentation
```
oops! Just tried to go to a location
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on
b4n requested changes on this pull request.
Still annoying :)
Otherwise looks good.
You also should resolve the conflict, but apart from that I think we're good do
go.
> @@ -2031,6 +2031,208 @@ gchar **utils_strv_join(gchar **first, gchar **second)
return strv;
}
+/* * Returns the
@kugel- pushed 1 commit.
0789cf2 Changes for review comments
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/geany/geany/pull/1445/files/16540421553139eb248e241118874c527737210f..0789cf2c9cafdaa381e47eb2e025c012ae0d596a
kugel- commented on this pull request.
> +
+ /* 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
kugel- commented on this pull request.
> + 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
codebrainz commented on this pull request.
> + 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
+ *
b4n commented on this pull request.
> + 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
kugel- commented on this pull request.
> + 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
b4n commented on this pull request.
> @@ -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).
+
b4n commented on this pull request.
> + 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
kugel- commented on this pull request.
> +
+ /* 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
kugel- commented on this pull request.
> @@ -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).
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
> Fixing this isn't trivial
That's why I suggested in an earlier comment that the fact
`utils_strv_find_lcs()` looks generic is cute, but in practice this function
does *not* do what `utils_strv_shorten_file_list()` needs.
> and without proper unit tests there is a risk to break previously
@kugel- there still are problems shortening some paths, like e.g.
*/src/a/app-1.2.3/src/lib/module/source.c* and
*/src/b/app-2.2.3/src/module/source.c*:
path | current result | my expectation
--||--
`/src/a/app-1.2.3/src/lib/module/source.c` |
> (ellipsizing is clearer when the 3 dots are encosed by words. I've never seen
> a ellipsizing that cuts of the entire front).
I agree with @kugel- here, and would have expected what @b4n showed as "current
result" anyway.
--
You are receiving this because you are subscribed to this thread.
@b4n friendly ping :-)
--
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#issuecomment-378340497
> Looking at the table below, is this expected, or is it a bug?
it's debatable. From the plain algorithm, your expectation is right and the
result is buggy. But one can argue that the actual result may be less confusing
to the user (ellipsizing is clearer when the 3 dots are encosed by words.
Also, your code doesn't seem to support Windows path separators. I'm not 100%
sure we need it in Geany here, but it would probably be good to have support
for it if it's supposed to be a generic utility.
--
You are receiving this because you are subscribed to this thread.
Reply to this email
Looking at the table below, is this expected, or is it a bug?
path | current result | what I expected
| |
`/aaa/bbb/cc/c/d` | `cc/.../d` | `.../d`
`/aaa/bbb/xxx/yyy/cc/c/d` | `xxx/yyy/cc/.../d` | `xxx/yyy/.../d`
--
You are receiving this because
I hope the code is clearer now and the bug you mentioned should be fixed
--
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#issuecomment-373435785
@kugel- pushed 1 commit.
1654042 Refactoring and review comments
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/geany/geany/pull/1445/files/47f43928054ba848d2e9b2c00daa1a7211bbf44f..16540421553139eb248e241118874c527737210f
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);
+
+
Well, yeah I don't like inefficient code when it can easily be improved in that
aspect. OTOH, I prefer easy to read/maintain code.
And I'll have to check again, maybe I was tired, but I found your code oddly
hard to follow, and tricky to fix the issues I found. At the time I found that
@b4n to be honest, I prefer my version. I find it easier to grasp because it's
split into 3 strv-related utility functions that have well defined and
separated functionality (the functions should be useful on their own). I don't
so how your code is really any shorter (and neither is worryingly
34 matches
Mail list logo
