Add Slice language (.ice) #4243

externl · 2018-08-21T19:18:35Z

Add new language syntax for Slice

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?utf8=%E2%9C%93&type=Code&ref=searchresults&q=extension%3Aice+module+NOT+nothack
- I have included a real-world usage sample for all extensions added in this PR:
  - Sample source(s):
    - samples/Slice/Test.ice
  - Sample license(s): bsd-3-clause
- lightshow example: see here
- I have updated the heuristics to distinguish my language from others using the same extension.
  According to the contributing guide this was not needed as there are no other languages using .ice.

externl · 2018-08-21T19:27:38Z

The errors seem unrelated, or maybe I'm mistaken?

pchaigno

Thanks for the pull request!

Please include a Lightshow example for the grammar as per the template.

externl · 2018-08-22T02:08:00Z

Thanks, @pchaigno. I've added that.

pchaigno · 2018-08-22T09:27:40Z

Are these files also Slice? They look like JSON.

~~Could you try to update your branch with the latest master to address the Travis CI issue?~~ Actually, I can reproduce the issue locally with the master branch. @lildude Any idea of what's happening here?

lildude · 2018-08-22T09:47:18Z

@lildude Any idea of what's happening here?

Looking now. My suspicion is licensed has changed again 😞

pchaigno · 2018-08-22T09:56:31Z

@lildude Actually, it's Licensee this time. I filled an issue: licensee/licensee#328.

lildude · 2018-08-22T09:56:58Z

Yeah, I was just about to confirm this.

externl · 2018-08-22T12:57:09Z

Are these files also Slice? They look like JSON.

Yeah, most of those are definitely JSON. Some binary files too.

pchaigno · 2018-08-22T14:29:19Z

Yeah, most of those are definitely JSON.

We'll need to find a way to distinguish JSON and Slice files then.

externl · 2018-08-22T16:07:31Z

Ok, I'll add a heuristic for Slice to heuristic.rb

externl · 2018-08-22T16:17:08Z

Ok, so I was going to add:

