Opened 11 years ago

Closed 11 years ago

Last modified 10 months ago

#4014 closed defect (fixed)

ALL: */? match path separator in FSDir::listMatchingMembers

Reported by: wjp Owned by: lordhoto
Priority: normal Component: --Other--
Keywords: Cc:
Game:

Description

The pattern "*.pak" matches for examples the member "fre/intro1.pak" in FSDirectory since it uses a general string-matching function rather than a path-matching function. This is (to me) very counter-intuitive (mainly since it's different than how POSIX globbing works).

(This bug report is mainly a reminder to fix this after an IRC discussion between LordHoto, peres and myself.)

Ticket imported from: #2309974. Ticket imported from: bugs/4014.

Attachments (2)

path_match_v1.patch (1.9 KB ) - added by lordhoto 11 years ago.
Patch against r35113.
patch_match_v2.patch (1.4 KB ) - added by lordhoto 11 years ago.
Patch against r35157.

Download all attachments as: .zip

Change History (12)

comment:1 by lordhoto, 11 years ago

I just added a very simple patch, I guess it can be done a lot easier / nicer though :-).

by lordhoto, 11 years ago

Attachment: path_match_v1.patch added

Patch against r35113.

comment:2 by fingolfin, 11 years ago

Interesting. *I* would find it highly counter-intuitive to *not* match "fre/intro1.pak"...

Can you elaborate why you would want this behavior, and not the other?

Resp.: If you only want to match files in a specific dir, then why don't you use FSDirs for each of the separate dirs to start with?

comment:3 by wjp, 11 years ago

Maybe my example didn't illustrate clearly:
"a*h.pak" would for example match the file "abcd/efgh.pak". I mainly find it counter-intuitive because this is very different than POSIX or DOS/Windows wildcard-expansion where the '*' doesn't match a directory separator.

(Also: FSDirectory defaults to only matching files in a specific dir: If there's a file "fre/intro1.pak", then hasFile("intro1.pak") will fail, but hasFile("fre/intro1.pak") will succeed.)

comment:4 by fingolfin, 11 years ago

I see your point here. I am still a bit uncomfortable with this, but OK...

The attached patch is IMO not OK; tokenizing the full string to do this seems overkill, a slight refinement of the matchString state machine (making "*" not match slashes) could do the same job...

FSNode::lookupFile

comment:5 by lordhoto, 11 years ago

I added a patch which uses a modified matchString implementation. I guess that should be fine for that task.
It assures that both '*' and '?' do not match any path seperators ('/').

by lordhoto, 11 years ago

Attachment: patch_match_v2.patch added

Patch against r35157.

comment:6 by fingolfin, 11 years ago

Owner: set to lordhoto
Summary: */? match path separator in FSDirectory::listMatchingMembersALL: */? match path separator in FSDir::listMatchingMembers

comment:7 by fingolfin, 11 years ago

Great, go ahead an commit this. (Might want to add a unit test for the new function, too. That would require exposing it to the public, though).

comment:8 by lordhoto, 11 years ago

Ok I committed a slightly changed version of my patch. Now FSDirectory::listMembers should be working fine again.

comment:9 by lordhoto, 11 years ago

Resolution: fixed
Status: newclosed

comment:10 by digitall, 10 months ago

Component: --Other--
Note: See TracTickets for help on using tickets.