Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doesn't detect files without extensions #7

Closed
thebyrd opened this issue May 7, 2015 · 7 comments
Closed

Doesn't detect files without extensions #7

thebyrd opened this issue May 7, 2015 · 7 comments

Comments

@thebyrd
Copy link

thebyrd commented May 7, 2015

You can reproduce this issue by putting a Makefile or Dockerfile in a directory and ls | fpp. It will work with other files in the directory that have an extension.

@sandyarmstrong
Copy link

Yeah, this is pretty annoying. I have the following little patch applied but it can cause some false positives when piping git status output, for example:

diff --git a/src/parse.py b/src/parse.py
index 8ee9175..1e2283e 100644
--- a/src/parse.py
+++ b/src/parse.py
@@ -22,10 +22,10 @@ import logger
 REPOS = ['www']

 MASTER_REGEX = re.compile(
-    '(\/?([a-z.A-Z0-9\-_]+\/)+[a-zA-Z0-9\-_.]+\.[a-zA-Z0-9]{1,10})[:-]{0,1}(\d+)?')
+    '(\/?([a-z.A-Z0-9\-_]+\/)+[a-zA-Z0-9\-_.]+)[:-]{0,1}(\d+)?')
 OTHER_BGS_RESULT_REGEX = re.compile(
     '(\/?([a-z.A-Z0-9\-_]+\/)+[a-zA-Z0-9_.]{3,})[:-]{0,1}(\d+)')
-JUST_FILE = re.compile('([a-zA-Z0-9\-_]+\.[a-zA-Z]{1,10})\s+')
+JUST_FILE = re.compile('([a-zA-Z0-9\-_.]+)\s+')
 FILE_NO_PERIODS = re.compile(
     '([a-zA-Z0-9\-_\/]{1,}\/[a-zA-Z0-9\-_]{1,})(\s|$|:)+')

Between this bug and #18, the released fpp is unusable on most of my repositories. But that diff makes it work well enough.

@mrjbq7
Copy link

mrjbq7 commented May 7, 2015

Why not just use something like os.path.isfile(pathname) to detect files?

@sandyarmstrong
Copy link

@mrjbq7 on Hacker News, the devs said it's because users at Facebook keep their code mounted on network drives, and that would be slow. But they are open to adding it as an option.

@mrjbq7
Copy link

mrjbq7 commented May 7, 2015

That makes sense - however, half of my files seem not to have extensions and so this doesn't work very well. I also, don't see the need to differentiate between files and directories, since opening directories could be a valid use-case, too.

@pedropedruzzi
Copy link
Contributor

After #50, this issue is actualy about dot files and with no period in the CWD.
It seems that those are not detected by design since it's more likely that they are not paths in the input. For the CWD I don't believe checking path existence would be a problem with network drive. Let's patch that.

@joni2back
Copy link

It makes sense, could be very hard to parse filenames without dots, taking into account that could be combined with another texts 😞
So, as @mrjbq7 says, os.path.isfile could be the best option although would be slow

@pcottle
Copy link
Contributor

pcottle commented May 12, 2015

Yeah lets move this into #83. We can likely put the filesystem logic as an experimental flag which should mitigate any performance issues we might encounter. I do think it's the right future of the project though, since then we can match on all sorts of things and eliminate a ton of false positives

@pcottle pcottle closed this as completed May 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants