> 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 working > cases. Yeah, we should probably add tests for this. Fortunately, it's probably one of the few things that should be easy enough to add a test for, because `utils.c` don't usually have dependencies on internal Geany state, which means it can likely be tested by a minimal test program linking libgeany. > I suggest to accept the behavior for now (and merge) and I'll follow up with > a fix + unit tests. But I would hate if this doesn't make it into the release > just because of select (edge) cases because the status quo is just plain > unusable for me. In fact I've been using this patch at work for ages where I > have real-world cases and I never came across issues like this. Fair enough; I must admit I specifically crafted the test to fail, looking at the weakness of the implementation :) This said, note that I have another implementation (see a message above) that doesn't have the issue. It has arguably worse performance, and can probably use some improvement in other area, but I didn't find a bug in its results yet. -- 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-439360404
