Tests passing is order dependent

[joshgoebel]

The specific example: Livescript and SQF.

If you change the order such that SQF runs BEFORE CPP (which it has a dependency on) then the Livescript "detect" test will fail and incorrectly identify the snippet as SQF. This is also a problem outside of tests as the results you get for SQF will vary depending on whether you have previously highlighted a CPP sample or not.

What is the problem?

The problem is that SQF is flagged as case in-sensative while CPP is not. SQF depends on the CPP preprocessor definition. So all the regular expressions for the preprocessor stuff is either "compiled" to case-insensative regexes (or not) depending on which happens to be used first (SQF or CPP).

What is the correct behavior here? Should we really even be pulling in the pre-processor from CPP just because the lines start with #? These appear to be comments in SQF, not pre-processor statements.

Thoughts?

[joshgoebel]

I'd say lets just have a simple rule for parsing comments, and remove the dependency on CPP. Objections? If can whip up a PR if that's a good route.

[egor-rogov]

What is the correct behavior here? Should we really even be pulling in the pre-processor from CPP just because the lines start with #? These appear to be comments in SQF, not pre-processor statements.

As far as I get, SQF uses C/CPP-like preprocessor: https://community.bistudio.com/wiki/PreProcessor_Commands (I don't have a faintest understanding of SQF though).
I agree we'd better remove the dependency, but it looks like we need to copy all the preprocessor stuff into sqf.js.

[joshgoebel]

ROFL. Then the test examples are terrible (no offense), because they make the # lines look like comments... so:

• Remove the dependency (just copy over the C stuff unless I see a glaring difference)
• Update the test sample to actually be realistic and look like some pre-processor examples? (like from your sample link)

Agree?

[egor-rogov]

Then the test examples are terrible (no offense), because they make the # lines look like comments...

Hm, where did you find them? I can see only detection test (no markup tests at all), and this one doesn't contain hashes.

Remove the dependency (just copy over the C stuff unless I see a glaring difference)

Yep.

Update the test sample to actually be realistic and look like some pre-processor examples? (like from your sample link)

I'm not sure if this is a good idea to touch tests (test) without understanding of the language... Maybe just leave it be?

[joshgoebel]

Hm, where did you find them? I can see only detection test (no markup tests at all), and this one doesn't contain hashes.

SO sorry for confusion, I was thinking of livescript... which evidently uses # for comments. Nevermind. So many languages.

[joshgoebel]

#2141

[joshgoebel]

@egor-rogov Sadly this doesn't fix the test but rather make it constantly fail (since SQF is case insensitive)... CPP being sensitive was the only reason this was hidden before and didn't come to light.

Livescript is a VERY light language without much to "detect"... Right now SQF wins against livescript 22 to 21 (most because a number is a number and Livescript samples has a lot of numbers)...

How are these types of issues usually resolved? It seems impossible in the real world that Highlight.js could 100% reliably detect which language if two languages are VERY similar.

[joshgoebel]

It would help (luckily, not a great fix) if just the preprocessor checks were case sensitive, but I don't think Highlight js can easily support mixed case rules, which is why that setting is currently only per language, yes?

[egor-rogov]

It seems impossible in the real world that Highlight.js could 100% reliably detect which language if two languages are VERY similar.

Absolutely.

How are these types of issues usually resolved?

All you need is black magic of 80th level... Basically we have two means:

1. Fine-tune relevance of some rules. For example, numbers have relevance = 0 in both languages.
2. Fine-tune detection test to make sure it contains enough language-specific constructs to be autodetected correctly.

[joshgoebel]

enough language-specific constructs

Yeah, that's the issue. I feel like livescript is trying really hard to avoid having it's own language specific constructs (that don't match say JS, etc)

For example, numbers have relevance = 0 in both languages.

Right, it wasn't numbers that was the problem here... I was mistaken... it's the built_in, keyword, and string matches the seem similar to SQF.

Don't help that we have data named name and SQF has a built-in named name.

<span class="hljs-meta"># take the first n objects from a list</span>
take = (n, [x, ...xs]:<span class="hljs-built_in">list</span>) --&gt;
  | n &lt;= <span>0</span>     =&gt; []
  | empty <span class="hljs-built_in">list</span> =&gt; []
  | otherwise  =&gt; [x] ++ take n - <span>1</span>, xs

take <span>2</span>, [<span>1</span>, <span>2</span>, <span>3</span>, <span>4</span>, <span>5</span>]

<span class="hljs-meta"># Curried functions</span>
take-three = take <span>3</span>
take-three [<span>6</span>, <span>7</span>, <span>8</span>, <span>9</span>, <span>10</span>]

<span class="hljs-meta"># Function composition</span>
last-three = <span class="hljs-built_in">reverse</span> &gt;&gt; take-three &gt;&gt; <span class="hljs-built_in">reverse</span>
last-three [<span>1</span> <span class="hljs-keyword">to</span> <span>8</span>]

<span class="hljs-meta"># List comprehensions and piping</span>
t1 =
  * id: <span>1</span>
    <span class="hljs-built_in">name</span>: <span class="hljs-string">'george'</span>
  * id: <span>2</span>
    <span class="hljs-built_in">name</span>: <span class="hljs-string">'mike'</span>
  * id: <span>3</span>
    <span class="hljs-built_in">name</span>: <span class="hljs-string">'donald'</span>

t2 =
  * id: <span>2</span>
    age: <span>21</span>
  * id: <span>1</span>
    age: <span>20</span>
  * id: <span>3</span>
    age: <span>26</span>
[{id:id1, <span class="hljs-built_in">name</span>, age}
  <span class="hljs-keyword">for</span> {id:id1, <span class="hljs-built_in">name</span>} <span class="hljs-built_in">in</span> t1
  <span class="hljs-keyword">for</span> {id:id2, age} <span class="hljs-built_in">in</span> t2
  where id1 is id2]
  |&gt; <span class="hljs-built_in">sort</span>-by \id
  |&gt; JSON.stringify

Any ideas on the best direction to go with this?

[joshgoebel]

I can just add a little more Livescript specific code to boost it's match score... or rename the variable name... but that feels like cheating...

[egor-rogov]

Like cheating, yeah. I can see no other way in this case, so feel free.

[joshgoebel]

#2141

[joshgoebel]

Closing as this was merged.