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

fix php disambiguation regex #4185

Merged
merged 1 commit into from
Jul 21, 2018
Merged

Conversation

smola
Copy link
Contributor

@smola smola commented Jul 9, 2018

Description

Because of an error in the regular expression, disambiguation rule for the .php
matched any non-empty file as PHP, except for Hack files.

Note that this means that now files with .php but containing no <? will be not classified as PHP unless other strategy kicks in. If that is not the desired behaviour, we should just remove the PHP regex and make it explicit that it is a catch-all fallback.

Checklist:

  • I am fixing a misclassified language
    • I have included a new sample for the misclassified language:
      • Sample source(s):
        • [URL to each sample source, if applicable]
      • Sample license(s):
    • I have included a change to the heuristics to distinguish my language from others using the same extension.

Because of an error in the regular expression, disambiguation rule for the .php
matched any non-empty file as PHP, except for Hack files.
@pchaigno
Copy link
Contributor

pchaigno commented Jul 9, 2018

Nice catch! Thanks a lot @smola!

Do you have a sample file we could use as a test for this error?

@smola
Copy link
Contributor Author

smola commented Jul 9, 2018

Not really. A real world example would involve a file with .php extension that is not PHP (or Hack) at all.

@pchaigno
Copy link
Contributor

pchaigno commented Jul 9, 2018

Hm, it's probably not worth it to add a new fixture file (files used for tests only, as opposed to sample files) then.

LGTM!

@pchaigno pchaigno requested a review from lildude July 21, 2018 09:28
@lildude lildude merged commit 4c1c652 into github-linguist:master Jul 21, 2018
@smola smola deleted the fix-hack branch July 24, 2018 08:25
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants