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

simplify .ms heuristic #4224

Merged
merged 1 commit into from
Aug 20, 2018
Merged

simplify .ms heuristic #4224

merged 1 commit into from
Aug 20, 2018

Conversation

smola
Copy link
Contributor

@smola smola commented Aug 6, 2018

Simplify .ms heuristic for Unix Assembly by just checking that there is no
/* string in the file. This is simpler and seems to work with all Unix Assembly
files in GitHub. A synthetic MAXScript file was also tested by adding assembly inside
a multiline comment.

Checklist:

None applies?

@smola smola mentioned this pull request Aug 6, 2018
2 tasks
@@ -323,7 +323,7 @@ def call(data)
disambiguate ".ms" do |data|
if /^[.'][a-z][a-z](\s|$)/i.match(data)
Language["Roff"]
elsif /(?<!\S)\.(include|globa?l)\s/.match(data) || /(?<!\/\*)(\A|\n)\s*\.[A-Za-z][_A-Za-z0-9]*:/.match(data.gsub(/"([^\\"]|\\.)*"|'([^\\']|\\.)*'|\\\s*(?:--.*)?\n/, ""))
elsif !/\/\*/.match(data) && /^\s*\.(include\s|globa?l\s|[A-Za-z][_A-Za-z0-9]*:)/.match(data)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't hurt to make this a non-capturing group:

        ↓↓
/^\s*\.(?:include\s|globa?l\s|[A-Za-z][_A-Za-z0-9]*:)
        ↑↑

Simplify .ms heuristic for Unix Assembly by just checking that there is no
`/*` string in the file. This is simpler and seems to work with all Unix Assembly
files in GitHub. A synthetic MAXScript file was also tested by adding assembly inside
a multiline comment.
@smola
Copy link
Contributor Author

smola commented Aug 20, 2018

@Alhadis could you have a look?

@Alhadis
Copy link
Collaborator

Alhadis commented Aug 20, 2018

Looks good to me. 👍

@pchaigno pchaigno requested a review from lildude August 20, 2018 07:46
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

@lildude lildude merged commit 1221480 into github-linguist:master Aug 20, 2018
@smola smola deleted the simplify-ms branch August 21, 2018 08:35
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants