1) Failure:
TestFileBlob#test_language [/home/runner/work/linguist/linguist/test/test_file_blob.rb:671]:
No language for /home/runner/work/linguist/linguist/test/fixtures/Solidity/ignored2.sol

 2) Failure:
TestBlob#test_language [/home/runner/work/linguist/linguist/test/test_blob.rb:276]:
No language for /home/runner/work/linguist/linguist/test/fixtures/Solidity/ignored2.sol

I'd like to know why these tests didn't fail when I ran them locally...

I'd also like to better understand where we use the samples/ files, apart from end-to-end tests and training of the Bayesian classifier. Those two are fine because we use fixture files for end-to-end tests and we don't use the Bayesian classifier. If we usually rely on the samples/ files for other things, we might have problems...

We currently don't have any Solidity "samples", only test "fixtures" (our nomenclature really needs improvement…). Adding .sol files to other sample directories would actually break the tests, as there's a check to make sure only extensions listed in languages.yml appear in the samples/* directories.

That being said, we can certainly add more real-world examples to the test/fixtures/Solidity directory, but not as samples/.

I'd also like to better understand where we use the samples/ files, apart from end-to-end tests and training of the Bayesian classifier. Those two are fine because we use fixture files for end-to-end tests and we don't use the Bayesian classifier. If we usually rely on the samples/ files for other things, we might have problems...

We currently don't have any Solidity "samples", only test "fixtures" (our nomenclature really needs improvement…). Adding .sol files to other sample directories would actually break the tests, as there's a check to make sure only extensions listed in languages.yml appear in the samples/* directories.

I understand that. My point was that we don't and can't have Solidity files in samples/ and I'm therefore wondering if that can have a negative impact. I.e., are there tests (or, less likely, features) we will be missing because of the lack of samples/ Solidity files.

That being said, we can certainly add more real-world examples to the test/fixtures/Solidity directory, but not as samples/.

Yes, that's my point 2 above.

I've also added .sol as a Gerber Image extension. In-the-wild usage checks out:

• Files: 2,389 out of 3,108 total
• Repos: 1,047
• Users: 884

Plus, adding this proves that more than one language can share a generic extension.

@pchaigno Think we'd be able to include what #4258 attempted to fix? Though we already have a dedicated Manpage strategy, it makes more sense to mark numeric file-extensions as "generic" instead.

@lildude Any input regarding @pchaigno's suggested change? To me, it makes more sense to have generic extensions added to a YAML database named generic.yml (a la vendored filenames and extensions). What do you think?

Sorry @Alhadis. Things are crazy busy for me at the moment as we're reading the next release of GitHub Enterprise. I'll try take a look when I've got a chance when things quieten down a bit.

I understand.

@pchaigno What if I added a generic.yml file and modified strategy/extension.rb to use that instead of an array hardcoded into the strategy? I can always extend the scope of this PR to include other generic file extensions:

# generic.yml
# (Some concise but informative description of what this file does and how to update it)

extensions:
- ".1"
- ".2"
- ".3"
- ".4"
- ".5"
- ".6"
- ".7"
- ".8"
- ".9"
- ".in"
- ".mod"
- ".sol"

filenames:
- unoriginal_but_common_filename

Hey @lildude, do you have a rough estimation about when you'll be able to review this? Not to pressure you, but just asking so we can better coordinate with @Alhadis. Thanks!

I'm looking at it right now. It might take me a few days to process it and respond, but I've got time this week for Linguist stuff.

Thanks for the quick answer, @lildude. This is great news :)

Excluding the numbered files @Alhadis has mentioned,

I'll leave those for a future PR. At the moment, I'm just anxious to get this under the door in time for the next Linguist release.

I've pondered this for the week, and I comfortable with the approach taken here.

This approach means we lose any further assistance from the XML and classifier strategies but I think this is OK as this we already expect the heuristics to be pretty specific to reduce the chances of falling through to the classifier. I think this will also be a rarely used "exception" (famous last words 😉) so I'm happy to experiment and iterate as needed.

My only request for this PR is to remove the unneeded examples from the test fixtures. I think we only need fixtures to confirm each of the heuristic scenarios, eg we only need one for "ignored" and Gerber and a few to match the various combos of the Solidity regex.

My only request for this PR is to remove the unneeded examples from the test fixtures. I think we only need fixtures to confirm each of the heuristic scenarios, eg we only need one for "ignored" and Gerber and a few to match the various combos of the Solidity regex.

That was my line of thinking as well, but @pchaigno requested the extra samples for some reason, so… 🤷 Getting some mixed signals here.

Will nuke 'em, hang 5.

I'm not sure why @lildude wanted to see these examples removed

Only cos they weren't really testing the heuristic so I didn't see much need for them.

can be please please please not squash all commits (🙏 @lildude).

Only if you say please once more 😜

It'll mean I have to do a little more work curating the release notes, but that's fine in this case.

It'll mean I have to do a little more work curating the release notes, but that's fine in this case.

@pchaigno requested this before I renamed and rewrote the PR description to place more emphasis on the generic-extension feature, rather than the reasons for adding it (Solidity). I feel what I've written nicely encapsulates everything that's been changed here… I'm still waiting on @pchaigno to give his approval.

In general not a big fan of squashing non-trivial PRs, but should be okay here with the new description.

Awesome. 🎉 Linguist finally has Solidity support.

Now all that's left to restore balance to the universe is getting rid of that f\*(&#g colour proximity requirement. 😜

Oh. You merged it already. I just allowed merge commits and was going to then merge. Oh well.

Uh, sorry, I mistook the sudden silence as "go ahead, you merge it". Sorry. 😅

Great news!

🎉 Lets do this!! I'll make a release next week once all the other PRs I've approved have been checked off by you two too.

Does this mean that in about a week it will be live on github?

Does this mean that in about a week it will be live on GitHub?

You bet your sweet arse it does.

Does this mean that in about a week it will be live on github?

Indeed. There are a few more PRs waiting reviews from @pchaigno and @Alhadis that I'd like to include but once approved, I'll get cracking with the release. I'm AFK tomorrow so won't start until Tuesday at the earliest.

Awesome! 🥳🥳🥳

Will this have any other impact apart from automatically associating the .sol extension to Solidity? I wonder if Linguist is used for other things at GitHub

No, it won't. I was careful to ensure the existing filename logic wasn't affected.

As for what else Linguist is used for… I'm not staff, so I can't tell you, but I do know that Linguist is used by GitLab, and possibly other VCS-hosting services too.

Thanks for your reply, @Alhadis

Yep, GitLab is the only other consumer of Linguist I'm aware of, but they tend to lack a bit behind in terms of releases. So it may take some time before the change is visible on gitlab.com and related software.