Add Email (eml) language #4201

wesinator · 2018-07-19T16:12:55Z

Description

Adds email header language support

Checklist:

I am adding a new language.
- The extension of the new language is used in hundreds of repositories on GitHub.com.
  - Search results for each extension:
    https://github.com/search?q=reply-to+extension%3Aeml
    https://github.com/search?q=reply-to+extension%3Amsg
    https://github.com/search?q=reply-to+extension%3Ambox
    https://github.com/search?q=from+extension%3Ambx
- I have included a real-world usage sample for all extensions added in this PR:
  - Sample source(s):
    - https://github.com/mariozaizar/eml-tmLanguage/blob/master/spec/fixtures/example.eml
  - Sample license(s): MIT
- I have included a syntax highlighting grammar.
- I have included a change to the heuristics to distinguish my language from others using the same extension.

pchaigno

Thanks for the pull request!

I left a few comments below we'll need to address before merging.

pchaigno · 2018-07-19T17:26:25Z

lib/linguist/languages.yml

+  - ".eml"
+  - ".email"
+  - ".mail"
+  - ".mime"


I'm not sure I understand what this extension has to do with this...?

@pchaigno Those were the extensions in the textmate grammar. I'll remove the last three

pchaigno · 2018-07-19T17:42:43Z

lib/linguist/languages.yml

+  type: data
+  extensions:
+  - ".eml"
+  - ".email"


Looks like a lot of the .email files are not EML. There's few benefit to add support for data languages anyway, so I think it'd be better to remove that file extension from the pull request.

pchaigno · 2018-07-19T17:44:39Z

lib/linguist/languages.yml

+  extensions:
+  - ".eml"
+  - ".email"
+  - ".mail"


Same for .mail as for .email: https://github.com/search?p=99&q=NOT+reply-to+extension%3Amail+NOT+from&type=Code

pchaigno · 2018-07-19T17:45:40Z

lib/linguist/languages.yml

@@ -1244,6 +1244,16 @@ Emacs Lisp:
  codemirror_mode: commonlisp
  codemirror_mime_type: text/x-common-lisp
  language_id: 102
+Email:


Wouldn't EML be a better name?

"Email" is more human readable and won't be confused/conflict with other potential languages (https://github.com/NCEAS/eml, http://xml.coverpages.org/eml.html), but it may not matter.

wesinator · 2018-07-19T18:14:30Z

Done. I actually tried changing the grammar to https://github.com/mariozaizar/language-eml, but it has the following error:

