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

Added case-insensitive matches for FreeBasic. #6367

Merged
merged 10 commits into from May 30, 2023

Conversation

XusinboyBekchanov
Copy link
Contributor

Description

Added case-insensitive matches for FreeBasic heuristics.

Checklist:

  • I am fixing a misclassified language (FreeBasic)

    • 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.
  • I am changing the source of a syntax highlighting grammar

    • Old: [URL to grammar repo]
    • New: [URL to grammar repo]
  • I am updating a grammar submodule

  • I am adding new or changing current functionality

    • I have added or updated the tests for the new or changed functionality.
  • I am changing the color associated with a language

    • I have obtained agreement from the wider language community on this color change.
      • [URL to public discussion]
      • [Optional: URL to official branding guidelines for the language]

@XusinboyBekchanov XusinboyBekchanov requested a review from a team as a code owner April 11, 2023 06:26
@DecimalTurn
Copy link
Contributor

DecimalTurn commented Apr 18, 2023

After looking a little more into it, I'm not sure if it's a good idea to make the heuristics case-insensitive for FreeBasic mainly because when I look at FreeBasic code on GitHub, I see that the use of lowercase is prevalent and I actually found no use of uppercase in the first five pages of results when looking at the highlighted keywords.

Do you have any examples of FreeBasic repos or files missclassified because they were using a different casing convention than the norm?

@XusinboyBekchanov
Copy link
Contributor Author

Do you have any examples of FreeBasic repos or files missclassified because they were using a different casing convention than the norm?

For example, here is a capital letter:
https://github.com/bob-the-hamster/ohrrpgce/blob/f8cb11c86de7eb1c9c32ae60bb7bd40f131ab97a/miditest.bas

@DecimalTurn
Copy link
Contributor

DecimalTurn commented Apr 22, 2023

For example, here is a capital letter:
bob-the-hamster/ohrrpgce@f8cb11c/miditest.bas

I see that this file still makes use of lowercase #include, so the lowercase heuristic would still work.

However, after looking further, I've found the following file:
https://github.com/doranchak/azdecrypt/blob/a9dbde3adc451c1aaccb4b4339f81e1ab5dafc0d/FreeBASIC-1.09.0-winlibs-gcc-9.3.0/examples/GUI/GTK%2B/FB_Calc/FB_Calc.bas#L3

It does only include the use of #INCLUDE and is indentified by Linguist as v7.25 as VBA, so I guess this would be a good case to justify the use of case-insensitive matches.

In that case, I'm in favor of this change even if it won't make a big difference since this won't interfere with other languages currently supported in Linguist.

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.

While we're here, we might as well harden these heuristics to deal with files that aren't terminated by a trailing newline (something that's more common on Windows). For example:

regex = /^[ \t]*#(?i)(?:define|endif|endmacro|ifn?def|include|lang|macro)\s/
"Foo\nBar\n#endif\n" =~ regex # => 8
"Foo\nBar\n#endif"   =~ regex # => nil

lib/linguist/heuristics.yml Outdated Show resolved Hide resolved
lib/linguist/heuristics.yml Outdated Show resolved Hide resolved
XusinboyBekchanov and others added 2 commits April 25, 2023 18:17
Co-authored-by: John Gardner <gardnerjohng@gmail.com>
Co-authored-by: John Gardner <gardnerjohng@gmail.com>
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.

LGTM. Thanks.

Note: this PR will not be merged until close to when the next release is made. See here for more details.

@lildude lildude requested a review from a team as a code owner May 30, 2023 09:14
@lildude lildude added this pull request to the merge queue May 30, 2023
Merged via the queue into github-linguist:master with commit 6f08b88 May 30, 2023
5 checks passed
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

4 participants