Make sure a GMS2 generated file is classified as such #4297

RobQuistNL · 2018-10-16T09:52:23Z

I'm making sure the generated .yy file is being matches as a generated GMS2 file, instead of Yacc.

Checklist:

I am fixing a misclassified language
- I have included a new sample for the misclassified language:
  - Sample source(s):
    - https://github.com/RobQuistNL/DSNet/blob/ad8bc9ffc37b920610f6524a86324f1b6503d3c0/options/main/inherited/options_main.inherited.yy
  - Sample license(s): MIT
- I have included a change to the heuristics to distinguish my language from others using the same extension.

RobQuistNL · 2018-10-16T10:13:52Z

lib/linguist/generated.rb

@@ -588,7 +588,8 @@ def generated_graphql_relay?
    def generated_gamemakerstudio?
      return false unless ['.yy', '.yyp'].include? extname
      return false unless lines.count > 3
-      return lines[2].match(/\"modelName\"\:\s*\"GM/)
+      return lines[2].match(/\"modelName\"\:\s*\"GM/) ||
+             lines[0].start_with?("1.0.0")


Would there be a way for me to include the weird character back in there? Its this one: ←(U+2190 - 0xE2 0x86 0x90 in utf-8).

It might make the checking a lot stricter, and then could possible replace starts with 1.0.0 with \d\.\d\.\d← which would then support future versions :)

PS: Just throwing the character in there breaks the testsuite;

1) Error: TestBlob#test_language: Encoding::CompatibilityError: incompatible character encodings: ASCII-8BIT and UTF-8 /home/travis/build/github/linguist/lib/linguist/generated.rb:592:in `start_with?' /home/travis/build/github/linguist/lib/linguist/generated.rb:592:in `generated_gamemakerstudio?' /home/travis/build/github/linguist/lib/linguist/generated.rb:93:in `generated?' /home/travis/build/github/linguist/lib/linguist/generated.rb:12:in `generated?' /home/travis/build/github/linguist/lib/linguist/blob_helper.rb:362:in `generated?' /home/travis/build/github/linguist/test/test_blob.rb:281:in `block (2 levels) in test_language' /home/travis/build/github/linguist/test/test_blob.rb:271:in `each' /home/travis/build/github/linguist/test/test_blob.rb:271:in `block in test_language' /home/travis/build/github/linguist/test/test_blob.rb:265:in `each' /home/travis/build/github/linguist/test/test_blob.rb:265:in `test_language' 2) Error: TestFileBlob#test_language: Encoding::CompatibilityError: incompatible character encodings: ASCII-8BIT and UTF-8 /home/travis/build/github/linguist/lib/linguist/generated.rb:592:in `start_with?' /home/travis/build/github/linguist/lib/linguist/generated.rb:592:in `generated_gamemakerstudio?' /home/travis/build/github/linguist/lib/linguist/generated.rb:93:in `generated?' /home/travis/build/github/linguist/lib/linguist/generated.rb:12:in `generated?' /home/travis/build/github/linguist/lib/linguist/blob_helper.rb:362:in `generated?' /home/travis/build/github/linguist/test/test_file_blob.rb:676:in `block (2 levels) in test_language' /home/travis/build/github/linguist/test/test_file_blob.rb:666:in `each' /home/travis/build/github/linguist/test/test_file_blob.rb:666:in `block in test_language' /home/travis/build/github/linguist/test/test_file_blob.rb:660:in `each' /home/travis/build/github/linguist/test/test_file_blob.rb:660:in `test_language'

@RobQuistNL Is LANG set in your environment? On OpenBSD, every test belched spurious encoding errors until I executed tests using LANG=en_AU.UTF-8 bundle exec rake (aliased in my shell as simply brake).

It fixed errors which looked extremely similar to the ones you've posted.

These are the reports from the tests ran on the CI: https://travis-ci.org/github/linguist/jobs/442092134

And it was fixed by removing the ← character from the matching line

Not an expert on how encodings work in Ruby (especially in CI), but you can express Unicode characters as 7-bit ASCII using:

/\u2190/ # RegExp escape "\u2190" # Analogous string escape "\u2190" =~ /\u2190/ # => Returns 0, the index of the first match "foobar" =~ /\u2190/ # => nil No match

The =~ operator is ~~shameless ripped off from Perl~~ Ruby's way of performing a match between a regular expression and a value.

In PHP, the equivalent syntax is pcre_match("/\\u2190/", "input"); (if memory serves, it's been years since I bothered working with PHP. #Node4Life

Ah cool, will try :)

I've looked into this today and attempting to match the ← is definitely a bit of a struggle. I can get it working locally on macOS by using that char directly, but it doesn't play nice on Travis.

An alternate solution may be to more closely match the format of the line ignoring the content, for example:

lines[0] =~ /^\d\.\d\.\d.+\|\{/

... matches the 1.0.0 at the beginning of the line and the |{ at the end, ignoring everything in between and does the trick locally and on Travis here. We could probably get more fancy with the regex, but I'm not sure it's worth the effort.

Ah thanks, I didn't see your comment there for a while. Sorry! Yeah, I think we're going to have to walk this way since the actual char is not going to work.

lib/linguist/generated.rb

RobQuistNL · 2018-10-16T13:39:55Z

Hmm, that explodes again..

TestFileBlob#test_language:
Encoding::CompatibilityError: incompatible encoding regexp match (UTF-8 regexp with ASCII-8BIT string)

Alhadis · 2018-10-16T13:53:46Z

@lildude Could you check this out, please? You're likely more familiar with this pipeline (and Ruby) than I am.

RobQuistNL · 2018-10-16T14:00:05Z

@Alhadis can you confirm that the tests run on your own machine? I can't since I don't have the project running :x

Alhadis · 2018-10-16T14:24:18Z

Yeah, no, I'm getting the same errors as CI is. This is with the brake alias I described earlier.

I'll leave this for an expert to figure out. 😜

RobQuistNL · 2018-10-16T15:00:23Z

IMHO we could remove that one character - allthough it would be a pretty solid detection method :)

lildude · 2018-10-16T17:34:26Z

I've not had the chance to look into this in-depth but this smells very similar to #4028

Alhadis · 2018-10-16T18:08:45Z

Is there any chance of it being related to charlock_holmes? I keep seeing this warning emitted whenever I run bundle exec rake …:

/home/travis/build/github/linguist/vendor/bundle/ruby/2.5.0/gems/charlock_holmes-0.7.6/lib/charlock_holmes/encoding_detector.rb:66: warning: instance variable encoding_list not initialized

... and this doesn't look very good:

    # A mapping table of supported encoding names from EncodingDetector
    # which point to the corresponding supported encoding name in Ruby.
    # Like: {"UTF-8" => "UTF-8", "IBM420_rtl" => "ASCII-8BIT"}
    #
    # Note that encodings that can't be mapped between Charlock and Ruby will resolve
    # to "ASCII-8BIT".
    @encoding_table = {}

Since charlock_holmes appears to be consulting a lower-level system dependency (ICU) I'm willing to bet this is undefined behaviour subject to environmental factors. It would explain why OpenBSD gets flooded with errors in volumes considered unacceptable by anybody's standards — whereas users of other platforms don't appear to have ever experienced this issue.

RobQuistNL · 2018-10-16T21:13:48Z

I really wish I could help, but I'm a real big newbie when it comes to Ruby - I will at least try to setup the project on my Xenial machine soon.

RobQuistNL · 2018-11-22T08:29:14Z

@Alhadis @lildude I have changed the regex to use lildude's /^\d\.\d\.\d.+\|\{/ which seems to make the testsuite run correctly.

Maybe its worth making an issue for the underlying problem - either UTF-8 characters (←), or ASCII-written, escaped UTF-8 characters (\u2190) do not seem to work.

For now this PR should be fine and fix the one wrongly detected file in GMS2 projects :)

Alhadis · 2018-11-22T08:33:20Z

Seems like a decent compromise. 👍 Thanks for your patience!

lib/linguist/generated.rb

pchaigno

Thanks for your contribution @RobQuistNL!

lildude

LGTM

RobQuistNL added 4 commits October 16, 2018 11:50

Make sure a GMS2 generated file is classified as such

9556c1b

Move generated file and update test accordingly

77aa036

Remove incompatible character (←)

6d5485c

Make sure its a fixture thats being loaded and not a sample

44dd06e

RobQuistNL commented Oct 16, 2018

View reviewed changes

RobQuistNL added 2 commits October 16, 2018 13:24

Update generated.rb

1c7a523

Update generated.rb

d50a5de

Alhadis requested changes Oct 16, 2018

View reviewed changes

lib/linguist/generated.rb Outdated Show resolved Hide resolved

Update generated.rb

3c6e083

Update regex and ake it more forgiving

44ceec4

Alhadis approved these changes Nov 22, 2018

View reviewed changes

pchaigno reviewed Nov 22, 2018

View reviewed changes

lib/linguist/generated.rb Show resolved Hide resolved

pchaigno approved these changes Nov 22, 2018

View reviewed changes

lildude approved these changes Dec 1, 2018

View reviewed changes

lildude merged commit 3ff6a9b into github-linguist:master Dec 1, 2018

github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sure a GMS2 generated file is classified as such #4297

Make sure a GMS2 generated file is classified as such #4297

RobQuistNL commented Oct 16, 2018 •

edited

Loading

RobQuistNL Oct 16, 2018

RobQuistNL Oct 16, 2018

Alhadis Oct 16, 2018

RobQuistNL Oct 16, 2018

RobQuistNL Oct 16, 2018

Alhadis Oct 16, 2018

Alhadis Oct 16, 2018 •

edited

Loading

RobQuistNL Oct 16, 2018

lildude Nov 1, 2018

RobQuistNL Nov 22, 2018

RobQuistNL commented Oct 16, 2018

Alhadis commented Oct 16, 2018

RobQuistNL commented Oct 16, 2018

Alhadis commented Oct 16, 2018

RobQuistNL commented Oct 16, 2018

lildude commented Oct 16, 2018

Alhadis commented Oct 16, 2018 •

edited

Loading

RobQuistNL commented Oct 16, 2018

RobQuistNL commented Nov 22, 2018 •

edited

Loading

Alhadis commented Nov 22, 2018

pchaigno left a comment

lildude left a comment

Make sure a GMS2 generated file is classified as such #4297

Make sure a GMS2 generated file is classified as such #4297

Conversation

RobQuistNL commented Oct 16, 2018 • edited Loading

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Alhadis Oct 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobQuistNL commented Oct 16, 2018

Alhadis commented Oct 16, 2018

RobQuistNL commented Oct 16, 2018

Alhadis commented Oct 16, 2018

RobQuistNL commented Oct 16, 2018

lildude commented Oct 16, 2018

Alhadis commented Oct 16, 2018 • edited Loading

RobQuistNL commented Oct 16, 2018

RobQuistNL commented Nov 22, 2018 • edited Loading

Alhadis commented Nov 22, 2018

pchaigno left a comment

Choose a reason for hiding this comment

lildude left a comment

Choose a reason for hiding this comment

RobQuistNL commented Oct 16, 2018 •

edited

Loading

Alhadis Oct 16, 2018 •

edited

Loading

Alhadis commented Oct 16, 2018 •

edited

Loading

RobQuistNL commented Nov 22, 2018 •

edited

Loading