$ script/grammar-compiler add vendor/grammars/language-eml

  > latest: Pulling from linguist/grammar-compiler
  > Digest: sha256:b019869fcf736922e21dd44d2982a0f78ce31b9d24f73f4f758ef1a071c8b4a7
  > Status: Image is up to date for linguist/grammar-compiler:latest
  > The new grammar repository `vendor/grammars/language-eml` (from https://github.com/mariozaizar/language-eml) contains 1 errors:
  >     - Missing include in grammar: `text.eml.basic` (in `grammars/language-eml.cson`) attempts to include `text.html.basic` but the scope cannot be found
  > 
  > failed to compile the given grammar
Command failed. Aborting.

pchaigno · 2018-07-19T18:24:06Z

@wesinator Why did you need to change it? Did you remove the other one before?

wesinator · 2018-07-19T18:32:23Z

@pchaigno I don't need to change it, language-eml just looks more active and has other file extensions

pchaigno · 2018-07-19T18:36:31Z

I see. Could you try following these instructions: https://github.com/github/linguist/blob/master/CONTRIBUTING.md#changing-the-source-of-a-syntax-highlighting-grammar?

wesinator · 2018-07-19T18:40:57Z

@pchaigno Yes, I did that and got the error in #4201 (comment)

pchaigno · 2018-07-19T20:02:08Z

@wesinator It works for me on your pull request. Are you sure you used the right command (./script/add-grammar --replace eml-tmLanguage https://github.com/mariozaizar/language-eml)?

wesinator · 2018-07-19T21:02:37Z

Yes, the grammar has the same error

Edit: interesting, it outputs Command failed. Aborting. but I see it did update the grammar.

pchaigno · 2018-07-20T07:27:21Z

Hm. Maybe I don't have the same error because I pulled the submodules. Could you either download the submodules (git submodules update --init --recursive, it takes a while) or add '-f' to https://github.com/github/linguist/blob/5db20d64a2db4339e88ed729954eb8e849092e41/script/add-grammar#L108 and run the command again (it should ignore compilation errors)?

wesinator · 2018-07-20T15:59:49Z

It works when I run with submodules, but it also removes some unrelated grammars from grammars.yml and vendor/README.md, breaking the build.
https://github.com/github/linguist/pull/4201/files#diff-a17c4358d2937044f2b1438211b3a8f2

Why would it be messing with other grammars ?

EDIT: manually removing and adding the new grammar without --replace works correctly.

pchaigno · 2018-07-20T18:18:02Z

lib/linguist/languages.yml

+  - ".eml"
+  - ".mbox"
+  - ".mbx"
+  - ".msg"


Please add a search query to your original post for each new extension. We need to check that they are popular enough as EML extensions on GitHub and that we won't risk miss-classifying other languages that use those extensions.

@pchaigno done.

Looks like .msg and .mbx have a lot of non-EML files that would be mis-classified:

https://github.com/search?q=NOT+reply-to+extension%3Amsg

https://github.com/search?q=NOT+from+extension%3Ambx

Where did you find these extensions?

Again they're defined in the grammar.

lib/linguist/languages.yml

pchaigno · 2018-09-18T11:32:31Z

This pull request is on hold right now because of a serious-enough issue with the grammar (cf. mariozaizar/language-eml#15). @wesinator Do you know another grammar we could use?

wesinator · 2018-09-18T17:43:50Z

@pchaigno could revert back to https://github.com/mariozaizar/eml-tmLanguage (8c539e9), but that might have the same issue.

pchaigno · 2018-09-18T18:54:16Z

@wesinator Did you test it with Lightshow?

wesinator · 2018-09-18T19:57:10Z

@pchaigno I just did - it does not have issue mariozaizar/language-eml#15, but it also does not have the HTML highlighting feature that language-eml does.

pchaigno · 2018-09-18T20:29:36Z

You might want to post that information in mariozaizar/language-eml#15 to help others debug the issue.

Alhadis · 2018-09-25T04:31:22Z

The issue with the include has been fixed in mariozaizar/language-eml#16, along with an arseload of other improvements. The HTML grammar we're using will either need fixing upstream, or being replaced with Atom's grammar.

See my response in @pchaigno's issue for details.

pchaigno · 2018-10-01T08:08:22Z

@wesinator Could you update your branch with the latest version of the grammar?

pchaigno · 2018-10-01T16:18:56Z

@lildude Do you know how to fix the following error?

cached license data out of date

wesinator · 2018-10-01T16:21:18Z

@pchaigno I ran ./script/licensed cache

pchaigno · 2018-10-01T16:29:44Z

@wesinator Looks like you know more about Linguist than I do 😄

All good then!

wesinator · 2018-10-01T16:34:17Z

I had to look at https://github.com/github/licensed#usage

It did seem to duplicate the license text

Missed an error

pchaigno · 2018-10-01T16:37:28Z

Oh right, I missed that.
@lildude Should we just remove the second license text manually?

lildude · 2018-10-01T16:46:55Z

@lildude Should we just remove the second license text manually?

I believe this is caused by running an old version of Licensed or Licensee. Running bundle update, deleting the license file and then script/licensed again should sort it out.

lildude · 2018-10-01T16:47:19Z

(you might need to merge master into your fork too)

wesinator added 2 commits July 19, 2018 12:05

Add Email (eml) highlighting

e5cd0d1

Update vendor README

cce5f00

pchaigno suggested changes Jul 19, 2018

View reviewed changes

wesinator added 2 commits July 19, 2018 13:54

Remove extra email file extensions

4da8cfa

Rename Email to EML

22805bd

Fix tests for EML

f2552e8

Move EML in languages.yml

8c539e9

wesinator added 3 commits July 20, 2018 13:06

Switch grammar to language-eml

566f6ab

Add msg, mbox extensions from grammar

665156a

Fix extensions sort test

dba6553

pchaigno reviewed Jul 20, 2018

View reviewed changes

pchaigno added the Awaiting Lightshow label Jul 21, 2018

Remove msg, mbx extensions

ad8c3ae

wesinator commented Jul 31, 2018

View reviewed changes

lib/linguist/languages.yml Show resolved Hide resolved

pchaigno mentioned this pull request Aug 14, 2018

Quoted-printable encoding unrecognized on GitHub mariozaizar/language-eml#15

Closed

1 task

pchaigno removed the Awaiting Lightshow label Sep 18, 2018

Merge branch 'master' into eml

89f1245

Alhadis mentioned this pull request Sep 25, 2018

Use Atom's HTML grammar instead of TextMate's #4274

Merged

1 task

Update language-eml to latest

350e483

Update license file

4b581fa

pchaigno previously approved these changes Oct 1, 2018

View reviewed changes

Fix license

7e3fc46

pchaigno approved these changes Oct 1, 2018

View reviewed changes

pchaigno requested a review from lildude October 4, 2018 16:49

lildude approved these changes Oct 5, 2018

View reviewed changes

lildude merged commit 878b30f into github-linguist:master Oct 5, 2018

wesinator deleted the eml branch December 27, 2018 20:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Email (eml) language #4201

Add Email (eml) language #4201

wesinator commented Jul 19, 2018 •

edited

pchaigno left a comment

pchaigno Jul 19, 2018

wesinator Jul 19, 2018

pchaigno Jul 19, 2018

pchaigno Jul 19, 2018

pchaigno Jul 19, 2018

wesinator Jul 19, 2018

wesinator commented Jul 19, 2018

pchaigno commented Jul 19, 2018

wesinator commented Jul 19, 2018

pchaigno commented Jul 19, 2018

wesinator commented Jul 19, 2018

pchaigno commented Jul 19, 2018

wesinator commented Jul 19, 2018 •

edited

pchaigno commented Jul 20, 2018

wesinator commented Jul 20, 2018 •

edited

pchaigno Jul 20, 2018

wesinator Jul 20, 2018

pchaigno Jul 21, 2018

wesinator Jul 21, 2018

pchaigno commented Sep 18, 2018

wesinator commented Sep 18, 2018

pchaigno commented Sep 18, 2018

wesinator commented Sep 18, 2018

pchaigno commented Sep 18, 2018

Alhadis commented Sep 25, 2018

pchaigno commented Oct 1, 2018

pchaigno commented Oct 1, 2018

wesinator commented Oct 1, 2018

pchaigno commented Oct 1, 2018

wesinator commented Oct 1, 2018

pchaigno commented Oct 1, 2018

lildude commented Oct 1, 2018

lildude commented Oct 1, 2018

Add Email (eml) language #4201

Add Email (eml) language #4201

Conversation

wesinator commented Jul 19, 2018 • edited

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

wesinator commented Jul 19, 2018

pchaigno commented Jul 19, 2018

wesinator commented Jul 19, 2018

pchaigno commented Jul 19, 2018

wesinator commented Jul 19, 2018

pchaigno commented Jul 19, 2018

wesinator commented Jul 19, 2018 • edited

pchaigno commented Jul 20, 2018

wesinator commented Jul 20, 2018 • edited

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

pchaigno commented Sep 18, 2018

wesinator commented Sep 18, 2018

pchaigno commented Sep 18, 2018

wesinator commented Sep 18, 2018

pchaigno commented Sep 18, 2018

Alhadis commented Sep 25, 2018

pchaigno commented Oct 1, 2018

pchaigno commented Oct 1, 2018

wesinator commented Oct 1, 2018

pchaigno commented Oct 1, 2018

wesinator commented Oct 1, 2018

pchaigno commented Oct 1, 2018

lildude commented Oct 1, 2018

lildude commented Oct 1, 2018

wesinator commented Jul 19, 2018 •

edited

wesinator commented Jul 19, 2018 •

edited

wesinator commented Jul 20, 2018 •

edited