Adding .yy and .yyp (GMS2 Project / metadata files) as JSON, and GML heuristics #4130

RobQuistNL · 2018-05-14T11:21:22Z

This change makes sure the .yy and .yyp files are correctly detected as JSON files within a Game Maker project.

I'm having difficulties running this tool in an example project - I could use some help with running my branch of linguist inside of an example project. (I get an error message Could not locate Gemfile or .bundle/ directory)

Description

Not sure how the Yacc language responds to this. Maybe I should also add some heuristic rules for .yy and .yyp files? They are pretty easily distinguishable from Yacc files.

Checklist:

I am associating a language with a new file extension.
- The new extension is used in hundreds of repositories on GitHub.com
  - Search results for each extension:
    - https://github.com/search?p=99&q=modelname+extension%3Ayy&type=Code&utf8=%E2%9C%93
    - https://github.com/search?p=99&q=modelname+extension%3Ayyp&type=Code&utf8=%E2%9C%93
- I have included a real-world usage sample for all extensions added in this PR:
  - Sample source(s):
  - Sample license(s): MIT
- I have included a change to the heuristics to distinguish my language from others using the same extension.

pchaigno

Thanks for taking a stab at this!

I could use some help with running my branch of linguist inside of an example project. (I get an error message Could not locate Gemfile or .bundle/ directory)

Do you have a proper Ruby environment configured?

Maybe I should also add some heuristic rules for .yy and .yyp files? They are pretty easily distinguishable from Yacc files.

👍

I've left a few comments to address below.

pchaigno · 2018-05-14T13:00:14Z

lib/linguist/languages.yml

@@ -2079,6 +2079,8 @@ JSON:
  - ".topojson"
  - ".webapp"
  - ".webmanifest"
+  - ".yyp"


The file extensions (except the first) need to be sorted.

pchaigno · 2018-05-14T13:01:53Z

lib/linguist/heuristics.rb

@@ -541,5 +541,13 @@ def call(data)
      end
    end

+    disambiguate ".yy" do |data|
+      Language["JSON"] if /\"modelName\"\:\s?\"/.match(data)
+    end


We'll need to add a case for Yacc here.

Ah, I thought it would fall through. Would a simple "else" be sufficient in this case?

Right, my need above might be a bit strong. It's better if we have a case for Yacc 😄
Is the presence of the modelName key sufficient to identify GML JSON files? (i.e., is it mandatory in those files?)

Pretty much. All the .yy files I've seen have a modelName. As does the .yyp file. But like you said - the .yyp file isn't documented in the languages.rb file so I've removed it in the latest commit. I've extended this testcase to first go for XML, then Graph Markup Language, and then a GameMaker file. The latter is harder because its a generic programming language and has no "required" keywords in the file. XML does, and it looks like Graph Markup Language does too (http://www.fim.uni-passau.de/fileadmin/files/lehrstuhl/brandenburg/projekte/gml/gml-technical-report.pdf) - the graph [ and potentially the node [.

pchaigno · 2018-05-14T13:02:09Z

lib/linguist/heuristics.rb

+
+    disambiguate ".yyp" do |data|
+      Language["JSON"] if /\"modelName\"\:\s?\"GMProject\"/.match(data)
+    end


Is this required? Did you find other languages using .yyp?

Not sure, don't think so. Will look this up.

Just curious: is there a guarantee that only one or zero whitespace characters follow modelName:? If not, you should replace \s? with \s* so it'll match 2+ blanks instead of 1 or none. 😉

Good one, the * would be better. They are automatically generated files though, so I'm sure its usually 1 whitespace, but the * is better. Committing as we speak!

I've extended it since I've checked pretty much all .yy files, and they all contain a modelName element with a value that starts with GM.

"modelName": "GM a match on that one is what I'm doing for this rule.

RobQuistNL · 2018-05-14T13:28:32Z

~~Would it be OK to add a .gml check like this;~~

disambiguate ".gml" do |data|
      Language["Game Maker Language"] if /^\s*(var|sprite_|vk_|image_|draw_|visible|sprite_)/.match(data)
    end

~~to this PR as well? I suppose I will have to add checks for Graph Modeling Language and XML as well?~~

EDIT: I've added checks for XML and Graph first, then make it fall through to Game Maker Language. Its probably easier to do this way and make sure.

I'm still pretty unfamiliar with Ruby so I'm not even sure if this code runs. I've been testing my regexes with this online tool; http://rubular.com/ but I wouldn't feel calm getting this merged without it being properly tested.

I'm sorry this is a bit slow & sluggish - I'm not familiar with Ruby. @pchaigno is there a IRC / slack / discord channel where I can ask for some guidance or this is place OK?

RobQuistNL · 2018-05-14T13:58:18Z

So the test is failing because;

TestSamples#test_Yacc_has_samples [/home/travis/build/github/linguist/test/test_samples.rb:86]:
Missing samples in "samples/Yacc/*.yy".

I'm not sure where I can pull an example file from - I can find some example files online but I'm not sure about the licenses on that one

TestSamples#test_JSON_has_samples [/home/travis/build/github/linguist/test/test_samples.rb:86]:
Missing samples in "samples/Yacc/*.yy".

I suppose this is to check if Yacc files don't get detected as JSON, while GM files do? So thats 2 birds one stone

TestClassifier#test_classify_ambiguous_languages:
NoMethodError: undefined method `[]' for nil:NilClass

I'm not sure what to do with this one :(

RobQuistNL · 2018-05-14T14:12:49Z

I've also taken this opportunity to add .gmx files. They are from GameMaker:Studio 1 (EOL). Haven't found any other .gmx files in the languages.yml so this should do just fine.

GameMakerStudio 2 uses the .gml, .yy and .yyp files.

RobQuistNL · 2018-05-30T20:45:35Z

Hey @pchaigno, are the changes I made valid like this?

RobQuistNL · 2018-06-12T09:36:55Z

Ping @pchaigno :x sorry for my impatience :P

pchaigno

For next time, it would be easier to review if the independent changes were in separate pull requests. For example, everything regarding the .yy and .yyp extensions looks ok and could be merged as is.

However, I couldn't find the license for the 3 other samples, including the two .gmx files. Did I miss something? If not, would it be possible to use other sample files, under a permissive license?

pchaigno · 2018-06-14T13:01:37Z

samples/Yacc/words2.yy

@@ -0,0 +1,53 @@
+/* File taken from http://home.adelphi.edu/~sbloch/class/271/examples/lexyacc/words2.y . */


Couldn't find the license for this. Is it indicated anywhere on the website?

Me neither, I find it very hard to find an example file for the .y files with a license - Since it was published like this online on an .edu site I was assuming. As we all know, assumptions are the mother of all f*ckups, so I'll have to find something else for this.

The GMX files are my own, I'll add a MIT license to the repository there.

RobQuistNL · 2018-06-16T19:42:06Z

@pchaigno Sorry for the inconvenience.

I've included a Yacc file from an MIT repository (https://github.com/zcbenz/yacci/blob/master/examples/funccal/calc.y) and I've added the MIT license to my own repository for the 2 GMX files.

…gamemaker-yy-and-yyp-files

RobQuistNL · 2018-06-17T07:25:21Z

I take it these failures are unrelated to this PR?

  1) Failure:
TestGrammars#test_whitelisted_hashes_dont_have_licenses [/home/travis/build/github/linguist/test/test_grammars.rb:130]:
The following whitelisted license hashes are unused:
* 7dfce11e2e3579ee43b83e69b1b64e77a2e378f0
* 62b97e52b78439c14550a44a3fe51332aeffb3a1
* 0acff2bb1536a3942a39ac74987ffd9c44905a6b
* 41cdc7e9f9d2e62eb8ac68a1a9359b9c39a7a9bf
Please remove them from the hash whitelist.
--- expected
+++ actual
@@ -1 +1 @@
-[]
+["7dfce11e2e3579ee43b83e69b1b64e77a2e378f0", "62b97e52b78439c14550a44a3fe51332aeffb3a1", "0acff2bb1536a3942a39ac74987ffd9c44905a6b", "41cdc7e9f9d2e62eb8ac68a1a9359b9c39a7a9bf"]
  2) Failure:
TestGrammars#test_submodules_have_approved_licenses [/home/travis/build/github/linguist/test/test_grammars.rb:115]:
The following submodules have unapproved licenses:
* vendor/grammars/FreeMarker.tmbundle: b81acf2ba52d312754bf5055845a723123bda388
* vendor/grammars/ant.tmbundle: 4da01d631a29c76456fd0bd16749c71e8d5f6dbf
* vendor/grammars/elixir-tmbundle: 220e011c8d686129e9c4163a7c655b9d64f61e59
* vendor/grammars/mako-tmbundle: 39f092c726491ca6a02354dbc6c3a0920bb44d4c
The license must be added to the LICENSE_WHITELIST in /test/test_grammars.rb once approved.
--- expected
+++ actual
@@ -1 +1 @@
-[]
+["vendor/grammars/FreeMarker.tmbundle: b81acf2ba52d312754bf5055845a723123bda388", "vendor/grammars/ant.tmbundle: 4da01d631a29c76456fd0bd16749c71e8d5f6dbf", "vendor/grammars/elixir-tmbundle: 220e011c8d686129e9c4163a7c655b9d64f61e59", "vendor/grammars/mako-tmbundle: 39f092c726491ca6a02354dbc6c3a0920bb44d4c"]

lildude · 2018-06-18T16:16:52Z

The test failures appear to be caused by recent changes to licensee which has resulted in a slightly modified hash for the four licenses in question. I'll address this in #4152

lildude · 2018-06-21T14:10:57Z

Merging master into your branch should fix the failing tests.

…gamemaker-yy-and-yyp-files

RobQuistNL · 2018-06-26T08:23:36Z

Thanks @lildude !

@pchaigno We should be good to go now I think :)

pchaigno

Thanks for the changes @RobQuistNL!

I've got two concerns below, but we're getting closer to something ready to be merged.

pchaigno · 2018-06-26T08:59:26Z

lib/linguist/heuristics.rb

@@ -531,5 +541,13 @@ def call(data)
      end
    end

+    disambiguate ".yy" do |data|
+      if /\"modelName\"\:\s*\"GM/.match(data)


Are these files generated? If not, shouldn't we match on modelName: "GM too?

They are generated, but I've only seen them with the double quotes. I put the \s in there just in case

Ok, thanks for the precision. We should probably add a case in generated.rb for those files then.

Pretty much all the .yy and .yyp files will be generated. The GML files (which the .yy files will point to) are 1:1 contents from the IDE. .yy contains options and a lot of meta-data for the IDE.

I've put this in place to disambuguate Yacc from GameMaker's JSON files - do you think we need the same check in generated.rb? Pretty much, if its a .yy and it has that "modelName": "GM format in there its a GM generated file for sure.

do you think we need the same check in generated.rb?

Yes, I think it's worth it. Is that the only indication that the file is generated? We often have clear comments such as Generated by XXXX.

Unfortunately GM doesn't do that. Its plain JSON, but at least it has a consistent format. I'll add a case to generated.rb.

pchaigno · 2018-06-26T09:08:10Z

lib/linguist/heuristics.rb

+        Language["Graph Modeling Language"]
+      else
+        Language["Game Maker Language"]
+      end


I don't think we can be sure we've eliminated the two other candidate languages here. Are we even sure /^\s*(graph\s?\[|node\s?\[)/i will match all Graph Modeling Language files?

As far as I could tell the Graph modeling Languages always have a graph [ or node [ element.

http://www.fim.uni-passau.de/fileadmin/files/lehrstuhl/brandenburg/projekte/gml/gml-technical-report.pdf

I haven't found a lot of docs, but I'm quite sure that it then will not be Game Maker Language. (text [ is invalid syntax), unless its in a comment ofcourse. I'll be searching a bit more for the Graph Modeling Language file formats.

Here there is some more documentation. All files I've seen so far have that graph [ and node [ stuff.

http://www.fim.uni-passau.de/index.php?id=17297&L=1

RobQuistNL · 2018-06-26T10:07:14Z

I've edited the Graph Modeling Language regex - it will now match most of the files found here;

https://github.com/search?p=9&q=extension%3Agml+graph&ref=searchresults&type=Code&utf8=%E2%9C%93

I can't find any hard evidence that graph [ or node [ has to be in a graph modeling language file - but all the examples I've found have that setup, and so do the files in the search results.

RobQuistNL · 2018-06-26T10:24:07Z

Hmm, might be an issue, since graph [4] = 20; is actually valid GML and it will match. Most people will write array accessors like graph[4] = 20; without the space though.. :/

pchaigno · 2018-06-26T11:42:05Z

Hmm, might be an issue, since graph [4] = 20; is actually valid GML and it will match. Most people will write array accessors like graph[4] = 20; without the space though.. :/

We could anchor the [ to the end of the line?

RobQuistNL · 2018-07-02T15:47:57Z

Here goes 🤞

Thanks a ton for your quick responses and your patience @pchaigno! I really appreciate it.

pchaigno · 2018-07-02T16:05:40Z

lib/linguist/heuristics.rb

+    disambiguate ".gml" do |data|
+      if /^\s*(\<\?xml|xmlns)/i.match(data)
+        Language["XML"]
+      elsif /^\s*(graph|node)\s+\[$/i.match(data)


We still want to match on:

graph [ foobar

right?
Any reason you changed the [\s\n\r]?

I thought \s would include \n and \r, hence I removed it

\s matches any whitespace character (equal to [\r\n\t\f\v ])

This format is in sample.gml and now also in sample3.gml. I think the succeeding tests should prove that the regex works, right? :)

Oh, right! my bad!

My bad for initially writing \s\r\n ;)

pchaigno · 2018-07-03T17:16:20Z

test/test_heuristics.rb

+  def test_gml_by_heuristics
+    assert_heuristics({
+      "Game Maker Language" => all_fixtures("Game Maker Language", "*.gml"),
+      "Graph Modeling Language" => all_fixtures("Graph Modeling Language", "*.gml")


Could we add a case for XML too?

Good catch!

I've added the testcases for .yy files as well.

RobQuistNL · 2018-07-16T09:19:07Z

@pchaigno Ping! Congratulations on winning the finals BTW :) Hope your hangover is not too bad ;)

…gamemaker-yy-and-yyp-files

RobQuistNL · 2018-08-08T11:47:19Z

@pchaigno Sorry :x ping again

pchaigno · 2018-08-08T12:21:05Z

Should we add a check in generated.rb?

pchaigno · 2018-08-08T13:51:41Z

lib/linguist/generated.rb

+    # Return true or false
+    def generated_gamemakerstudio?
+      return false unless extname == '.yy' || extname == '.yyp'
+      return false unless lines.count > 5


Why require min. 6 lines if we only look at the third?

pchaigno · 2018-08-08T13:52:10Z

lib/linguist/generated.rb

+    def generated_gamemakerstudio?
+      return false unless extname == '.yy' || extname == '.yyp'
+      return false unless lines.count > 5
+      return /\"modelName\"\:\s*\"GM/.match(lines[3])


In the comment you mention the third line, but here you match on the fourth. Typo?

Sorry, yeah, was already on it ^_^

(can't run the tests locally so abusing Travis for it :x )

…ccount

RobQuistNL · 2018-08-08T14:13:27Z

@pchaigno The generated test should work now ^_^

pchaigno

LGTM!

Thanks for your patience @RobQuistNL!

RobQuistNL · 2018-08-10T11:25:19Z

Oh, if anything I should thank you for your patience @pchaigno !

lildude

LGTM. Thanks for the PR and welcome to Linguist.

RobQuistNL · 2018-08-12T12:47:09Z

Awesome! Thanks :)

RobQuistNL added 2 commits May 14, 2018 13:07

Adding .yy and .yyp (GMS2 Project / metadata files) as JSON

08f0604

Add heuristics for .yy and .yyp

6a639d8

pchaigno suggested changes May 14, 2018

View reviewed changes

RobQuistNL added 4 commits May 14, 2018 15:29

Swap order for .yy and .yyp files

6a543c3

Improve heuristics for .gml and .yy files

2d1ef40

Fix potentially more whitespaces for .yy detection.

5cd5542

Make heuristics for GameMaker .yy files even more specific.

a514723

Add GameMakerStudio 1 project files

918dddc

Add Yacc sample file

5405347

pchaigno suggested changes Jun 14, 2018

View reviewed changes

Update and rename words2.yy to calc.y

a1fd111

RobQuistNL and others added 2 commits June 17, 2018 09:21

Rename calc.y to calc.yy

aa7dcc2

Merge branch 'master' of github.com:github/linguist into feature/add-…

d914675

…gamemaker-yy-and-yyp-files

Merge branch 'master' of github.com:github/linguist into feature/add-…

72ffeaf

…gamemaker-yy-and-yyp-files

pchaigno suggested changes Jun 26, 2018

View reviewed changes

Update heuristics.rb

60606cf

Create sample2.gml

91659db

Add heuristics testcase for game maker languages

d1cefb6

RobQuistNL added 2 commits July 2, 2018 17:52

Update heuristics.rb

1d4def7

Update test_heuristics.rb

6efeacd

pchaigno reviewed Jul 2, 2018

View reviewed changes

Add extra Graph sample file with different formatting

eac7cd8

RobQuistNL changed the title ~~Adding .yy and .yyp (GMS2 Project / metadata files) as JSON~~ Adding .yy and .yyp (GMS2 Project / metadata files) as JSON, and GML heuristics Jul 3, 2018

pchaigno reviewed Jul 3, 2018

View reviewed changes

RobQuistNL added 2 commits July 4, 2018 02:24

Add XML test case for GML files and JSON / Yacc test for .yy files

ff57fde

Fix typo

f760870

Merge branch 'master' of github.com:github/linguist into feature/add-…

d1d0492

…gamemaker-yy-and-yyp-files

RobQuistNL added 2 commits August 8, 2018 15:41

Add generated check

c16177f

Add testcase for GMS2 generated files

66a15d6

pchaigno suggested changes Aug 8, 2018

View reviewed changes

RobQuistNL added 2 commits August 8, 2018 15:59

Make validation in generated clearer + take 0 index line array into a…

37921da

…ccount

Trying to make regex match work

048cdd0

pchaigno approved these changes Aug 8, 2018

View reviewed changes

lildude approved these changes Aug 11, 2018

View reviewed changes

lildude merged commit 8b752c8 into github-linguist:master Aug 11, 2018

RobQuistNL mentioned this pull request Aug 28, 2018

Change GameMakerLanguage colour to reflect the "Game Maker Green" #4255

Merged

RobQuistNL mentioned this pull request Oct 10, 2018

Some GameMakerLanguage files are still marked as Graph Modeling Language #4294

Closed

lildude mentioned this pull request Oct 10, 2018

Game Maker Studio 2's broken "json" file is misclassified as Yacc #4293

Closed

lildude mentioned this pull request Aug 15, 2023

GML Projects get marked as Yacc because of .yy files #4129

Closed

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

		@@ -0,0 +1,53 @@
		/* File taken from http://home.adelphi.edu/~sbloch/class/271/examples/lexyacc/words2.y . */

Adding .yy and .yyp (GMS2 Project / metadata files) as JSON, and GML heuristics #4130

Adding .yy and .yyp (GMS2 Project / metadata files) as JSON, and GML heuristics #4130

Conversation

RobQuistNL commented May 14, 2018 • edited by pchaigno Loading

Description

Checklist:

pchaigno left a 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

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

RobQuistNL commented May 14, 2018 • edited Loading

RobQuistNL commented May 14, 2018 • edited Loading

RobQuistNL commented May 14, 2018

RobQuistNL commented May 30, 2018

RobQuistNL commented Jun 12, 2018

pchaigno left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobQuistNL commented Jun 16, 2018

RobQuistNL commented Jun 17, 2018

lildude commented Jun 18, 2018

lildude commented Jun 21, 2018

RobQuistNL commented Jun 26, 2018

pchaigno left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobQuistNL Jun 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobQuistNL commented Jun 26, 2018

RobQuistNL commented Jun 26, 2018

pchaigno commented Jun 26, 2018

RobQuistNL commented Jul 2, 2018

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobQuistNL commented Jul 16, 2018

RobQuistNL commented Aug 8, 2018

pchaigno commented Aug 8, 2018

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 Aug 8, 2018

pchaigno left a comment

Choose a reason for hiding this comment

RobQuistNL commented Aug 10, 2018

lildude left a comment

Choose a reason for hiding this comment

RobQuistNL commented Aug 12, 2018

RobQuistNL commented May 14, 2018 •

edited by pchaigno

Loading

RobQuistNL commented May 14, 2018 •

edited

Loading

RobQuistNL commented May 14, 2018 •

edited

Loading

pchaigno left a comment •

edited

Loading

RobQuistNL Jun 26, 2018 •

edited

Loading