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 support for Expect scripts #6614

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cooljeanius
Copy link
Contributor

@cooljeanius cooljeanius commented Nov 12, 2023

Closes #6613

Description

Expect is an extension to the Tcl scripting language, so it can be categorized as Tcl. See #6613

Checklist:

@cooljeanius cooljeanius marked this pull request as ready for review November 12, 2023 23:49
@cooljeanius cooljeanius requested a review from a team as a code owner November 12, 2023 23:49
@cooljeanius
Copy link
Contributor Author

OK I think this is ready for review now; I don't think a new heuristic is necessary, since I don't see anything else using the .exp extension that would need to be distinguished from.

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.

I don't think a new heuristic is necessary, since I don't see anything else using the .exp extension that would need to be distinguished from.

I just remembered that .exp is sometimes used in test-suites to contain the expected output of a test run. It's usually paired with a file ending in .act (to denote the actual output from a test), which is compared with the .exp file using the diff(1) utility.

There also appear to be roughly ~203k search results for the .exp extension that doesn't include lines beginning with set or expect. They don't appear to be fixtures for a test-suite either, so this might warrant a more in-depth investation.

lib/linguist/languages.yml Outdated Show resolved Hide resolved
@Alhadis Alhadis changed the title Update languages.yml to recognize expect as a form of Tcl Add support for Expect scripts Nov 13, 2023
sort tcl interpreters properly

Co-authored-by: John Gardner <gardnerjohng@gmail.com>
@cooljeanius
Copy link
Contributor Author

I don't think a new heuristic is necessary, since I don't see anything else using the .exp extension that would need to be distinguished from.

I just remembered that .exp is sometimes used in test-suites to contain the expected output of a test run. It's usually paired with a file ending in .act (to denote the actual output from a test), which is compared with the .exp file using the diff(1) utility.

There also appear to be roughly ~203k search results for the .exp extension that doesn't include lines beginning with set or expect. They don't appear to be fixtures for a test-suite either, so this might warrant a more in-depth investation.

Hm, so maybe just a really simple heuristic then? e.g. '^\s*(?:set|expect|proc)\s' or something... (I'm not very good at regexes, so please correct me if I got it wrong)

@Alhadis
Copy link
Collaborator

Alhadis commented Nov 30, 2023

Hm, so maybe just a really simple heuristic then?

Actually, I was thinking .exp might need to be registered as a generic extension. This is a more esoteric feature of Linguist that's called upon when an extension has widespread usage in unrelated contexts, but none of them have enough adoption individually to merit their own entries in languages.yml. The original use-case was to support Solidity's .sol extension without misclassifying the numerous repositories that used .sol as an abbreviated suffix for "solution".

However, generic extensions have the caveat that an accurate, airtight heuristic is needed, preferably one that matches a pattern or string that's all but guaranteed to appear in the files that we're interested in supporting. The regex ^\s*(?:set|expect|proc)\s will match any line that happens to begin with the word "set", "expect" or "proc", followed by a whitespace character (including a newline). Needless to say, that's very, very error-prone; even if we weren't looking to add a generic extension, I still wouldn't recommend such an open-ended heuristic.

I'm happy to help you with penning the regex, but that assumes there even is a predictable and guaranteed pattern unique to Expect scripts.

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.

Add support for expect (as a form of tcl)
2 participants