Bug 416376

Thanks

Git commit 26f4a2218f52f790d4de087216098619a753164e by Ahmad Samir.
Committed on 22/03/2020 at 18:05.
Pushed by hindenburg into branch 'master'.

[FileFilter] Create HotSpot's for files in child dirs too

When creating HotSpot's for local files, create them for files in the
current dir and for files in sub-directories too.

Re-format the code used to build the regex pattern for more readability
(easier for seeing what the regex pattern will look like, I hope).

Use a static QRegularExpression object, so as to only construct it once,
the pattern doesn't change and this should help with performance. Also
make createFileRegex() not static.

Don't convert QList<QString> to QSet<QString>, without actual benchmarking
it could be that QList is actually efficient here.

Use const where appropriate.

M  +47   -37   src/Filter.cpp
M  +1    -1    src/Filter.h

https://invent.kde.org/kde/konsole/commit/26f4a2218f52f790d4de087216098619a753164e

In the case that a filename matched one of the things in the current directory, you still need to check that it actually exists, though, right?

With this patch:
- we get the list of files in the current dir, these are underlined (previous behaviour)
- we get the list of sub-dirs and underline all files that have a path that begins with one of the sub-dir names, but we don't check that any files in the sub-dirs actually exist.

It's not ideal, but it's a trade-off, listing all the sub-dirs would be costly performance-wise (and how much depth? 2-3 folders deep or more?), and the use cases of e.g. using 'ls', or files in the output of 'grep' would work.

Why not just check if the file exists once you encounter it? There's no need to build a recursive list in advance.

(In reply to FeepingCreature from comment #5)
> Why not just check if the file exists once you encounter it?
Performance: https://cgit.kde.org/konsole.git/commit/?id=2322c5605ea267e766f8795731263738f9e60dcc

And patches to improve file path matchinh are welcome of course. :)

by the way, one of the mentioned commits totally brake my experience with konsole ;)

I had a directory named `t` in my homedir ad some crap inside it.

Also, I'm using weechat, poezio, and sometimes mutt and lynx.

So, every time I got there (in any of them) a text that contains two or more "don't" words, all the text (even f it takes the half of visible screen) gots underlined on mouse over :D

And clicking it makes an error like "there is no file /home/mva/t bla bla bla bla bla bla foo bar moo meow ...".


So, I think those fixes needs a fix themselves ;)

Well, I think, if it at would least check for "/" after "t", that would be more correct behaviour and I would not encounter that bug.


// by the way, it would also be nice to react on relative ("./file.moo") and global ("/home/user/file.moo") paths ;)

btw, I just found that filter matches (and so konsole underlines) **ALL** the strings **starting with** known file name (not only between ''s).

Preconditions:
I have "dev" and "1" directories in home dir. Also there is some files named "2", "4", "5" (notice, no "3").

Situation:
I opened tmux, changed dir to one of my git repos (here konsole still thinks I'm in home dir, since tmux was opened there). Let it be the Gentoo portage tree (package build scripts, sorted by dirs). Then I pulled changes, and looking git log.

Here, all the path like "dev-python", "dev-lua", "dev-libs" and whatsoever - underlines (and files to open since konsole tries to open it in relation to home dir).

Also things like "100%", "247483" ("... files"), and so on (starting with 1,2,4,5) - underlines too. And yes, "3249" - is not underlined.


So, I think, it definitelly should not work that way (match all that starts with substring that equals to known file).
At least, I think, for dirs it definitelly needs to check for "/" after the name.

As for files, I think it should stop the match on last character of the filename (so, if suspected string is longer, it will not match).

Although, there is a neighbour bug around, that requests filter to support line/columns specifications (like in compiler output: `my/source/file.c:21:37`) to and being able to pass that line/col numbers to the custom command.
I think, that should also be taken in mind when fixing