disambiguate ".ice" do |data|
    if /^\s*(#\s*(include|if[n]def|pragma)|module\s+[A-Za-z][_A-Za-z0-9]*)/i.match(data)
        Language["Slice"]
    end
end

This should be a good check for Slice. I don't really see a good check for JSON.

Is this okay?

pchaigno · 2018-08-25T20:16:41Z

@externl Could you update your branch with the latest master? It includes a fix for the Travis CI tests.

I don't really see a good check for JSON.

Would checking for the root object of the JSON document ({ or [) be enough? /cc @Alhadis who, with a bit of luck, knows more about the JSON spec. than I do.

externl · 2018-08-28T13:57:22Z

@pchaigno I've merged in the latest. Maybe the best is to just not check for JSON, leave it non-highlighted?

pchaigno · 2018-08-28T14:04:59Z

Unfortunately, that's not an option our classifier has at the moment. With this pull request all .ice files will be classified as either JSON or Slice, so we need to do a good job distinguishing the two.

externl · 2018-08-28T14:09:18Z

What happens to files that don't match the heuristic?

disambiguate ".ice" do |data|
    if /^\s*(#\s*(include|if[n]def|pragma)|module\s+[A-Za-z][_A-Za-z0-9]*)/i.match(data)
        Language["Slice"]
    end
end

vs

disambiguate ".ice" do |data|
    if /^\s*(#\s*(include|if[n]def|pragma)|module\s+[A-Za-z][_A-Za-z0-9]*)/i.match(data)
        Language["Slice"]
    elsif SOME_JSON_CHECK
        Language["JSON"]
    end
end

?

pchaigno · 2018-08-28T14:26:11Z

They go to the naïve Bayesian classifier (see How Linguist Works) which assigns a score to each language based on the tokens identified in the file.

externl · 2018-09-21T14:35:14Z

@pchaigno I've rebased my branch on the latest. Do you still want me to look into adding a JSON disambiguations check for these other .ice files?

pchaigno · 2018-09-21T14:42:51Z

Do you still want me to look into adding a JSON disambiguations check for these other .ice files?

Yes, that would be much better.

Would checking for the root object of the JSON document ({ or [) be enough?

@lildude @Alhadis Any ideas on the above?

Alhadis · 2018-09-21T14:46:09Z

Any ideas on the above?

Yep. Don't forget to check for leading whitespace too (either vertical or horizontal). E.g, something like this should be enough:

/\A\s*[{[]/

pchaigno · 2018-09-21T14:55:20Z

@externl Could you implement the above heuristic rule with the corresponding test in test/test_heuristics.rb?

Alhadis · 2018-09-21T14:56:10Z

vendor/licenses/grammar/vscode-slice.txt

What happened here?

What do you mean? In my recent commit I added the version.

This line appears to have been modified, although GitHub's diff-view isn't showing what changed... Strange.

A trailing newline may have been added or removed when you edited the license, but GitHub normally indicates that with an 🚫 icon.

Yeah my local diff shows that I added a trailing newline

diff --git a/vendor/licenses/grammar/vscode-slice.txt b/vendor/licenses/grammar/vscode-slice.txt index b1e84f5c..0103ec63 100644 --- a/vendor/licenses/grammar/vscode-slice.txt +++ b/vendor/licenses/grammar/vscode-slice.txt @@ -1,6 +1,7 @@ --- type: grammar name: vscode-slice +version: 6a28c2125bb442fa6a7f31c995ce423b1c9bbfb7 license: bsd-3-clause --- BSD 3-Clause License @@ -31,4 +32,4 @@ DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. \ No newline at end of file +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

Alhadis · 2018-09-21T14:58:48Z

  1) Error:
TestFileBlob#test_include_in_language_stats:
RegexpError: empty char-class: /\A\s*[{[]/

Ah whoops, my bad. Sorry, that expression should have been:

/\A\s*[{\[]/

externl · 2018-09-21T14:59:54Z

@externl Could you implement the above heuristic rule with the corresponding test in test/test_heuristics.rb?

Is this correct?

def test_ice_by_heuristics
    assert_heuristics({
      "Slice" => all_fixtures("Slice", "*.ice"),
      "JSON" => all_fixtures("JSON", "*.ice")
    })
  end

Alhadis · 2018-09-21T15:00:37Z

Looks fine to me.

externl · 2018-09-21T15:07:51Z

How do I fix:

/home/travis/build/github/linguist/vendor/licenses/grammar/vscode-slice.txt:
  - cached license data out of date0m

lildude · 2018-09-21T18:33:58Z

How do I fix:

/home/travis/build/github/linguist/vendor/licenses/grammar/vscode-slice.txt:
  - cached license data out of date0m

You need to ensure that the SHA in the version is the same as the SHA for the committed grammar submodule. script/licensed should have done this for you if your local repo was on the same SHA as that that you've committed in this PR.

Committed submodule SHA: 635d41ba3924f168ba5a54101bf71fbf8aef3d20
License version: 6a28c2125bb442fa6a7f31c995ce423b1c9bbfb7

As you can see, they're not the same.

pchaigno

LGTM! Thanks @externl for your patience!

Alhadis

LGTM. I'm mortified that I didn't think to make an "Ice-ense" pun somewhere, though. 😓

externl · 2018-09-22T12:10:35Z

@pchaigno @Alhadis Thanks for guiding me through the process.

LGTM. I'm mortified that I didn't think to make an "Ice-ense" pun somewhere, though. 😓

I'm going to start using this 😄

lildude · 2018-09-25T14:32:17Z

lib/linguist/languages.yml

+  ace_mode: text
+  extensions:
+  - ".ice"
+  language_id: 894641667


Errm, don't you want a colour associated with this language? At the moment it's going to fallback to the default of #cccccc.

Looking at https://doc.zeroc.com/ice/latest/the-slice-language, it seems blue is the colour of choice so it's probably going to be a PITA to find one that doesn't conflict with one already associated with a language on GitHub.com - blue is really really popular.

Whoops, would definitely like a colour. Can we use #477ed0?

I doubt it. Various shades of blue are already highly used. Try it. Add the color: entry and then run bundle exec ruby test/test_color_proximity.rb (I'm assuming you've already run script/bootstrap). Rinse and repeat until you find a colour the test likes, then commit your colour and push.

Thanks @lildude, I found a blue! #003fa2

🎉 nice!!

lildude

LGTM. Thanks for the PR and welcome to the world of Linguist.

lib/linguist/heuristics.yml

Add Slice language (.ice)

4a755f5

pchaigno suggested changes Aug 21, 2018

View reviewed changes

Add heuristic to distinguish Slice

5576723

Merge remote-tracking branch 'github/master'

daf1ee0

externl added 2 commits September 21, 2018 10:27

Merge remote-tracking branch 'github/master'

18ab06c

Add Slice disambiguations

631d199

Fix license and add JSON disambiguation

cb4e1d5

Alhadis reviewed Sep 21, 2018

View reviewed changes

Add Slice heuristic test and fix JSON regex

e050ee4

Fix ice license version

a321d9a

pchaigno approved these changes Sep 22, 2018

View reviewed changes

Alhadis approved these changes Sep 22, 2018

View reviewed changes

lildude reviewed Sep 25, 2018

View reviewed changes

Add Slice color

ed727fd

lildude approved these changes Sep 25, 2018

View reviewed changes

lildude merged commit 8cd9d74 into github-linguist:master Sep 25, 2018

bzz reviewed Jan 8, 2019

View reviewed changes

lib/linguist/heuristics.yml Show resolved Hide resolved

bzz mentioned this pull request Jan 10, 2019

Disambiguate Slice and JSON languages for .ice #4376

Merged

3 tasks

bzz mentioned this pull request Jan 24, 2019

Tests: assert_heuristics does not fail on missing samples #4391

Closed

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

Add Slice language (.ice) #4243

Add Slice language (.ice) #4243

Conversation

externl commented Aug 21, 2018 • edited Loading

Checklist:

externl commented Aug 21, 2018

pchaigno left a comment

Choose a reason for hiding this comment

externl commented Aug 22, 2018

pchaigno commented Aug 22, 2018 • edited Loading

lildude commented Aug 22, 2018

pchaigno commented Aug 22, 2018

lildude commented Aug 22, 2018

externl commented Aug 22, 2018 • edited Loading

pchaigno commented Aug 22, 2018

externl commented Aug 22, 2018

externl commented Aug 22, 2018 • edited Loading

pchaigno commented Aug 25, 2018

externl commented Aug 28, 2018

pchaigno commented Aug 28, 2018

externl commented Aug 28, 2018

pchaigno commented Aug 28, 2018

externl commented Sep 21, 2018

pchaigno commented Sep 21, 2018

Alhadis commented Sep 21, 2018

pchaigno commented Sep 21, 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

Alhadis commented Sep 21, 2018 • edited Loading

externl commented Sep 21, 2018 • edited Loading

Alhadis commented Sep 21, 2018

externl commented Sep 21, 2018

lildude commented Sep 21, 2018 • edited Loading

pchaigno left a comment

Choose a reason for hiding this comment

Alhadis left a comment

Choose a reason for hiding this comment

externl commented Sep 22, 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

lildude left a comment

Choose a reason for hiding this comment

externl commented Aug 21, 2018 •

edited

Loading

pchaigno commented Aug 22, 2018 •

edited

Loading

externl commented Aug 22, 2018 •

edited

Loading

externl commented Aug 22, 2018 •

edited

Loading

Alhadis commented Sep 21, 2018 •

edited

Loading

externl commented Sep 21, 2018 •

edited

Loading

lildude commented Sep 21, 2018 •

edited

Loading