-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 missing Modula-2 *.def extension #6671
base: master
Are you sure you want to change the base?
Add missing Modula-2 *.def extension #6671
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See inline comment
lib/linguist/heuristics.yml
Outdated
- extensions: ['.def'] | ||
rules: | ||
- language: Modula-2 | ||
pattern: '^\s*(?i:DEFINITION\s+MODULE|END)\s+[\w\.]+\s*;' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This heuristic will never be used as this is the only user of this extension within linguist which in itself is potentially a problem as .def
is an incredibly generic extension used by many things.
If you're 100% certain this regex will only match Modula-2 files, then adding this extension to the generic.yml
file will ensure this regex takes effect and is used. If there's a risk this misidentifies other languages, it's best to leave off this extension and use an override when needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for missing that, I thought I'd tested a failing case but obviously not.
The pattern for *.def
is more specific than that applied to *.mod
filenames and there are only 30% the number of files on GitHub. I tested as well as I can with GitHub search and didn't find any false positives.
The current matching for *.mod
is also very poor. Most of the GNU Modula-2 repository appears to match as AMPL, for example. So, I've also fixed that pattern to limit it to Modula-2 syntax.
Some breaking changes were made to address #3433 but they don't make any sense to me. The issue reporter seems to be using Active Oberon, a different language. They may be asking for these files to match as Modula-2 because the syntax is similar, Oberon being the next in the line of Niklaus Wirth's languages but not recognised by Linguist.
However, Oberon is is no more Modula-2 than it is Pascal. Modula-2 does not support a .
in the module name and I can find no examples of any old non-standard Modula-2 dialects supporting that either.
f607d34
to
a03e63e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a heuristic test for this extension.
Improve *.mod pattern to match implementation modules. These were only being incidentally matched via the /END\./ pattern branch. See github-linguist#3657 and github-linguist#6451 for related discussions.
a03e63e
to
313cbf8
Compare
Done, thanks. |
Sorry. I should have been clearer... we need the heuristic test and fixtures as implemented by other generic extensions. See what has been done for the likes of |
Modula-2 splits the definition of a module into separate interface and implementation files having extensions
*.def
and*.mod
respectively.Linguist currently only detects
*.mod
files. This PR adds support for*.def
file detection.See #3657 and #6451 for related discussions.
I'm not sure why
END
is in those patterns. Assuming a top down search theMODULE
line will obviously be found first but I may be missing something about the implementation.Description
Checklist: