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

Add Odin language to linguist #4676

Merged
merged 15 commits into from Jan 27, 2020
Merged

Add Odin language to linguist #4676

merged 15 commits into from Jan 27, 2020

Conversation

ThisDevDane
Copy link
Contributor

@ThisDevDane ThisDevDane commented Oct 16, 2019

Description

The Odin programming language is fast, concise, readable, pragmatic and open sourced. It is designed with the intent of replacing C. Github Repo

Checklist:

@stale
Copy link

stale bot commented Nov 15, 2019

This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

@stale stale bot added the Stale label Nov 15, 2019
@stale stale bot removed the Stale label Nov 18, 2019
@stale
Copy link

stale bot commented Dec 18, 2019

This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

@stale stale bot added the Stale label Dec 18, 2019
@gingerBill
Copy link

Please do not close this pull request.

@stale stale bot removed the Stale label Dec 19, 2019
Alhadis added a commit to Alhadis/Silos that referenced this pull request Dec 20, 2019
Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

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

Alrighty, from the results I harvested earlier, it looks like Odin's in-the-wild distribution is okay. 👍 However, there's a conflicting format with the .odin extension called Object Data Instance Notation, whose distribution is non-trivial and needs to be considered as well. Examples can be seen here.

I suggest adding something like:

Object Data Instance Notation:
  type: data
  extensions:
  - ".odin"
  tm_scope: none
  ace_mode: text

We might need to add a heuristic if the classifier does a bad job at identifying the .odin samples.

Summary (1,811 unique results)
Count Ratio Format
1 0.06% i3 Config
1 0.06% rdist(1)
15 0.83% Makefile
28 1.55% Unknown
771 42.57% Odin
995 54.94% Object Data Instance Notation

@ThisDevDane
Copy link
Contributor Author

@Alhadis So do you mean I should add that Object Data Instance Notation with grammar, samples and all that? Or just that snippet into languages.yml ?

@Alhadis
Copy link
Collaborator

Alhadis commented Dec 26, 2019

Samples are required, a grammar is not (although it's a nice-to-have). You can use these samples:

@ThisDevDane
Copy link
Contributor Author

@Alhadis I have added the definition for Object Data Instance Notation now

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.

We're going to need a heuristic to clearly differentiate the two .odin extensions.

Can you please update the template to link directly to the source of the two sample files you've included and explicitly state the licenses of each. Referencing the samples/odin directory within the Linguist repo or your fork doesn't tell us where you actually got the files 😉

@ThisDevDane
Copy link
Contributor Author

ThisDevDane commented Jan 6, 2020

@lildude Ahh, I misunderstood the PR template ;P I've added the direct link to the sample, do you want me to copy the BSD-2 license from the repo to the top of the file?

I'll try and write a heuristic or get somebody in our community to do it!

@lildude
Copy link
Member

lildude commented Jan 6, 2020

do you want me to copy the BSD-2 license from the repo to the top of the file?

No need. We only need to know where the file comes from and the license.

Are you sure https://github.com/wolandscat/EOMF/blob/master/resources/test_files/odin/openehr_rm_102.odin is covered by the BSD-2 license too? The repo doesn't have a license file so I'm not sure how you've come to the conclusion it is covered by this license.

@ThisDevDane
Copy link
Contributor Author

Oh yea sorry, Alhadis just posted a list and I took one of them since he said I could, should have checked licenses. I can switch it out to this https://github.com/opencimi/archetypes/blob/master/export/odin/flat/CIMI-CORE-ITEM_GROUP.amino_acidemias_dbsimp.v1.0.0.odin which seems to mention CC-3.0 but I honestly don't know anything about Object Data Instance Notation so I don't know where else to look for some samples?

@ThisDevDane
Copy link
Contributor Author

@lildude
Copy link
Member

lildude commented Jan 6, 2020

I think the latter is the better option. It's less ambiguous.

@ThisDevDane
Copy link
Contributor Author

@lildude So I added a heuristic, but I am unsure of how to test it

lib/linguist/heuristics.yml Show resolved Hide resolved
@Alhadis
Copy link
Collaborator

Alhadis commented Jan 7, 2020

Oh yea sorry, Alhadis just posted a list and I took one of them since he said I could, should have checked licenses

My bad. I must have read Copyright (c) 2009- openEHR Foundation in openehr_rm_102.odin and assumed it was taken from OpenEHR's specifications repository (which is Apache 2.0-licensed). Guess I had my browser tabs mixed up. 😜

Alhadis added a commit to Alhadis/language-etc that referenced this pull request Jan 8, 2020
@Alhadis
Copy link
Collaborator

Alhadis commented Jan 8, 2020

@ThisDrunkDane I've pushed an update to your branch. Specifically, I've

  • Added a grammar for highlighting Object Data Instance Notation
  • Added the aforementioned heuristic for matching openEHR ODIN files
  • Amended your heuristic to be more specific (as well as corrected several RegExp mistakes). One of the patterns you were matching (for .* in) was very general and had the potential to match two word fragments on two different lines

@ThisDevDane
Copy link
Contributor Author

Sounds good 👍 Haven't used regex all that much so yea it was probably riddled with mistakes, I just used regexr.com to see if my regex only matched lines in the odin and example and none in the other one.

@ThisDevDane
Copy link
Contributor Author

So is there anything else that needs to be done to this PR?

@Alhadis
Copy link
Collaborator

Alhadis commented Jan 9, 2020

@ThisDrunkDane Nothing on our end. We just have to wait for @lildude to give his 👍 of approval, as we don't merge PRs until a GitHub staff member green-lights them.

@Alhadis Alhadis requested a review from lildude January 25, 2020 11:39
@Alhadis
Copy link
Collaborator

Alhadis commented Jan 27, 2020

@lildude This is good to merge, BTW.

@lildude lildude merged commit 12f5e63 into github-linguist:master Jan 27, 2020
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

5 participants