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

Not all Hack files are detected correctly #4346

Closed
4 tasks done
azjezz opened this issue Dec 8, 2018 · 7 comments · Fixed by #4347
Closed
4 tasks done

Not all Hack files are detected correctly #4346

azjezz opened this issue Dec 8, 2018 · 7 comments · Fixed by #4347

Comments

@azjezz
Copy link
Contributor

azjezz commented Dec 8, 2018

Preliminary Steps

Problem Description

URL of the affected repository:

https://github.com/azjezz/waffle

Last modified on:

2018-12-08

Expected language:

Hack ( 100% )

Detected language:

Hack ( 61.5% )
C++ ( 38.5% )

@pchaigno
Copy link
Contributor

pchaigno commented Dec 8, 2018

Thanks for the report @azjezz!

We could probably improve results here with a partial heuristic rule based on <?hh for Hack. I'll look into it.

You checked the considered implementing an override checkbox. Why did you choose not to implement such an override in the end?

@azjezz
Copy link
Contributor Author

azjezz commented Dec 8, 2018

i could do that, but i guess this can be fix as you said based on <?hh.
a long term solution would be better than using override.

@azjezz
Copy link
Contributor Author

azjezz commented Dec 8, 2018

@pchaigno
Copy link
Contributor

pchaigno commented Dec 9, 2018

i could do that, but i guess this can be fix as you said based on <?hh.
a long term solution would be better than using override.

My new heuristic rule is unlikely to handle all cases, unless I consider all .hh files without <?hh as C++.

there's also the lack of color support in hack.
e.g : https://github.com/azjezz/waffle/blob/f746ca07c39fba31b7e921b8f33c54c0b02cacec/src/Waffle/Http/Message/Response/TextResponse.hh

It looks like that file is detected as C++, so not an issue with the Hack highlighting.

@azjezz
Copy link
Contributor Author

azjezz commented Dec 9, 2018

@pchaigno i talked with on of hack core team member before about <?hh and he said that when @hhvm drops support for php - there's chance that <?hh would be gone too ( unlike php, in hack you don't close <?hh and you can't mix hack with html ) so that's another problem.

@pchaigno
Copy link
Contributor

pchaigno commented Dec 9, 2018

Okay. Thanks for the information.

I've made a pull request (cf #4347) to add a new heuristic rule. It should help detect Hack .hh files properly, but won't address misclassifications of C++ .hh files.

@pchaigno
Copy link
Contributor

The fix will take effect with the next release of Linguist, which usually happens once a month.

@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 a pull request may close this issue.

2 participants