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

Heuristic to disambiguate GAP and GDScript #4378

Merged
merged 5 commits into from Jan 31, 2019
Merged

Heuristic to disambiguate GAP and GDScript #4378

merged 5 commits into from Jan 31, 2019

Conversation

anantn
Copy link
Contributor

@anantn anantn commented Jan 12, 2019

GDScript files (.gd) are often misclassified as GAP code on Github.

Description

This rule updates classification of .gd files as either GAP or GDScript. Currently, several GDScript files are misclassified as GAP on Github (example search: https://github.com/search?q=language%3AGAP+game&type=Repositories)

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.

GDScript files (.gd) are often misclassified as GAP code on Github.
Copy link
Contributor

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

lib/linguist/heuristics.yml Outdated Show resolved Hide resolved
@pchaigno
Copy link
Contributor

@anantn Could you add a test for these changes in test_heuristics.rb?

Copy link
Contributor

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Pinging @fingolfin, who added support for both GAP and GDScript in Linguist (#1046 and #1529), in case they want to review.

Co-Authored-By: anantn <anant@kix.in>
@anantn
Copy link
Contributor Author

anantn commented Jan 24, 2019

If the heuristic doesn't match I think we should still classify the script as GAP since we would try other strategies as @pchaigno mentioned. But no harm making the heuristic more robust!

Thanks to @fingolfin and @pchaigno for your help with this patch 👍

@anantn
Copy link
Contributor Author

anantn commented Jan 29, 2019

@pchaigno Could we land this now? 😄

@pchaigno pchaigno requested a review from lildude January 29, 2019 15:25
Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@lildude lildude merged commit 6867f79 into github-linguist:master Jan 31, 2019
@anantn anantn deleted the patch-1 branch February 5, 2019 03:42
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

4 participants