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 go.mod heuristics #6996

Closed
wants to merge 1 commit into from

Conversation

goto1134
Copy link
Contributor

Description

Checklist:

  • I am fixing a misclassified language

    • I have included a new sample for the misclassified language:
      • Sample source(s):
        • [URL to each sample source, if applicable]
      • Sample license(s):
    • I have included a change to the heuristics to distinguish my language from others using the same extension.

    As for the samples I've provided, they are modified copies of existing samples in the repository.

@goto1134 goto1134 requested a review from a team as a code owner August 16, 2024 09:50
@lildude
Copy link
Member

lildude commented Aug 16, 2024

This won't work as .mod is not associated with the "Go Module" language, and it probably shouldn't be.

A better solution would be to add .go.mod as an extension to the current language definition:

Go Module:
type: data
color: "#00ADD8"
aliases:
- go.mod
- go mod
filenames:
- go.mod
tm_scope: go.mod
ace_mode: text
language_id: 947461016

... though usage isn't anywhere near popular enough for inclusion right now.

Until then, peeps will need to use an override to force the langauge.

@goto1134
Copy link
Contributor Author

goto1134 commented Aug 16, 2024

though usage isn't anywhere near popular enough for inclusion right now.

What are the criteria for being popular enough?

For the record, almost every Golang repo uses the go.mod files. >900k results in the following search: https://github.com/search?q=path:go.mod&type=code

This won't work as .mod is not associated with the "Go Module" language, and it probably shouldn't be.

A better solution would be to add .go.mod as an extension to the current language definition:

@lildude , thank you for your suggestion!

Most of the current go.mod files are misclassified for AMPL in the search, unless I query path:go.mod language:"Go Module".

As far as I understand, the following code:

@exts.any? { |ext| filename.end_with?(ext) }

... says that heuristics are applied based on a filename suffix, and they do not depend on the extensions listed in languages.yml.

Could you please elaborate more on how are languages.yml connected with the heuristics.yml and how modifying the languages.yml will fix the problem I see?

@lildude
Copy link
Member

lildude commented Aug 16, 2024

What are the criteria for being popular enough?

See #5756, also referenced from the CONTRIBUTING.md file.

For the record, almost every Golang repo uses the go.mod files. >900k results in the following search

Indeed. And Linguist already has support for go.mod files. I was referring to *.go.mod files. There are only 61 files indexed with the .go.mod extension.

Most of the current go.mod files are misclassified for AMPL in the search, unless I query path:go.mod language:"Go Module".

That's only in the search results, which uses a different internal library to detect the language. This internal library happens to feed off Linguist, but not perfectly as it's not a 1:1 match in terms of functionality. The actual files are already correctly identified by Linguist and the syntax highlighter applies the correct syntax highlighting too.

We can confirm this from the first result in your search results: https://github.com/jmoiron/sqlx. This is what the search results show - wrong language and wrong syntax highlighting:

Screenshot 2024-08-16 at 11 47 05

However if you view the file, you get the correct syntax highlighting:

Screenshot 2024-08-16 at 11 48 21

And if we use Linguist to analyse it, we get the correct language:

github-linguist sqlx/
99.74%  220442     Go
0.26%   583        Makefilegithub-linguist sqlx/go.mod
sqlx/go.mod: 9 lines (7 sloc)
  type:      Text
  mime type: text/plain
  language:  Go Module

Which aligns with the language stats shown in the sidebar of the repo:

Screenshot 2024-08-16 at 11 54 19

AMPL is considered a detectable language by default in Linguist so if Linguist had incorrectly determined the language as AMPL, it would appear in the results above.

Unfortunately, no amount of tweaking Linguist is going to resolve this issue with what Search is showing. The team that handles Search will need to address this incorrect behaviour.

Please use the "Contact" link at the bottom of the page showing the wrong search results and log a support ticket. The issue will get routed to the correct team to address.

@goto1134
Copy link
Contributor Author

goto1134 commented Aug 16, 2024

@lildude , thank you for clarifying that! It looks like my changes are not required, and the problem is on the GitHub side. Should I report the problem somewhere?

@lildude
Copy link
Member

lildude commented Aug 16, 2024

@lildude , thank you for clarifying that! It looks like my changes are not required, and the problem is on the GitHub side. Should I report the problem somewhere?

Yes. From my previous comment 😉:

Please use the "Contact" link at the bottom of the page showing the wrong search results and log a support ticket. The issue will get routed to the correct team to address.

@goto1134 goto1134 closed this Aug 16, 2024
@goto1134 goto1134 deleted the go-mod-heuristics branch August 16, 2024 11:10
@goto1134
Copy link
Contributor Author

For the record, here is a bug report: https://github.com/orgs/community/discussions/136097

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.

3 participants