Fix over-eager file matching in skip files and --file option

[furtib]

Why:
In the current implementation of the skipfiles, every file declared will match any other file that contains the declared file name at the beginning of its name. An example would be: simple.c and simple.cpp. Specifying simple.c with the --flag option would result in a regex: /path/to/simple.c.*

What:
Removed adding '*' to the end of any regex given through --file or the skipfile.

Notes:
According to the documentation, this change is breaking, but I could see some users declaring directories in their skip file without placing a '*' at the end.

Fixes: #4664

[barnabasdomozi]

Good find, I think this should be the default behavior that we don't include an extra * at the end of the regex pattern.

However, as you mentioned, this change could easily break users' skipfiles, so this change should be clearly documented in the upcoming release notes.

[Szelethus]

However, as you mentioned, this change could easily break users' skipfiles, so this change should be clearly documented in the upcoming release notes.

I'm pretty anxious about this one. How widely is the trailing * known? Wouldn't it be friendly to emit a warning for files that would have been skipped with the trailing * but no longer is?

[furtib]

I have added a clause to add the star to entries that are evidently folders. (filenames ending with '/')
Users will use the '/' to point to folders even on Windows.
With this, I do respect that no filename may end with the '/' character.
Filenames can't contain the '/' character on Windows.
On the Linux side of things, I have been unable to create such a file with: touch, vim, vscode, thunar. Also I found this on a forum
@barnabasdomozi, what are your thoughts?

[barnabasdomozi]

I have added a clause to add the star to entries that are evidently folders. (filenames ending with '/') Users will use the '/' to point to folders even on Windows. With this, I do respect that no filename may end with the '/' character. Filenames can't contain the '/' character on Windows. On the Linux side of things, I have been unable to create such a file with: touch, vim, vscode, thunar. Also I found this on a forum @barnabasdomozi, what are your thoughts?

What if the user specified a directory without a / character at the end?
E.g.

-/workspace/myproject/build

[furtib]

I cannot be sure if the path you gave is for a file named build or for a folder, so I do not handle that case.

During a conversation with @HoBoIs, I got the suggestion to, instead of adding a * after a /, we could add an optional /.* to the end of our regex. This would look something like this: (?:\/.*)?. See an explanation for it here

This way, this change is NOT breaking anymore!

This would solve the issue of simple.c and simple.cc while also preserving the workings for a directory path defined like: /workspace/myproject/build

The only consideration to be taken now is: what if the user placed /* at the end?
I suspect that this would not cause an issue because /*/* is a subset of /*

Currently there is a slight issue with the //Z fnmatch.translate places to the end of the user defined regex, but I hope to iron out these issues.

[furtib]

I have fixed the regex; the issue was fnmatch placing an end of line/end of input. I have moved this to the end of the appended regex.

I also added a comment explaining what the regex does, so in the future, it will be easier to decipher.

We have been thinking about the windows side of things too. Does the paths in compile_commands.json contain '' or '/' on Windows?

[barnabasdomozi]

Great work!
See my comment about a minor issue.

We should also investigate which slash type is used on Windows.

[barnabasdomozi]

If fnmatch.translate() function's implementation changes, e.g. it no longer puts \Z in the end, then [:-2] breaks the regex. So instead of [:-2] consider using rstrip()

Also, is there a reason we are using \Z in the end of the regex and not $?

[furtib]

I wrote a simple check to see if \Z is there, before removing it.
rstrip(r"\Z") would have stripped away any path ending in z.
e.g: "abcz\Z" -> "abc"

[furtib]

I have added a check to the fnmatch.translate output, so as not to depend so much on its implementation.
I also tried to support Windows systems. os.path.normalize("/") will return "\\" (an escaped backslash). This way, we will check for an optional \\.* on Windows and /.* on linux.

[barnabasdomozi]

LGTM