Added Support for Bro language

[wayward710]

Added support for Bro language.

[wayward710]

Added support for Bro language. Please let me know if you need any additions or changes.

[wayward710]

Please ignore the change I made to examples.js -- I did that for local testing purposes.

[zeitgeist87]

Hi @wayward710

Thanks for your contribution! Bro is a really interesting language.

Here are some things I've noticed about your PR:

• There are some minor whitespace issues in your PR. You mixed tabs and spaces for indentation. You should only use tabs.

• Please ignore the change I made to examples.js -- I did that for local testing purposes.

  Well you have to remove it from the PR eventually 😄

• You overuse the array syntax for tokens a little bit. The performance is actually quite bad, so it is better to avoid it if possible. There are other issues too. For example the order of the array elements is important. The first elements are matched first and can prevent longer and better matches from other elements down the line.

• You should use non-capturing groups whereever possible \b(?:keyword1|keyword2)\b

• On the example page you use the module keyword in an example, but it is missing in the language definition.

• I don't know anything about Bro, but in your example, the redef builtin seems to be used without the leading ampersand. Is the ampersand part of the builtin keyword or more like an operator? On this page it seems to be highlighted as an operator https://www.bro.org/sphinx/scripting/index.html

[zeitgeist87]

You can use a single regex instead of the array here. You should also use a non-capturing group.

[zeitgeist87]

I am away for the weekend, so I cannot answer any questions until Monday...

[wayward710]

Thanks for your feedback! I'm also traveling this weekend, so it may be a few days before I can work on the changes.

[wayward710]

OK, I think I fixed the issues that you pointed out. Please let me know if anything else needs to be changed.

[zeitgeist87]

Hi @wayward710,

Thanks! It looks really nice now.

There are only a few small issues. Other than that I think it is ready to be merged.

• You still have some whitespace issues. I know it is not all that important, but it just looks unprofessional:
  
  You should configure your editor to display whitespace characters and automatically remove trailing whitespace on save.

• Your tokens seem to be in a random order, but the order is important, because the tokens are matched by Prism in that order. For example operator and punctuation should be last. Operators that are matched too early can prevent later matches. For example the minus sign in @load-sigs is matched as an operator:
  

• I don't know Bro, but I assume that this 'italic': /\b(TODO|FIXME|XXX)\b/, should be inside of a comment?

  'comment': {
  pattern: /(^|[^\\$])#.*/,
  lookbehind: true,
  inside: {
      'italic':  /\b(TODO|FIXME|XXX)\b/,
  }
  },

• What minimizer do you use? If I use gulp to generate prism-bro.min.js I get a different result than yours. Maybe you used another version of gulp...

• When you are finished you should also squash all your commits down into one or two nice commits, so that we don't have all the corrections in the log.

[wayward710]

OK, I fixed the whitespaces, reordered the tokens, fixed the italics, and used Gulp for magnification instead of the other tool.

[zeitgeist87]

Thanks for your contribution!

It would be really nice if you could create a few test cases for the Bro language. The tests are in the directory tests/languages. You can run the tests with the command npm test. The syntax should be straight forward.

[zeitgeist87]

I missed that this was a PR for the master branch. You should create your PR for the gh-pages branch. I moved your commits there and removed them from master.

[Golmote]

Guidelines regarding how to write tests can be found here: http://prismjs.com/test-suite.html

[wayward710]

OK, and use the gh-pages branch for the test suites?

[Golmote]

Yes. All PRs should be done on gh-pages ;)