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

Read only the first few bytes of a file to check for #! #1026

Merged
merged 1 commit into from Aug 16, 2012

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jul 30, 2012

Saves trying to load a potentially very large file into memory and causing all sorts of problems

Tidier pull request than the first.
Apologies

Saves trying to load a potentially very large file into memory and causing all sorts of problems
@skurfer
Copy link
Member

@skurfer skurfer commented Aug 2, 2012

Now that you’ve made this more efficient, I wonder if we should just use this technique to assign a particular type like QSExecutableType as file objects are created. (Or maybe just set a flag in its metadata.) That way, we wouldn’t need code doing it both here and in the plug-ins.

@skurfer
Copy link
Member

@skurfer skurfer commented Aug 2, 2012

That assumes we would only open and examine the first 5 bytes for files with no extension. If this would run on all files, it might be too slow.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Aug 9, 2012

Sounds like a good idea in theory, but I think setting the flag wouldn't help too much

  • If we only do it for extension-less files, we'd still need to check files with extensions to check if they're executable (duplicate code in the plugin and core, although a lot less duplicate code)
  • As you say, checking and assigning all files would probably be a waste
  • Setting a type for all files might be a bit too wasteful as well. In reality, how many files does the average user every want to try and execute?

skurfer added a commit that referenced this issue Aug 16, 2012
Read only the first few bytes of a file to check for #!
@skurfer skurfer merged commit 927d06d into quicksilver:master Aug 16, 2012
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

Successfully merging this pull request may close these issues.

None yet

2 participants