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

Update YARA language config #4202

Merged
merged 5 commits into from
Oct 5, 2018
Merged

Update YARA language config #4202

merged 5 commits into from
Oct 5, 2018

Conversation

wesinator
Copy link
Contributor

@wesinator wesinator commented Jul 19, 2018

Description

  • Changes YARA language ace mode to c_cpp
  • Change type to programming

These are more appropriate for the syntax

Checklist:

  • I am adding new or changing current functionality
    • I have added or updated the tests for the new or changed functionality.

@pchaigno
Copy link
Contributor

@lildude Are you making progress on Lightshow? It would be good if we could test this.

@lildude
Copy link
Member

lildude commented Jul 20, 2018

@lildude Are you making progress on Lightshow? It would be good if we could test this.

Unfortunately not. It's out of my hands at the moment.

@wesinator wesinator changed the title Update YARA ace mode Update YARA language config Aug 7, 2018
Copy link
Contributor

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Not sure why I thought we needed Lightshow here. It's not something Lightshow can test anyway.

extensions:
- ".yar"
- ".yara"
tm_scope: source.yara
ace_mode: c_cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

@wesinator The syntax looks very different from C++. Did you check the output this mode produces for YARA? Could you add a screenshot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comments, indentation, and bracketing are similar, but if it adds C/Cpp keywords then I'll change it back.

Copy link
Contributor

Choose a reason for hiding this comment

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

@lildude Any way to test this?

Copy link
Member

Choose a reason for hiding this comment

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

Nope, none what-so-ever, as as I've just discovered all the editors on the GitHub.com interface are now using CodeMirror (as of Sep 2016 and GitHub Enterprise 2.8 onwards) and reflected as such in the docs at https://help.github.com/articles/editing-files-in-your-repository/ and https://help.github.com/articles/about-gists/ and https://help.github.com/articles/using-keyboard-shortcuts/.

#3243 was the Linguist side of things that put the finishing touches in place.

So this line will have absolutely no effect on how things look in GitHub.com right now, though may in future as we may switch back to Ace, or other Ace-compatible editor, if it's deemed necessary to do so at some time in the future.

@wesinator If you want to make an effective change, you'll need to implement the codemirror_mode and codemirror_mime_type equivalents too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, none what-so-ever, as as I've just discovered all the editors on the GitHub.com interface are now using CodeMirror

Oh, right. I forgot that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, codemirror doesn't have a builtin YARA mode.

linguist doesn't use the language grammars for editing ?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it uses CodeMirror.

Copy link
Contributor

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Being a programming language, YARA will be displayed in language statistics so we'll need to add a color.

@wesinator
Copy link
Contributor Author

@pchaigno Done - that was not why the tests were failing though, I don't see why they would be failing.

@pchaigno
Copy link
Contributor

They're failing because of an error in #4208 which was fixed in #4230. Could you update your branch with the latest master?

@lildude lildude merged commit c1d81cc into github-linguist:master Oct 5, 2018
@wesinator wesinator deleted the yara-update branch December 27, 2018 20:41
@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.

4 participants